[clang] e841d50 - [clang] Ensure that Attr::Create(Implicit) chooses a valid syntax

Richard Sandiford via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 13 02:15:57 PDT 2023


Author: Richard Sandiford
Date: 2023-04-13T10:14:48+01:00
New Revision: e841d50926347c3596009dd4fbc4cbd1a1a2e0b8

URL: https://github.com/llvm/llvm-project/commit/e841d50926347c3596009dd4fbc4cbd1a1a2e0b8
DIFF: https://github.com/llvm/llvm-project/commit/e841d50926347c3596009dd4fbc4cbd1a1a2e0b8.diff

LOG: [clang] Ensure that Attr::Create(Implicit) chooses a valid syntax

The purpose of this patch and follow-on patches is to ensure that
AttributeCommonInfos always have a syntax that is appropriate for
their kind (i.e. that it matches one of the entries in Attr.td).

The attribute-specific Create and CreateImplicit methods had four
overloads, based on their tail arguments:

(1) no extra arguments
(2) an AttributeCommonInfo
(3) a SourceRange
(4) a SourceRange, a syntax, and (where necessary) a spelling

When (4) had a spelling argument, it defaulted to
SpellingNotCalculated.

One disadvantage of this was that (1) and (3) zero-initialized
the syntax field of the AttributeCommonInfo, which corresponds
to AS_GNU.  But AS_GNU isn't always listed as a possibility
in Attr.td.

This patch therefore removes (1) and (3) and instead provides
the same functionality using default arguments on (4) (a bit
like the existing default argument for the spelling).
The default syntax is taken from the attribute's first valid
spelling.

Doing that raises the question: what should happen for attributes
like AlignNatural and CUDAInvalidTarget that are only ever created
implicitly, and so have no source-code manifestation at all?
The patch adds a new AS_Implicit "syntax" for that case.
The patch also removes the syntax argument for these attributes,
since the syntax must always be AS_Implicit.

For similar reasons, the patch removes the syntax argument if
there is exactly one valid spelling.

Doing this means that AttributeCommonInfo no longer needs the
single-argument constructors.  It is always given a syntax instead.

Differential Revision: https://reviews.llvm.org/D148101

Added: 
    

Modified: 
    clang/include/clang/Basic/Attr.td
    clang/include/clang/Basic/AttributeCommonInfo.h
    clang/lib/Sema/SemaDecl.cpp
    clang/lib/Sema/SemaDeclCXX.cpp
    clang/lib/Serialization/ASTReaderDecl.cpp
    clang/unittests/AST/DeclTest.cpp
    clang/utils/TableGen/ClangAttrEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 79da53aba39ff..a23dd5ce3634b 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -834,7 +834,7 @@ def Annotate : InheritableParamAttr {
     return AnnotateAttr::Create(Ctx, Annotation, nullptr, 0, CommonInfo);
   }
   static AnnotateAttr *CreateImplicit(ASTContext &Ctx, llvm::StringRef Annotation, \
-              const AttributeCommonInfo &CommonInfo = AttributeCommonInfo{SourceRange{}}) {
+              const AttributeCommonInfo &CommonInfo) {
     return AnnotateAttr::CreateImplicit(Ctx, Annotation, nullptr, 0, CommonInfo);
   }
   }];

