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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
