[PATCH] D116833: [clang][Sema] Disable -Wc++20-designator in system macros
Carlos Galvez via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 7 11:26:40 PST 2022
carlosgalvezp added inline comments.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:7161
+ !DiagnosedNestedDesignator && !DiagnosedMixedDesignator &&
+ !getSourceManager().isInSystemMacro(FirstDesignator)) {
Diag(FirstDesignator, getLangOpts().CPlusPlus20
----------------
rsmith wrote:
> I think we should be checking `Diags.getSuppressSystemWarnings()` before considering whether we're in a system header. (Eg, if we're building libc++ and it enables warnings even in system headers, we want to warn if it uses non-C++ designator syntax.)
>
> Given that this is something that we will presumably want to do for a bunch of diagnostics, not just this one, I think we should extend the current `ShowInSystemHeader` / `SuppressInSystemHeader` markings in our diagnostics `.td` files to also have a `SuppressInSystemMacro` level, and handle this centrally with the other "suppress warnings in system headers" logic, rather than special-casing this here.
Thanks for the quick feedback!
Fully agree, thanks for the pointers. I tried to apply a similar fix [[ https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/DiagnosticIDs.cpp#L584 | here ]] but got quite a few tests failing so I thought maybe the intention was to apply this on a case-by-case basis.
I'll see what I can do with the `.td` files, thanks!
What unit test file should I use to verify my changes?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116833/new/
https://reviews.llvm.org/D116833
More information about the cfe-commits
mailing list