[PATCH] D141177: [Clang] Don't tell people to place _Alignas on a struct in diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

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