diff  --git a/clang/include/clang/Basic/AttributeCommonInfo.h b/clang/include/clang/Basic/AttributeCommonInfo.h
index f89bdbdaa0d28..f38f9687124b1 100644
--- a/clang/include/clang/Basic/AttributeCommonInfo.h
+++ b/clang/include/clang/Basic/AttributeCommonInfo.h
@@ -51,6 +51,10 @@ class AttributeCommonInfo {
 
     /// <vardecl> : <semantic>
     AS_HLSLSemantic,
+
+    /// The attibute has no source code manifestation and is only created
+    /// implicitly.
+    AS_Implicit
   };
   enum Kind {
 #define PARSED_ATTR(NAME) AT_##NAME,
@@ -76,14 +80,6 @@ class AttributeCommonInfo {
   static constexpr unsigned SpellingNotCalculated = 0xf;
 
 public:
-  explicit AttributeCommonInfo(SourceRange AttrRange)
-      : AttrRange(AttrRange), ScopeLoc(), AttrKind(0), SyntaxUsed(0),
-        SpellingIndex(SpellingNotCalculated) {}
-
-  explicit AttributeCommonInfo(SourceLocation AttrLoc)
-      : AttrRange(AttrLoc), ScopeLoc(), AttrKind(0), SyntaxUsed(0),
-        SpellingIndex(SpellingNotCalculated) {}
-
   AttributeCommonInfo(const IdentifierInfo *AttrName,
                       const IdentifierInfo *ScopeName, SourceRange AttrRange,
                       SourceLocation ScopeLoc, Syntax SyntaxUsed)

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 6c6bf78ffc9c8..409c71662ee30 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -10125,9 +10125,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
   NewFD->setParams(Params);
 
   if (D.getDeclSpec().isNoreturnSpecified())
-    NewFD->addAttr(C11NoReturnAttr::Create(Context,
-                                           D.getDeclSpec().getNoreturnSpecLoc(),
-                                           AttributeCommonInfo::AS_Keyword));
+    NewFD->addAttr(
+        C11NoReturnAttr::Create(Context, D.getDeclSpec().getNoreturnSpecLoc()));
 
   // Functions returning a variably modified type violate C99 6.7.5.2p2
   // because all functions have linkage.
@@ -10142,7 +10141,7 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
       !NewFD->hasAttr<SectionAttr>())
     NewFD->addAttr(PragmaClangTextSectionAttr::CreateImplicit(
         Context, PragmaClangTextSection.SectionName,
-        PragmaClangTextSection.PragmaLocation, AttributeCommonInfo::AS_Pragma));
+        PragmaClangTextSection.PragmaLocation));
 
   // Apply an implicit SectionAttr if #pragma code_seg is active.
   if (CodeSegStack.CurrentValue && D.isFunctionDefinition() &&
@@ -10163,8 +10162,7 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
   if (StrictGuardStackCheckStack.CurrentValue && D.isFunctionDefinition() &&
       !NewFD->hasAttr<StrictGuardStackCheckAttr>())
     NewFD->addAttr(StrictGuardStackCheckAttr::CreateImplicit(
-        Context, PragmaClangTextSection.PragmaLocation,
-        AttributeCommonInfo::AS_Pragma));
+        Context, PragmaClangTextSection.PragmaLocation));
 
   // Apply an implicit CodeSegAttr from class declspec or
   // apply an implicit SectionAttr from #pragma code_seg if active.
@@ -14210,8 +14208,7 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
     // attribute.
     if (CurInitSeg && var->getInit())
       var->addAttr(InitSegAttr::CreateImplicit(Context, CurInitSeg->getString(),
-                                               CurInitSegLoc,
-                                               AttributeCommonInfo::AS_Pragma));
+                                               CurInitSegLoc));
   }
 
   // All the following checks are C++ only.
@@ -14313,23 +14310,19 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
     if (PragmaClangBSSSection.Valid)
       VD->addAttr(PragmaClangBSSSectionAttr::CreateImplicit(
           Context, PragmaClangBSSSection.SectionName,
-          PragmaClangBSSSection.PragmaLocation,
-          AttributeCommonInfo::AS_Pragma));
+          PragmaClangBSSSection.PragmaLocation));
     if (PragmaClangDataSection.Valid)
       VD->addAttr(PragmaClangDataSectionAttr::CreateImplicit(
           Context, PragmaClangDataSection.SectionName,
-          PragmaClangDataSection.PragmaLocation,
-          AttributeCommonInfo::AS_Pragma));
+          PragmaClangDataSection.PragmaLocation));
     if (PragmaClangRodataSection.Valid)
       VD->addAttr(PragmaClangRodataSectionAttr::CreateImplicit(
           Context, PragmaClangRodataSection.SectionName,
-          PragmaClangRodataSection.PragmaLocation,
-          AttributeCommonInfo::AS_Pragma));
+          PragmaClangRodataSection.PragmaLocation));
     if (PragmaClangRelroSection.Valid)
       VD->addAttr(PragmaClangRelroSectionAttr::CreateImplicit(
           Context, PragmaClangRelroSection.SectionName,
-          PragmaClangRelroSection.PragmaLocation,
-          AttributeCommonInfo::AS_Pragma));
+          PragmaClangRelroSection.PragmaLocation));
   }
 
   if (auto *DD = dyn_cast<DecompositionDecl>(ThisDecl)) {

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index eb6a4f3296e15..45806261f2ffc 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -3567,8 +3567,7 @@ Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D,
   }
 
   if (VS.isOverrideSpecified())
