jerinphilip · jerinphilip · Aug 19, 2023 · Sep 7, 2023 · Sep 7, 2023 · Aug 19, 2023
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -145,6 +145,11 @@ Attribute Changes in Clang
   supports but that are never the result of default argument promotion, such as
   ``float``. (`#59824: <https://github.com/llvm/llvm-project/issues/59824>`_)
 
+- Clang now warns you that the ``_Alignas`` attribute on declaration specifiers
+  is ignored, changed from the former incorrect suggestion to move it past
+  declaration specifiers.
+
+
 Improvements to Clang's diagnostics
 -----------------------------------
 - Clang constexpr evaluator now prints template arguments when displaying

diff --git a/clang/include/clang/Basic/AttributeCommonInfo.h b/clang/include/clang/Basic/AttributeCommonInfo.h
@@ -78,6 +78,7 @@ class AttributeCommonInfo {
   unsigned SpellingIndex : 4;
   unsigned IsAlignas : 1;
   unsigned IsRegularKeywordAttribute : 1;
+  unsigned Tok : 4;
 
 protected:
   static constexpr unsigned SpellingNotCalculated = 0xf;
@@ -91,14 +92,17 @@ class AttributeCommonInfo {
                    bool IsRegularKeywordAttribute)
         : SyntaxUsed(SyntaxUsed), SpellingIndex(SpellingIndex),
           IsAlignas(IsAlignas),
-          IsRegularKeywordAttribute(IsRegularKeywordAttribute) {}
+          IsRegularKeywordAttribute(IsRegularKeywordAttribute),
+          Tok(tok::TokenKind::unknown) {}
     constexpr Form(tok::TokenKind Tok)
         : SyntaxUsed(AS_Keyword), SpellingIndex(SpellingNotCalculated),
-          IsAlignas(Tok == tok::kw_alignas),
-          IsRegularKeywordAttribute(tok::isRegularKeywordAttribute(Tok)) {}
+          IsAlignas(Tok == tok::kw_alignas || Tok == tok::kw__Alignas),
+          IsRegularKeywordAttribute(tok::isRegularKeywordAttribute(Tok)),
+          Tok(Tok) {}
 
     Syntax getSyntax() const { return Syntax(SyntaxUsed); }
     unsigned getSpellingIndex() const { return SpellingIndex; }
+    unsigned getTok() const { return Tok; }
     bool isAlignas() const { return IsAlignas; }
     bool isRegularKeywordAttribute() const { return IsRegularKeywordAttribute; }
 
@@ -119,12 +123,14 @@ class AttributeCommonInfo {
   private:
     constexpr Form(Syntax SyntaxUsed)
         : SyntaxUsed(SyntaxUsed), SpellingIndex(SpellingNotCalculated),
-          IsAlignas(0), IsRegularKeywordAttribute(0) {}
+          IsAlignas(0), IsRegularKeywordAttribute(0),
+          Tok(tok::TokenKind::unknown) {}
 
     unsigned SyntaxUsed : 4;
     unsigned SpellingIndex : 4;
     unsigned IsAlignas : 1;
     unsigned IsRegularKeywordAttribute : 1;
+    unsigned Tok : 4;
   };
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
@@ -135,7 +141,8 @@ class AttributeCommonInfo {
         SyntaxUsed(FormUsed.getSyntax()),
         SpellingIndex(FormUsed.getSpellingIndex()),
         IsAlignas(FormUsed.isAlignas()),
-        IsRegularKeywordAttribute(FormUsed.isRegularKeywordAttribute()) {
+        IsRegularKeywordAttribute(FormUsed.isRegularKeywordAttribute()),
+        Tok(FormUsed.getTok()) {
     assert(SyntaxUsed >= AS_GNU && SyntaxUsed <= AS_Implicit &&
            "Invalid syntax!");
   }
@@ -182,18 +189,37 @@ class AttributeCommonInfo {
 
   bool isDeclspecAttribute() const { return SyntaxUsed == AS_Declspec; }
   bool isMicrosoftAttribute() const { return SyntaxUsed == AS_Microsoft; }
+  bool isCXX11Attribute() const { return SyntaxUsed == AS_CXX11; }
+  bool isC23Attribute() const { return SyntaxUsed == AS_C23; }
 
   bool isGNUScope() const;
   bool isClangScope() const;
 
-  bool isCXX11Attribute() const { return SyntaxUsed == AS_CXX11 || IsAlignas; }
+  bool isAlignas() const { return IsAlignas; }
 
-  bool isC23Attribute() const { return SyntaxUsed == AS_C23; }
+  /// Can be treated attribute-specifier in C++
+  ///    [[]] || alignas()
+  bool isCXX11AttributeSpecifier() const {
+    return SyntaxUsed == AS_CXX11 ||
+           (IsAlignas && Tok != tok::TokenKind::kw__Alignas);
+  }
+
+  /// Can be treated attribute-specifier in C
+  ///    [[]] || alignas(...) || _Alignas(...)
+  ///
+  /// Note: `attribute-specifier` and `alignment-specifier` are different per
+  /// C23-standard, but is treated similarly by this function.
+  bool isCAttributeSpecifier() const {
+    bool isCAlignAs = (Tok == tok::TokenKind::kw_alignas ||
+                       Tok == tok::TokenKind::kw__Alignas);
+    return SyntaxUsed == AS_C23 || isCAlignAs;
+  }
 
-  /// The attribute is spelled [[]] in either C or C++ mode, including standard
-  /// attributes spelled with a keyword, like alignas.
+  /// Can be treated attribute-specifier either of
+  ///   * C :   [[]] || alignas(...) || _Alignas(...)
+  ///   * C++:  [[]] || alignas(...)
   bool isStandardAttributeSyntax() const {
-    return isCXX11Attribute() || isC23Attribute();
+    return isCXX11AttributeSpecifier() || isCAttributeSpecifier();
   }
 
   bool isGNUAttribute() const { return SyntaxUsed == AS_GNU; }

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3491,10 +3491,14 @@ def err_attribute_invalid_on_decl : Error<
 def warn_type_attribute_deprecated_on_decl : Warning<
   "applying attribute %0 to a declaration is deprecated; apply it to the type instead">,
   InGroup<DeprecatedAttributes>;
-def warn_declspec_attribute_ignored : Warning<
+def warn_declspec_attribute_ignored_place_after : Warning<
   "attribute %0 is ignored, place it after "
   "\"%select{class|struct|interface|union|enum|enum class|enum struct}1\" to apply attribute to "
   "type declaration">, InGroup<IgnoredAttributes>;
+def warn_declspec_attribute_ignored : Warning<
+  "attribute %0 before "
+  "\"%select{class|struct|interface|union|enum|enum class|enum struct}1\" "
+  "is ignored">, InGroup<IgnoredAttributes>;
 def err_declspec_keyword_has_no_effect : Error<
   "%0 cannot appear here, place it after "
   "\"%select{class|struct|interface|union|enum}1\" to apply it to the "

diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h
@@ -37,6 +37,8 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 
+#include <csignal>
+
 namespace clang {
   class ASTContext;
   class CXXRecordDecl;
@@ -1970,11 +1972,20 @@ class Declarator {
         DeclarationAttrs(DeclarationAttrs), AsmLabel(nullptr),
         TrailingRequiresClause(nullptr),
         InventedTemplateParameterList(nullptr) {
-    assert(llvm::all_of(DeclarationAttrs,
-                        [](const ParsedAttr &AL) {
-                          return (AL.isStandardAttributeSyntax() ||
-                                  AL.isRegularKeywordAttribute());
-                        }) &&
+    if (!(DeclarationAttrs.empty() ||
+          llvm::all_of(DeclarationAttrs, [](const ParsedAttr &AL) {
+            return (AL.isStandardAttributeSyntax() ||
+                    AL.isRegularKeywordAttribute());
+          }))) {
+
+      std::raise(SIGTRAP);
+    }
+    assert((DeclarationAttrs.empty() ||
+            llvm::all_of(DeclarationAttrs,
+                         [](const ParsedAttr &AL) {
+                           return (AL.isStandardAttributeSyntax() ||
+                                   AL.isRegularKeywordAttribute());
+                         })) &&
            "DeclarationAttrs may only contain [[]] and keyword attributes");
   }
 

diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -767,7 +767,8 @@ void Parser::ParseLexedAttribute(LateParsedAttribute &LA,
     Diag(Tok, diag::warn_attribute_no_decl) << LA.AttrName.getName();
   }
 
-  if (OnDefinition && !Attrs.empty() && !Attrs.begin()->isCXX11Attribute() &&
+  if (OnDefinition && !Attrs.empty() &&
+      !Attrs.begin()->isCXX11AttributeSpecifier() &&
       Attrs.begin()->isKnownToGCC())
     Diag(Tok, diag::warn_attribute_on_function_definition)
       << &LA.AttrName;

diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
@@ -3405,7 +3405,7 @@ void Parser::ParseDeclarationSpecifiers(
       else {
         // Reject C++11 / C23 attributes that aren't type attributes.
         for (const ParsedAttr &PA : attrs) {
-          if (!PA.isCXX11Attribute() && !PA.isC23Attribute() &&
+          if (!PA.isStandardAttributeSyntax() &&
               !PA.isRegularKeywordAttribute())
             continue;
           if (PA.getKind() == ParsedAttr::UnknownAttribute)

diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -2562,7 +2562,7 @@ bool Parser::ParseCXXMemberDeclaratorBeforeInitializer(
       // If we saw any GNU-style attributes that are known to GCC followed by a
       // virt-specifier, issue a GCC-compat warning.
       for (const ParsedAttr &AL : DeclaratorInfo.getAttributes())
-        if (AL.isKnownToGCC() && !AL.isCXX11Attribute())
+        if (AL.isKnownToGCC() && !AL.isCXX11AttributeSpecifier())
           Diag(AL.getLoc(), diag::warn_gcc_attribute_location);
 
       MaybeParseAndDiagnoseDeclSpecAfterCXX11VirtSpecifierSeq(DeclaratorInfo,
@@ -3057,7 +3057,7 @@ Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS,
       // Diagnose attributes that appear in a friend member function declarator:
       //   friend int foo [[]] ();
       for (const ParsedAttr &AL : DeclaratorInfo.getAttributes())
-        if (AL.isCXX11Attribute() || AL.isRegularKeywordAttribute()) {
+        if (AL.isCXX11AttributeSpecifier() || AL.isRegularKeywordAttribute()) {
           auto Loc = AL.getRange().getBegin();
           (AL.isRegularKeywordAttribute()
                ? Diag(Loc, diag::err_keyword_not_allowed) << AL

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
@@ -5339,16 +5339,24 @@ Decl *Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS,
         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)
+
+      auto EmitAttributeDiagnostic = [this, &DS](const ParsedAttr &AL) {
+        unsigned DiagnosticId;
+        if (AL.isAlignas() && !getLangOpts().CPlusPlus) {
+          // Don't use the message with placement with _Alignas.
+          // This is because C doesnt let you use _Alignas on type declarations.
+          DiagnosticId = diag::warn_declspec_attribute_ignored;
+        } else if (AL.isRegularKeywordAttribute()) {
+          DiagnosticId = diag::err_declspec_keyword_has_no_effect;
+        } else {
+          DiagnosticId = diag::warn_declspec_attribute_ignored_place_after;
+        }
+        Diag(AL.getLoc(), DiagnosticId)
             << AL << GetDiagnosticTypeSpecifierID(DS);
+      };
+
+      llvm::for_each(DS.getAttributes(), EmitAttributeDiagnostic);
+      llvm::for_each(DeclAttrs, EmitAttributeDiagnostic);
     }
   }
 

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -2342,7 +2342,7 @@ static void handleDependencyAttr(Sema &S, Scope *Scope, Decl *D,
 }
 
 static void handleUnusedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
-  bool IsCXX17Attr = AL.isCXX11Attribute() && !AL.getScopeName();
+  bool IsCXX17Attr = AL.isCXX11AttributeSpecifier() && !AL.getScopeName();
 
   // If this is spelled as the standard C++17 attribute, but not in C++17, warn
   // about using it as an extension.
@@ -8178,7 +8178,8 @@ static void handleDeprecatedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
            !S.checkStringLiteralArgumentAttr(AL, 1, Replacement))
     return;
 
-  if (!S.getLangOpts().CPlusPlus14 && AL.isCXX11Attribute() && !AL.isGNUScope())
+  if (!S.getLangOpts().CPlusPlus14 && AL.isCXX11AttributeSpecifier() &&
+      !AL.isGNUScope())
     S.Diag(AL.getLoc(), diag::ext_cxx14_attr) << AL;
 
   D->addAttr(::new (S.Context) DeprecatedAttr(S.Context, AL, Str, Replacement));

diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -43,7 +43,7 @@ static Attr *handleFallThroughAttr(Sema &S, Stmt *St, const ParsedAttr &A,
 
   // If this is spelled as the standard C++17 attribute, but not in C++17, warn
   // about using it as an extension.
-  if (!S.getLangOpts().CPlusPlus17 && A.isCXX11Attribute() &&
+  if (!S.getLangOpts().CPlusPlus17 && A.isCXX11AttributeSpecifier() &&
       !A.getScopeName())
     S.Diag(A.getLoc(), diag::ext_cxx17_attr) << A;
 
@@ -307,7 +307,8 @@ static Attr *handleMustTailAttr(Sema &S, Stmt *St, const ParsedAttr &A,
 static Attr *handleLikely(Sema &S, Stmt *St, const ParsedAttr &A,
                           SourceRange Range) {
 
-  if (!S.getLangOpts().CPlusPlus20 && A.isCXX11Attribute() && !A.getScopeName())
+  if (!S.getLangOpts().CPlusPlus20 && A.isCXX11AttributeSpecifier() &&
+      !A.getScopeName())
     S.Diag(A.getLoc(), diag::ext_cxx20_attr) << A << Range;
 
   return ::new (S.Context) LikelyAttr(S.Context, A);
@@ -316,7 +317,8 @@ static Attr *handleLikely(Sema &S, Stmt *St, const ParsedAttr &A,
 static Attr *handleUnlikely(Sema &S, Stmt *St, const ParsedAttr &A,
                             SourceRange Range) {
 
-  if (!S.getLangOpts().CPlusPlus20 && A.isCXX11Attribute() && !A.getScopeName())
+  if (!S.getLangOpts().CPlusPlus20 && A.isCXX11AttributeSpecifier() &&
+      !A.getScopeName())
     S.Diag(A.getLoc(), diag::ext_cxx20_attr) << A << Range;
 
   return ::new (S.Context) UnlikelyAttr(S.Context, A);

diff --git a/clang/test/C/drs/dr4xx.c b/clang/test/C/drs/dr4xx.c
@@ -168,7 +168,7 @@ void dr444(void) {
   * 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 {{attribute '_Alignas' before "struct" is ignored}} */
     int i;
   };
 

diff --git a/clang/test/Parser/c1x-alignas.c b/clang/test/Parser/c1x-alignas.c
@@ -9,5 +9,6 @@ char c4 _Alignas(32); // expected-error {{expected ';' after top level declarato
 
 char _Alignas(_Alignof(int)) c5;
 
+_Alignas(int) struct c6; // expected-warning {{attribute '_Alignas' before "struct" is ignored}}
 // CHECK-EXT: '_Alignas' is a C11 extension
 // CHECK-EXT: '_Alignof' is a C11 extension
diff --git a/clang/test/Parser/c2x-alignas.c b/clang/test/Parser/c2x-alignas.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=c23 -fsyntax-only -verify %s
+
+_Alignas(int) struct c1; // expected-warning {{attribute '_Alignas' before "struct" is ignored}}
+
+// FIXME: This is meant to behave the same as _Alignas, as in C23, `alignas` is a
+// keyword and `_Alignas` is an alternate spelling.
+// However, `alignas` is a keyword in C++ since CXX11, and is baked into
+// parsing code, will need a more holistic fix to treat it also as C23
+// `alignas`. 
+alignas(int) struct c2; // expected-error {{misplaced attributes; expected attributes here}}
diff --git a/clang/test/Parser/cxx0x-attributes.cpp b/clang/test/Parser/cxx0x-attributes.cpp
@@ -451,3 +451,5 @@ namespace P2361 {
                                  // 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}}
diff --git a/clang/test/Sema/alignas.c b/clang/test/Sema/alignas.c
@@ -17,6 +17,8 @@ void f(_Alignas(1) char c) { // expected-error {{'_Alignas' attribute cannot be
   _Alignas(1) register char k; // expected-error {{'_Alignas' attribute cannot be applied to a variable with 'register' storage class}}
 }
 
+_Alignas(int) struct  alignas_before { int content;}; // expected-warning {{attribute '_Alignas' before "struct" is ignored}}
+
 #ifdef USING_C11_SYNTAX
 // expected-warning@+4{{'_Alignof' applied to an expression is a GNU extension}}
 // expected-warning@+4{{'_Alignof' applied to an expression is a GNU extension}}