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