-    Member->addAttr(OverrideAttr::Create(Context, VS.getOverrideLoc(),
-                                         AttributeCommonInfo::AS_Keyword));
+    Member->addAttr(OverrideAttr::Create(Context, VS.getOverrideLoc()));
   if (VS.isFinalSpecified())
     Member->addAttr(FinalAttr::Create(
         Context, VS.getFinalLoc(), AttributeCommonInfo::AS_Keyword,

diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index ee100824d65d1..aa3b21c65eb63 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4617,9 +4617,8 @@ void ASTDeclReader::UpdateDecl(Decl *D,
       break;
 
     case UPD_DECL_MARKED_OPENMP_THREADPRIVATE:
-      D->addAttr(OMPThreadPrivateDeclAttr::CreateImplicit(
-          Reader.getContext(), readSourceRange(),
-          AttributeCommonInfo::AS_Pragma));
+      D->addAttr(OMPThreadPrivateDeclAttr::CreateImplicit(Reader.getContext(),
+                                                          readSourceRange()));
       break;
 
     case UPD_DECL_MARKED_OPENMP_ALLOCATE: {
@@ -4629,8 +4628,7 @@ void ASTDeclReader::UpdateDecl(Decl *D,
       Expr *Alignment = Record.readExpr();
       SourceRange SR = readSourceRange();
       D->addAttr(OMPAllocateDeclAttr::CreateImplicit(
-          Reader.getContext(), AllocatorKind, Allocator, Alignment, SR,
-          AttributeCommonInfo::AS_Pragma));
+          Reader.getContext(), AllocatorKind, Allocator, Alignment, SR));
       break;
     }
 
@@ -4651,7 +4649,7 @@ void ASTDeclReader::UpdateDecl(Decl *D,
       unsigned Level = Record.readInt();
       D->addAttr(OMPDeclareTargetDeclAttr::CreateImplicit(
           Reader.getContext(), MapType, DevType, IndirectE, Indirect, Level,
-          readSourceRange(), AttributeCommonInfo::AS_Pragma));
+          readSourceRange()));
       break;
     }
 

diff  --git a/clang/unittests/AST/DeclTest.cpp b/clang/unittests/AST/DeclTest.cpp
index ad89596328edb..0cc7f93153f43 100644
--- a/clang/unittests/AST/DeclTest.cpp
+++ b/clang/unittests/AST/DeclTest.cpp
@@ -88,12 +88,8 @@ TEST(Decl, AsmLabelAttr) {
   NamedDecl *DeclG = *(++DeclS->method_begin());
 
   // Attach asm labels to the decls: one literal, and one not.
-  DeclF->addAttr(::new (Ctx) AsmLabelAttr(
-      Ctx, AttributeCommonInfo{SourceLocation()}, "foo",
-      /*LiteralLabel=*/true));
-  DeclG->addAttr(::new (Ctx) AsmLabelAttr(
-      Ctx, AttributeCommonInfo{SourceLocation()}, "goo",
-      /*LiteralLabel=*/false));
+  DeclF->addAttr(AsmLabelAttr::Create(Ctx, "foo", /*LiteralLabel=*/true));
+  DeclG->addAttr(AsmLabelAttr::Create(Ctx, "goo", /*LiteralLabel=*/false));
 
   // Mangle the decl names.
   std::string MangleF, MangleG;

diff  --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 84cbf60ad05d2..cb9f582aa78fa 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -2505,15 +2505,8 @@ static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
           return &R == P.second;
         });
 
-    enum class CreateKind {
-      WithAttributeCommonInfo,
-      WithSourceRange,
-      WithNoArgs,
-    };
-
     // Emit CreateImplicit factory methods.
