aaron.ballman added a comment.
Thank you for the fix, please be sure to add a release note for the fix as
well. Also, I'd like to see some additional C++ test coverage for:
`alignas(int) struct S {};` to demonstrate we still suggest moving the keyword
in that case.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:5296
+ auto WarnAttributeIgnored = [this, TypeSpecType](const ParsedAttr &AL) {
+ if (AL.isAlignasAttribute()) {
+ // Don't use the message with placement with _Alignas.
----------------
I think this isn't quite right -- it'll return true in C++ for `alignas`:
`alignas(int) struct S {};` will give the wrong diagnostic now, I suspect. You
need to check the language mode as well to ensure we're not in C++ mode.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:5299-5304
+ Diag(AL.getLoc(), diag::warn_attribute_ignored)
+ << GetDiagnosticTypeSpecifierID(TypeSpecType);
+ } else {
+ Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
+ << AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+ }
----------------
================
Comment at: clang/lib/Sema/SemaDecl.cpp:5312-5317
+ for (const ParsedAttr &AL : DS.getAttributes()) {
+ WarnAttributeIgnored(AL);
+ }
+ for (const ParsedAttr &AL : DeclAttrs) {
+ WarnAttributeIgnored(AL);
+ }
----------------
Pretty sure you can use something like this instead now.
================
Comment at: clang/test/C/drs/dr4xx.c:167
- /* FIXME: The diagnostic in this case is really bad; moving the specifier to
- * where the diagnostic recommends causes a different, more inscrutable error
- * about anonymous structures.
- */
- _Alignas(int) struct T { /* expected-warning {{attribute '_Alignas' is
ignored, place it after "struct" to apply attribute to type declaration}} */
+ _Alignas(int) struct T { /* expected-warning {{1 attribute ignored}} */
int i;
----------------
This diagnostic is incorrect but should be fixed with the suggested edit above.
================
Comment at: clang/test/Parser/c1x-alignas.c:12-13
+_Alignas(int) struct c6; // expected-warning {{1 attribute ignored}}
+
// CHECK-EXT: '_Alignas' is a C11 extension
----------------
Same here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141177/new/
https://reviews.llvm.org/D141177
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
