Skip to content

allow "used only once" warnings to be fatal - #21693

Merged
tonycoz merged 1 commit into
Perl:bleadfrom
tonycoz:13814-fatal-only-once
Dec 7, 2023
Merged

tonycoz merged 1 commit into
Perl:bleadfrom
tonycoz:13814-fatal-only-once

Conversation

@tonycoz

@tonycoz tonycoz commented Dec 5, 2023

Copy link
Copy Markdown
Contributor

"used only once" warnings are special, instead of being emitted at the code where the name in question is used, they are emitted during a scan of the symbol table done after parsing has finished.

This meant that any FATAL flags set in the COP for the parse point of the name is no longer in scope, so the warnings we emit can't be treated as fatal.

To make them behave as FATAL set a new flag on the name if fatal WARN_ONCE warnings are enabled and use that to dispatch the warnings as normal or fatally when we do the symbol table scan.

I originally approached the dispatch as fatal or non-fatal by messing around with cop_warnings, but that was dumb, and I went for a much simpler change.

Fixes #13814

Comment thread gv.c Outdated
HEKfARG(HvNAME_HEK(stash)),
HEKfARG(GvNAME_HEK(gv)));
if (GvONCE_FATAL(gv)) {
Perl_fatal_warner(aTHX_ packWARN(WARN_ONCE),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to use the full Perl_...(aTHX_...) version of a function call? Is the macro generated in embed.h visible here, so this could be just fatal_warner(packWARN...)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New-fangled varargs macros 馃樀

I've updated both of these.

Comment thread gv.c Outdated
HEKfARG(GvNAME_HEK(gv)));
}
else {
Perl_warner(aTHX_ packWARN(WARN_ONCE),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto here

Comment thread util.c Outdated
Comment on lines +2140 to +2141
This outputs a warning as if fatal warnings were enabled. This may
return in some cases.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't quite follow from these words what it is that the function does. Can you perhaps use more words to describe it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added more words. 馃槃

"used only once" warnings are special, instead of being emitted at
the code where the name in question is used, they are emitted during
a scan of the symbol table done after parsing has finished.

This meant that any FATAL flags set in the COP for the parse point of
the name is no longer in scope, so the warnings we emit can't be
treated as fatal.

To make them behave as FATAL set a new flag on the name if fatal
WARN_ONCE warnings are enabled and use that to dispatch the warnings
as normal or fatally when we do the symbol table scan.

I originally approached the dispatch as fatal or non-fatal by messing
around with cop_warnings, but that was dumb, and I went for a much
simpler change.

Fixes Perl#13814
@tonycoz
tonycoz force-pushed the 13814-fatal-only-once branch from 5d0e799 to 9bb85c7 Compare December 7, 2023 04:03

@leonerd leonerd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tonycoz
tonycoz merged commit 01b6007 into Perl:blead Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Name "%s::%s" used only once" warnings are not fatalized

2 participants