Page MenuHomePhabricator

[clang][diagnostics] Add '-Wundef-prefix' warning option
ClosedPublic

Authored by zixuw on May 28 2020, 11:56 AM.

Details

Summary

Add an -Wundef-prefix=<arg1>,<arg2>... option, which is similar to -Wundef, but only give warnings for undefined macros with the given prefixes.

Diff Detail

Event Timeline

zixuw created this revision.May 28 2020, 11:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 28 2020, 11:56 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
zixuw edited the summary of this revision. (Show Details)May 28 2020, 12:00 PM
ributzka added inline comments.Mon, Jun 8, 10:12 AM
clang/include/clang/Driver/Options.td
493

We should provide a description that doesn't mention that this is an alias of -Wundef-prefix, because the alias is an implementation detail.

clang/lib/Lex/PPExpressions.cpp
265

What happens when you use -Werror=undef and -Wundef-prefix=<arg>?

arphaman added inline comments.Mon, Jun 8, 3:13 PM
clang/include/clang/Basic/DiagnosticGroups.td
108 ↗(On Diff #266979)

It seems like you're not using the UndefinedPrefix group in this patch. Is that intentional, or was this left in the patch by mistake?

Can you still modify the state of -Wundef with #pragma clang diagnostic? Could you add a test to cover it.

zixuw added inline comments.Mon, Jun 8, 4:23 PM
clang/include/clang/Basic/DiagnosticGroups.td
108 ↗(On Diff #266979)

This is intentional to map both Wundef and Wundef-prefix to the same warning. And since Wundef is changed to an alias, the Undefined subgroup is used to keep existing outputs/behaviors consistent (to make -Werror=undef work, and to keep the [-Wundef] option name in the diagnostic message).

zixuw updated this revision to Diff 269628.Tue, Jun 9, 12:00 PM

Update test case for #pragma clang diagnostic
Update test case to verify that the state of -Wundef can still be modified using #pragma clang diagnostic

zixuw added inline comments.Tue, Jun 9, 12:15 PM
clang/lib/Lex/PPExpressions.cpp
265

Then we only get errors for undef-prefix=<arg>. Yes this needs to be fixed.
Perhaps a better way to handle the -Werror=undef implication of -Wundef.

arphaman added inline comments.Fri, Jun 12, 2:43 PM
clang/include/clang/Basic/DiagnosticGroups.td
108 ↗(On Diff #266979)

Got it, thanks!

clang/lib/Lex/PPExpressions.cpp
265

We should try to fix this in this PR then. Can you check the severity of the diagnostic, and if it's an error always emit it here?

zixuw added inline comments.Fri, Jun 12, 3:50 PM
clang/lib/Lex/PPExpressions.cpp
265

That is an option. But then we lose the ability to intentionally turning just the prefixes into errors, like -Wundef-prefix=FOO -Werror or -Wundef-prefix=BAR -Werror=undef-prefix

zixuw updated this revision to Diff 274212.Mon, Jun 29, 12:55 PM
  • Refine test cases to check combinations of 'Wundef' and 'Wundef-prefix', and with/without 'Werror';
  • Fix issues with '-Werror=undef' by explicitly looking for the option.
zixuw updated this revision to Diff 274219.Mon, Jun 29, 1:18 PM
  • Remove implementation details from the help text of 'Wundef';
  • Hide help text for 'Wundef-prefix' and 'Wundef'.
zixuw marked 5 inline comments as done.Mon, Jun 29, 1:18 PM
zixuw updated this revision to Diff 274276.Mon, Jun 29, 3:59 PM

Abort the design of making 'Wundef' an alias to 'Wundef-prefix' because it
depends on the alias expansion to work, which adds an empty string to 'UndefPrefixes'
to do the trick. However, any other way to enable 'Wundef', for example, via 'Werror=undef'
or '#pragma clang diagnostic', will not work and cannot be handled easily.

Instead, just suppress 'Wundef-prefix' when 'Wundef' is enabled.

zixuw edited the summary of this revision. (Show Details)Mon, Jun 29, 4:03 PM
arphaman accepted this revision.Mon, Jun 29, 9:30 PM

LGTM.

This revision is now accepted and ready to land.Mon, Jun 29, 9:30 PM
This revision was automatically updated to reflect the committed changes.