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

[theo-lw]

Summary

This changes the diagnostic for _Alignas so it no longer tells people to place _Alignas after the "struct" keyword.

Fixes https://github.com/llvm/llvm-project/issues/58637

[theo-lw]

Accidentally deleted a line causing this to not compile :(

[theo-lw]

Didn't realize this test was failing so I've updated it.

[aaron.ballman]

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.

clang/lib/Sema/SemaDecl.cpp

5296

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.

5299–5304

5312–5317

Pretty sure you can use something like this instead now.

clang/test/C/drs/dr4xx.c

167

This diagnostic is incorrect but should be fixed with the suggested edit above.

clang/test/Parser/c1x-alignas.c

12–13

Same here.

[aaron.ballman]

@theo-lw -- are you planning to continue working on this review? (It's okay if you're not, I'm more wondering if I should remove you as the assignee on https://github.com/llvm/llvm-project/issues/58637)

[theo-lw]

In D141177#4592158, @aaron.ballman wrote:

@theo-lw -- are you planning to continue working on this review? (It's okay if you're not, I'm more wondering if I should remove you as the assignee on https://github.com/llvm/llvm-project/issues/58637)

Hey! Sorry, got busy with school. I can continue working on this.

[aaron.ballman]

In D141177#4592361, @theo-lw wrote:

In D141177#4592158, @aaron.ballman wrote:

@theo-lw -- are you planning to continue working on this review? (It's okay if you're not, I'm more wondering if I should remove you as the assignee on https://github.com/llvm/llvm-project/issues/58637)

Hey! Sorry, got busy with school. I can continue working on this.

No worries, it happens! Thank you for continuing work on this -- if you find you don't have the time, it's not a big deal, just let us know. :-)

[theo-lw]

Rebased on the main branch & addressed comments.

Added additional test case for C++ alignas.

[theo-lw]

Ran clang-format on this.

[aaron.ballman]

clang/lib/Sema/SemaDecl.cpp

5337–5346

I think this might be a bit more clear:

unsigned DiagID;
if (AL.isC11AlignasAttribute()) {
  // Don't use the message with placement with _Alignas.
  // This is because C doesnt let you use _Alignas on type declarations.
  DiagID = diag::warn_attribute_ignored;
} else if (AL.isRegularKeywordAttribute())
  DiagID = diag::err_declspec_keyword_has_no_effect;
else
  DiagID = diag::warn_declspec_attribute_ignored;

Diag(AL.getLoc(), DiagID) << AL << GetDiagnosticTypeSpecifierID(DS);

clang/test/Parser/c1x-alignas.c

12

I'd like to see a C23-specific test that does:

alignas(int) struct c7;

(the test doesn't have to be in this file, but it's to validate that we correctly handle alignas when it's a keyword that aliases to _Alignas.)

clang/docs/ReleaseNotes.rst

   tautologies like ``x && !x`` and ``!x || x`` in expressions. This also
   makes ``-Winfinite-recursion`` diagnose more cases.
   (`#56035: <https://github.com/llvm/llvm-project/issues/56035>`_).
+- Clang now informs you that the ``_Alignas`` attribute on a struct is ignored, instead of
+  suggesting that it should be moved.
 
 Bug Fixes in This Version
 -------------------------

clang/include/clang/Basic/AttributeCommonInfo.h

   bool isGNUScope() const;
   bool isClangScope() const;
 
+  bool isC11AlignasAttribute() const {
+    return getParsedKind() == AT_Aligned && isKeywordAttribute() && !IsAlignas;
+  }
+
   bool isCXX11Attribute() const { return SyntaxUsed == AS_CXX11 || IsAlignas; }
 
   bool isC23Attribute() const { return SyntaxUsed == AS_C23; }

clang/lib/Sema/SemaDecl.cpp

   // __attribute__((aligned)) struct A;
   // Attributes should be placed after tag to apply to type declaration.
   if (!DS.getAttributes().empty() || !DeclAttrs.empty()) {
+    auto WarnAttributeIgnored = [this, &DS](const ParsedAttr &AL) {
+      if (AL.isC11AlignasAttribute()) {
+        // Don't use the message with placement with _Alignas.
+        // This is because C doesnt let you use _Alignas on type declarations.
+        Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL;
+      } else {
+        Diag(AL.getLoc(), AL.isRegularKeywordAttribute()
+                              ? diag::err_declspec_keyword_has_no_effect
+                              : diag::warn_declspec_attribute_ignored)
+            << AL << GetDiagnosticTypeSpecifierID(DS);
+      }
+    };
+
     DeclSpec::TST TypeSpecType = DS.getTypeSpecType();
     if (TypeSpecType == DeclSpec::TST_class ||
         TypeSpecType == DeclSpec::TST_struct ||
         TypeSpecType == DeclSpec::TST_interface ||
         TypeSpecType == DeclSpec::TST_union ||
         TypeSpecType == DeclSpec::TST_enum) {
-      for (const ParsedAttr &AL : DS.getAttributes())
-        Diag(AL.getLoc(), AL.isRegularKeywordAttribute()
-                              ? diag::err_declspec_keyword_has_no_effect
-                              : diag::warn_declspec_attribute_ignored)
-            << AL << GetDiagnosticTypeSpecifierID(DS);
-      for (const ParsedAttr &AL : DeclAttrs)
-        Diag(AL.getLoc(), AL.isRegularKeywordAttribute()
-                              ? diag::err_declspec_keyword_has_no_effect
-                              : diag::warn_declspec_attribute_ignored)
-            << AL << GetDiagnosticTypeSpecifierID(DS);
+      llvm::for_each(DS.getAttributes(), WarnAttributeIgnored);
+      llvm::for_each(DeclAttrs, WarnAttributeIgnored);
     }
   }
 

clang/test/C/drs/dr4xx.c

   /* FIXME: This should be accepted as per this DR. */
   int j = (_Alignas(int) int){12}; /* expected-error {{expected expression}} */
 
- /* 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 {{'_Alignas' attribute ignored}} */
     int i;
   };
 

clang/test/Parser/c1x-alignas.c

 
 char _Alignas(_Alignof(int)) c5;
 
+_Alignas(int) struct c6; // expected-warning {{'_Alignas' attribute ignored}}
+
 // CHECK-EXT: '_Alignas' is a C11 extension
 // CHECK-EXT: '_Alignof' is a C11 extension

clang/test/Parser/cxx0x-attributes.cpp

                                  // expected-warning {{use of the 'deprecated' attribute is a C++14 extension}}
 [[nodiscard("\123")]] int b(); // expected-error{{invalid escape sequence '\123' in an unevaluated string literal}}
 }
+
+alignas(int) struct AlignAsAttribute {}; // expected-error {{misplaced attributes; expected attributes here}}