[clang] Fix FIXME in isAlignasAttribute()

AttributeCommonInfo::isAlignasAttribute() was used in one place: isCXX11Attribute(). The intention was for isAlignasAttribute() to return true for the C++ alignas keyword. However, as a FIXME noted, the function also returned true for the C _Alignas keyword. This meant that isCXX11Attribute() returned true for _Alignas as well as for alignas. AttributeCommonInfos are now always constructed with an AttributeCommonInfo::Form. We can use that Form to convey whether a keyword is alignas or not. The patch uses 1 bit of an 8-bit hole in the current layout of AttributeCommonInfo. This might not be the best long-term design, but it should be easy to adapt the layout if necessary (that is, if other uses are found for the spare bits). I don't know of a way of testing this (other than grep -c FIXME) Differential Revision: https://reviews.llvm.org/D148105
llvm · Apr 13, 2023 · bd41371 · bd41371
1 parent aec3f95
commit bd41371
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 39 deletions.
diff --git a/clang/include/clang/Basic/AttributeCommonInfo.h b/clang/include/clang/Basic/AttributeCommonInfo.h
@@ -76,6 +76,7 @@ class AttributeCommonInfo {
   /// Corresponds to the Syntax enum.
   unsigned SyntaxUsed : 4;
   unsigned SpellingIndex : 4;
+  unsigned IsAlignas : 1;
 
 protected:
   static constexpr unsigned SpellingNotCalculated = 0xf;
@@ -85,31 +86,38 @@ class AttributeCommonInfo {
   /// including its syntax and spelling.
   class Form {
   public:
-    constexpr Form(Syntax SyntaxUsed, unsigned SpellingIndex)
-        : SyntaxUsed(SyntaxUsed), SpellingIndex(SpellingIndex) {}
-    constexpr Form(tok::TokenKind)
-        : SyntaxUsed(AS_Keyword), SpellingIndex(SpellingNotCalculated) {}
+    constexpr Form(Syntax SyntaxUsed, unsigned SpellingIndex, bool IsAlignas)
+        : SyntaxUsed(SyntaxUsed), SpellingIndex(SpellingIndex),
+          IsAlignas(IsAlignas) {}
+    constexpr Form(tok::TokenKind Tok)
+        : SyntaxUsed(AS_Keyword), SpellingIndex(SpellingNotCalculated),
+          IsAlignas(Tok == tok::kw_alignas) {}
 
     Syntax getSyntax() const { return Syntax(SyntaxUsed); }
     unsigned getSpellingIndex() const { return SpellingIndex; }
+    bool isAlignas() const { return IsAlignas; }
 
     static Form GNU() { return AS_GNU; }
     static Form CXX11() { return AS_CXX11; }
     static Form C2x() { return AS_C2x; }
     static Form Declspec() { return AS_Declspec; }
     static Form Microsoft() { return AS_Microsoft; }
-    static Form Keyword() { return AS_Keyword; }
+    static Form Keyword(bool IsAlignas) {
+      return Form(AS_Keyword, SpellingNotCalculated, IsAlignas);
+    }
     static Form Pragma() { return AS_Pragma; }
     static Form ContextSensitiveKeyword() { return AS_ContextSensitiveKeyword; }
     static Form HLSLSemantic() { return AS_HLSLSemantic; }
     static Form Implicit() { return AS_Implicit; }
 
   private:
     constexpr Form(Syntax SyntaxUsed)
-        : SyntaxUsed(SyntaxUsed), SpellingIndex(SpellingNotCalculated) {}
+        : SyntaxUsed(SyntaxUsed), SpellingIndex(SpellingNotCalculated),
+          IsAlignas(0) {}
 
     unsigned SyntaxUsed : 4;
     unsigned SpellingIndex : 4;
+    unsigned IsAlignas : 1;
   };
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
@@ -119,35 +127,39 @@ class AttributeCommonInfo {
         ScopeLoc(ScopeLoc),
         AttrKind(getParsedKind(AttrName, ScopeName, FormUsed.getSyntax())),
         SyntaxUsed(FormUsed.getSyntax()),
-        SpellingIndex(FormUsed.getSpellingIndex()) {}
+        SpellingIndex(FormUsed.getSpellingIndex()),
+        IsAlignas(FormUsed.isAlignas()) {}
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,
                       const IdentifierInfo *ScopeName, SourceRange AttrRange,
                       SourceLocation ScopeLoc, Kind AttrKind, Form FormUsed)
       : AttrName(AttrName), ScopeName(ScopeName), AttrRange(AttrRange),
         ScopeLoc(ScopeLoc), AttrKind(AttrKind),
         SyntaxUsed(FormUsed.getSyntax()),
-        SpellingIndex(FormUsed.getSpellingIndex()) {}
+        SpellingIndex(FormUsed.getSpellingIndex()),
+        IsAlignas(FormUsed.isAlignas()) {}
 
   AttributeCommonInfo(const IdentifierInfo *AttrName, SourceRange AttrRange,
                       Form FormUsed)
       : AttrName(AttrName), ScopeName(nullptr), AttrRange(AttrRange),
         ScopeLoc(),
         AttrKind(getParsedKind(AttrName, ScopeName, FormUsed.getSyntax())),
         SyntaxUsed(FormUsed.getSyntax()),
-        SpellingIndex(FormUsed.getSpellingIndex()) {}
+        SpellingIndex(FormUsed.getSpellingIndex()),
+        IsAlignas(FormUsed.isAlignas()) {}
 
   AttributeCommonInfo(SourceRange AttrRange, Kind K, Form FormUsed)
       : AttrName(nullptr), ScopeName(nullptr), AttrRange(AttrRange), ScopeLoc(),
         AttrKind(K), SyntaxUsed(FormUsed.getSyntax()),
-        SpellingIndex(FormUsed.getSpellingIndex()) {}
+        SpellingIndex(FormUsed.getSpellingIndex()),
+        IsAlignas(FormUsed.isAlignas()) {}
 
   AttributeCommonInfo(AttributeCommonInfo &&) = default;
   AttributeCommonInfo(const AttributeCommonInfo &) = default;
 
   Kind getParsedKind() const { return Kind(AttrKind); }
   Syntax getSyntax() const { return Syntax(SyntaxUsed); }
-  Form getForm() const { return Form(getSyntax(), SpellingIndex); }
+  Form getForm() const { return Form(getSyntax(), SpellingIndex, IsAlignas); }
   const IdentifierInfo *getAttrName() const { return AttrName; }
   SourceLocation getLoc() const { return AttrRange.getBegin(); }
   SourceRange getRange() const { return AttrRange; }
@@ -168,29 +180,7 @@ class AttributeCommonInfo {
   bool isGNUScope() const;
   bool isClangScope() const;
 
-  bool isAlignasAttribute() const {
-    // FIXME: Use a better mechanism to determine this.
-    // We use this in `isCXX11Attribute` below, so it _should_ only return
-    // true for the `alignas` spelling, but it currently also returns true
-    // for the `_Alignas` spelling, which only exists in C11. Distinguishing
-    // between the two is important because they behave differently:
-    // - `alignas` may only appear in the attribute-specifier-seq before
-    //   the decl-specifier-seq and is therefore associated with the
-    //   declaration.
-    // - `_Alignas` may appear anywhere within the declaration-specifiers
-    //   and is therefore associated with the `DeclSpec`.
-    // It's not clear how best to fix this:
-    // - We have the necessary information in the form of the `SpellingIndex`,
-    //   but we would need to compare against AlignedAttr::Keyword_alignas,
-    //   and we can't depend on clang/AST/Attr.h here.
-    // - We could test `getAttrName()->getName() == "alignas"`, but this is
-    //   inefficient.
-    return getParsedKind() == AT_Aligned && isKeywordAttribute();
-  }
-
-  bool isCXX11Attribute() const {
-    return SyntaxUsed == AS_CXX11 || isAlignasAttribute();
-  }
+  bool isCXX11Attribute() const { return SyntaxUsed == AS_CXX11 || IsAlignas; }
 
   bool isC2xAttribute() const { return SyntaxUsed == AS_C2x; }
 

diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
@@ -4893,9 +4893,9 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
 
     // If we're supposed to infer nullability, do so now.
     if (inferNullability && !inferNullabilityInnerOnlyComplete) {
-      ParsedAttr::Form form = inferNullabilityCS
-                                  ? ParsedAttr::Form::ContextSensitiveKeyword()
-                                  : ParsedAttr::Form::Keyword();
+      ParsedAttr::Form form =
+          inferNullabilityCS ? ParsedAttr::Form::ContextSensitiveKeyword()
+                             : ParsedAttr::Form::Keyword(false /*IsAlignAs*/);
       ParsedAttr *nullabilityAttr = Pool.create(
           S.getNullabilityKeyword(*inferNullability), SourceRange(pointerLoc),
           nullptr, SourceLocation(), nullptr, 0, form);

diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3091,11 +3091,14 @@ Attr *ASTRecordReader::readAttr() {
   unsigned ParsedKind = Record.readInt();
   unsigned Syntax = Record.readInt();
   unsigned SpellingIndex = Record.readInt();
+  bool IsAlignas = (ParsedKind == AttributeCommonInfo::AT_Aligned &&
+                    Syntax == AttributeCommonInfo::AS_Keyword &&
+                    SpellingIndex == AlignedAttr::Keyword_alignas);
 
   AttributeCommonInfo Info(
       AttrName, ScopeName, AttrRange, ScopeLoc,
       AttributeCommonInfo::Kind(ParsedKind),
-      {AttributeCommonInfo::Syntax(Syntax), SpellingIndex});
+      {AttributeCommonInfo::Syntax(Syntax), SpellingIndex, IsAlignas});
 
 #include "clang/Serialization/AttrPCHRead.inc"
 

diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -2381,8 +2381,11 @@ static void emitClangAttrAcceptsExprPack(RecordKeeper &Records,
 static void emitFormInitializer(raw_ostream &OS,
                                 const FlattenedSpelling &Spelling,
                                 StringRef SpellingIndex) {
+  bool IsAlignas =
+      (Spelling.variety() == "Keyword" && Spelling.name() == "alignas");
   OS << "{AttributeCommonInfo::AS_" << Spelling.variety() << ", "
-     << SpellingIndex << "}";
+     << SpellingIndex << ", " << (IsAlignas ? "true" : "false")
+     << " /*IsAlignas*/}";
 }
 
 static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,