[PATCH] D80751: [clang][diagnostics] Add '-Wundef-prefix' warning option

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticGroups.td:108
+def Undefined : DiagGroup<"undef">;
+def UndefinedPrefix : DiagGroup<"undef-prefix", [Undefined]>;
 def UnsupportedNan : DiagGroup<"unsupported-nan">;
----------------
zixuw wrote:
> arphaman wrote:
> > 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?
> 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).
Got it, thanks!


================
Comment at: clang/lib/Lex/PPExpressions.cpp:262
+          // string to UndefPrefixes as an explicit "-Wundef" does.
+          if (UndefPrefixes.empty() ||
+              llvm::any_of(UndefPrefixes,
----------------
zixuw wrote:
> ributzka wrote:
> > What happens when you use `-Werror=undef` and `-Wundef-prefix=<arg>`?
> 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`.
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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80751/new/

https://reviews.llvm.org/D80751