-    auto emitCreate = [&](bool Implicit, bool DelayedArgsOnly,
-                          bool emitFake, CreateKind Kind) {
+    auto emitCreate = [&](bool Implicit, bool DelayedArgsOnly, bool emitFake) {
       if (Header)
         OS << "  static ";
       OS << R.getName() << "Attr *";
@@ -2537,10 +2530,7 @@ static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
         OS << ", ";
         DelayedArgs->writeCtorParameters(OS);
       }
-      if (Kind == CreateKind::WithAttributeCommonInfo)
-        OS << ", const AttributeCommonInfo &CommonInfo";
-      else if (Kind == CreateKind::WithSourceRange)
-        OS << ", SourceRange R";
+      OS << ", const AttributeCommonInfo &CommonInfo";
       OS << ")";
       if (Header) {
         OS << ";\n";
@@ -2549,12 +2539,7 @@ static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
 
       OS << " {\n";
       OS << "  auto *A = new (Ctx) " << R.getName();
-      if (Kind == CreateKind::WithAttributeCommonInfo)
-        OS << "Attr(Ctx, CommonInfo";
-      else if (Kind == CreateKind::WithSourceRange)
-        OS << "Attr(Ctx, AttributeCommonInfo{R}";
-      else if (Kind == CreateKind::WithNoArgs)
-        OS << "Attr(Ctx, AttributeCommonInfo{SourceLocation{}}";
+      OS << "Attr(Ctx, CommonInfo";
 
       if (!DelayedArgsOnly) {
         for (auto const &ai : Args) {
@@ -2606,7 +2591,14 @@ static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
         OS << ", ";
         DelayedArgs->writeCtorParameters(OS);
       }
-      OS << ", SourceRange Range, AttributeCommonInfo::Syntax Syntax";
+      OS << ", SourceRange Range";
+      if (Header)
+        OS << " = {}";
+      if (Spellings.size() > 1) {
+        OS << ", AttributeCommonInfo::Syntax Syntax";
+        if (Header)
+          OS << " = AttributeCommonInfo::AS_" << Spellings[0].variety();
+      }
       if (!ElideSpelling) {
         OS << ", " << R.getName() << "Attr::Spelling S";
         if (Header)
@@ -2626,7 +2618,13 @@ static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
       else
         OS << "NoSemaHandlerAttribute";
 
-      OS << ", Syntax";
+      if (Spellings.size() == 0)
+        OS << ", AttributeCommonInfo::AS_Implicit";
+      else if (Spellings.size() == 1)
+        OS << ", AttributeCommonInfo::AS_" << Spellings[0].variety();
+      else
+        OS << ", Syntax";
+
       if (!ElideSpelling)
         OS << ", S";
       OS << ");\n";
@@ -2651,19 +2649,9 @@ static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
       OS << "}\n\n";
     };
 
-    auto emitBothImplicitAndNonCreates = [&](bool DelayedArgsOnly,
-                                             bool emitFake, CreateKind Kind) {
-      emitCreate(true, DelayedArgsOnly, emitFake, Kind);
-      emitCreate(false, DelayedArgsOnly, emitFake, Kind);
-    };
-
     auto emitCreates = [&](bool DelayedArgsOnly, bool emitFake) {
-      emitBothImplicitAndNonCreates(DelayedArgsOnly, emitFake,
-                                    CreateKind::WithNoArgs);
-      emitBothImplicitAndNonCreates(DelayedArgsOnly, emitFake,
-                                    CreateKind::WithAttributeCommonInfo);
-      emitBothImplicitAndNonCreates(DelayedArgsOnly, emitFake,
-                                    CreateKind::WithSourceRange);
+      emitCreate(true, DelayedArgsOnly, emitFake);
+      emitCreate(false, DelayedArgsOnly, emitFake);
       emitCreateNoCI(true, DelayedArgsOnly, emitFake);
       emitCreateNoCI(false, DelayedArgsOnly, emitFake);
     };
@@ -3468,6 +3456,10 @@ void EmitClangAttrHasAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
   OS << "case AttributeCommonInfo::Syntax::AS_ContextSensitiveKeyword:\n";
   OS << "  llvm_unreachable(\"hasAttribute not supported for keyword\");\n";
   OS << "  return 0;\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_Implicit:\n";
+  OS << "  llvm_unreachable (\"hasAttribute not supported for "
+        "AS_Implicit\");\n";
+  OS << "  return 0;\n";
 
   OS << "}\n";
 }


        


More information about the cfe-commits mailing list