Skip to content

Commit e841d50

Browse files
rsandifo-armrsandifo-arm
committed
[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
1 parent cafefe0 commit e841d50

File tree

7 files changed

+45
-71
lines changed

7 files changed

+45
-71
lines changed

‎clang/include/clang/Basic/Attr.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -834,7 +834,7 @@ def Annotate : InheritableParamAttr {
834834
return AnnotateAttr::Create(Ctx, Annotation, nullptr, 0, CommonInfo);
835835
}
836836
static AnnotateAttr *CreateImplicit(ASTContext &Ctx, llvm::StringRef Annotation, \
837-
const AttributeCommonInfo &CommonInfo = AttributeCommonInfo{SourceRange{}}) {
837+
const AttributeCommonInfo &CommonInfo) {
838838
return AnnotateAttr::CreateImplicit(Ctx, Annotation, nullptr, 0, CommonInfo);
839839
}
840840
}];

‎clang/include/clang/Basic/AttributeCommonInfo.h

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ class AttributeCommonInfo {
5151

5252
/// <vardecl> : <semantic>
5353
AS_HLSLSemantic,
54+
55+
/// The attibute has no source code manifestation and is only created
56+
/// implicitly.
57+
AS_Implicit
5458
};
5559
enum Kind {
5660
#define PARSED_ATTR(NAME) AT_##NAME,
@@ -76,14 +80,6 @@ class AttributeCommonInfo {
7680
static constexpr unsigned SpellingNotCalculated = 0xf;
7781

7882
public:
79-
explicit AttributeCommonInfo(SourceRange AttrRange)
80-
: AttrRange(AttrRange), ScopeLoc(), AttrKind(0), SyntaxUsed(0),
81-
SpellingIndex(SpellingNotCalculated) {}
82-
83-
explicit AttributeCommonInfo(SourceLocation AttrLoc)
84-
: AttrRange(AttrLoc), ScopeLoc(), AttrKind(0), SyntaxUsed(0),
85-
SpellingIndex(SpellingNotCalculated) {}
86-
8783
AttributeCommonInfo(const IdentifierInfo *AttrName,
8884
const IdentifierInfo *ScopeName, SourceRange AttrRange,
8985
SourceLocation ScopeLoc, Syntax SyntaxUsed)

‎clang/lib/Sema/SemaDecl.cpp

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10125,9 +10125,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
1012510125
NewFD->setParams(Params);
1012610126

1012710127
if (D.getDeclSpec().isNoreturnSpecified())
10128-
NewFD->addAttr(C11NoReturnAttr::Create(Context,
10129-
D.getDeclSpec().getNoreturnSpecLoc(),
10130-
AttributeCommonInfo::AS_Keyword));
10128+
NewFD->addAttr(
10129+
C11NoReturnAttr::Create(Context, D.getDeclSpec().getNoreturnSpecLoc()));
1013110130

1013210131
// Functions returning a variably modified type violate C99 6.7.5.2p2
1013310132
// because all functions have linkage.
@@ -10142,7 +10141,7 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
1014210141
!NewFD->hasAttr<SectionAttr>())
1014310142
NewFD->addAttr(PragmaClangTextSectionAttr::CreateImplicit(
1014410143
Context, PragmaClangTextSection.SectionName,
10145-
PragmaClangTextSection.PragmaLocation, AttributeCommonInfo::AS_Pragma));
10144+
PragmaClangTextSection.PragmaLocation));
1014610145

1014710146
// Apply an implicit SectionAttr if #pragma code_seg is active.
1014810147
if (CodeSegStack.CurrentValue && D.isFunctionDefinition() &&
@@ -10163,8 +10162,7 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
1016310162
if (StrictGuardStackCheckStack.CurrentValue && D.isFunctionDefinition() &&
1016410163
!NewFD->hasAttr<StrictGuardStackCheckAttr>())
1016510164
NewFD->addAttr(StrictGuardStackCheckAttr::CreateImplicit(
10166-
Context, PragmaClangTextSection.PragmaLocation,
10167-
AttributeCommonInfo::AS_Pragma));
10165+
Context, PragmaClangTextSection.PragmaLocation));
1016810166

1016910167
// Apply an implicit CodeSegAttr from class declspec or
1017010168
// apply an implicit SectionAttr from #pragma code_seg if active.
@@ -14210,8 +14208,7 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
1421014208
// attribute.
1421114209
if (CurInitSeg && var->getInit())
1421214210
var->addAttr(InitSegAttr::CreateImplicit(Context, CurInitSeg->getString(),
14213-
CurInitSegLoc,
14214-
AttributeCommonInfo::AS_Pragma));
14211+
CurInitSegLoc));
1421514212
}
1421614213

1421714214
// All the following checks are C++ only.
@@ -14313,23 +14310,19 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
1431314310
if (PragmaClangBSSSection.Valid)
1431414311
VD->addAttr(PragmaClangBSSSectionAttr::CreateImplicit(
1431514312
Context, PragmaClangBSSSection.SectionName,
14316-
PragmaClangBSSSection.PragmaLocation,
14317-
AttributeCommonInfo::AS_Pragma));
14313+
PragmaClangBSSSection.PragmaLocation));
1431814314
if (PragmaClangDataSection.Valid)
1431914315
VD->addAttr(PragmaClangDataSectionAttr::CreateImplicit(
1432014316
Context, PragmaClangDataSection.SectionName,
14321-
PragmaClangDataSection.PragmaLocation,
14322-
AttributeCommonInfo::AS_Pragma));
14317+
PragmaClangDataSection.PragmaLocation));
1432314318
if (PragmaClangRodataSection.Valid)
1432414319
VD->addAttr(PragmaClangRodataSectionAttr::CreateImplicit(
1432514320
Context, PragmaClangRodataSection.SectionName,
14326-
PragmaClangRodataSection.PragmaLocation,
14327-
AttributeCommonInfo::AS_Pragma));
14321+
PragmaClangRodataSection.PragmaLocation));
1432814322
if (PragmaClangRelroSection.Valid)
1432914323
VD->addAttr(PragmaClangRelroSectionAttr::CreateImplicit(
1433014324
Context, PragmaClangRelroSection.SectionName,
14331-
PragmaClangRelroSection.PragmaLocation,
14332-
AttributeCommonInfo::AS_Pragma));
14325+
PragmaClangRelroSection.PragmaLocation));
1433314326
}
1433414327

1433514328
if (auto *DD = dyn_cast<DecompositionDecl>(ThisDecl)) {

‎clang/lib/Sema/SemaDeclCXX.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3567,8 +3567,7 @@ Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D,
35673567
}
35683568

35693569
if (VS.isOverrideSpecified())
3570-
Member->addAttr(OverrideAttr::Create(Context, VS.getOverrideLoc(),
3571-
AttributeCommonInfo::AS_Keyword));
3570+
Member->addAttr(OverrideAttr::Create(Context, VS.getOverrideLoc()));
35723571
if (VS.isFinalSpecified())
35733572
Member->addAttr(FinalAttr::Create(
35743573
Context, VS.getFinalLoc(), AttributeCommonInfo::AS_Keyword,

‎clang/lib/Serialization/ASTReaderDecl.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4617,9 +4617,8 @@ void ASTDeclReader::UpdateDecl(Decl *D,
46174617
break;
46184618

46194619
case UPD_DECL_MARKED_OPENMP_THREADPRIVATE:
4620-
D->addAttr(OMPThreadPrivateDeclAttr::CreateImplicit(
4621-
Reader.getContext(), readSourceRange(),
4622-
AttributeCommonInfo::AS_Pragma));
4620+
D->addAttr(OMPThreadPrivateDeclAttr::CreateImplicit(Reader.getContext(),
4621+
readSourceRange()));
46234622
break;
46244623

46254624
case UPD_DECL_MARKED_OPENMP_ALLOCATE: {
@@ -4629,8 +4628,7 @@ void ASTDeclReader::UpdateDecl(Decl *D,
46294628
Expr *Alignment = Record.readExpr();
46304629
SourceRange SR = readSourceRange();
46314630
D->addAttr(OMPAllocateDeclAttr::CreateImplicit(
4632-
Reader.getContext(), AllocatorKind, Allocator, Alignment, SR,
4633-
AttributeCommonInfo::AS_Pragma));
4631+
Reader.getContext(), AllocatorKind, Allocator, Alignment, SR));
46344632
break;
46354633
}
46364634

@@ -4651,7 +4649,7 @@ void ASTDeclReader::UpdateDecl(Decl *D,
46514649
unsigned Level = Record.readInt();
46524650
D->addAttr(OMPDeclareTargetDeclAttr::CreateImplicit(
46534651
Reader.getContext(), MapType, DevType, IndirectE, Indirect, Level,
4654-
readSourceRange(), AttributeCommonInfo::AS_Pragma));
4652+
readSourceRange()));
46554653
break;
46564654
}
46574655

‎clang/unittests/AST/DeclTest.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,8 @@ TEST(Decl, AsmLabelAttr) {
8888
NamedDecl *DeclG = *(++DeclS->method_begin());
8989

9090
// Attach asm labels to the decls: one literal, and one not.
91-
DeclF->addAttr(::new (Ctx) AsmLabelAttr(
92-
Ctx, AttributeCommonInfo{SourceLocation()}, "foo",
93-
/*LiteralLabel=*/true));
94-
DeclG->addAttr(::new (Ctx) AsmLabelAttr(
95-
Ctx, AttributeCommonInfo{SourceLocation()}, "goo",
96-
/*LiteralLabel=*/false));
91+
DeclF->addAttr(AsmLabelAttr::Create(Ctx, "foo", /*LiteralLabel=*/true));
92+
DeclG->addAttr(AsmLabelAttr::Create(Ctx, "goo", /*LiteralLabel=*/false));
9793

9894
// Mangle the decl names.
9995
std::string MangleF, MangleG;

‎clang/utils/TableGen/ClangAttrEmitter.cpp

Lines changed: 24 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2505,15 +2505,8 @@ static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
25052505
return &R == P.second;
25062506
});
25072507

2508-
enum class CreateKind {
2509-
WithAttributeCommonInfo,
2510-
WithSourceRange,
2511-
WithNoArgs,
2512-
};
2513-
25142508
// Emit CreateImplicit factory methods.
2515-
auto emitCreate = [&](bool Implicit, bool DelayedArgsOnly,
2516-
bool emitFake, CreateKind Kind) {
2509+
auto emitCreate = [&](bool Implicit, bool DelayedArgsOnly, bool emitFake) {
25172510
if (Header)
25182511
OS << " static ";
25192512
OS << R.getName() << "Attr *";
@@ -2537,10 +2530,7 @@ static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
25372530
OS << ", ";
25382531
DelayedArgs->writeCtorParameters(OS);
25392532
}
2540-
if (Kind == CreateKind::WithAttributeCommonInfo)
2541-
OS << ", const AttributeCommonInfo &CommonInfo";
2542-
else if (Kind == CreateKind::WithSourceRange)
2543-
OS << ", SourceRange R";
2533+
OS << ", const AttributeCommonInfo &CommonInfo";
25442534
OS << ")";
25452535
if (Header) {
25462536
OS << ";\n";
@@ -2549,12 +2539,7 @@ static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
25492539

25502540
OS << " {\n";
25512541
OS << " auto *A = new (Ctx) " << R.getName();
2552-
if (Kind == CreateKind::WithAttributeCommonInfo)
2553-
OS << "Attr(Ctx, CommonInfo";
2554-
else if (Kind == CreateKind::WithSourceRange)
2555-
OS << "Attr(Ctx, AttributeCommonInfo{R}";
2556-
else if (Kind == CreateKind::WithNoArgs)
2557-
OS << "Attr(Ctx, AttributeCommonInfo{SourceLocation{}}";
2542+
OS << "Attr(Ctx, CommonInfo";
25582543

25592544
if (!DelayedArgsOnly) {
25602545
for (auto const &ai : Args) {
@@ -2606,7 +2591,14 @@ static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
26062591
OS << ", ";
26072592
DelayedArgs->writeCtorParameters(OS);
26082593
}
2609-
OS << ", SourceRange Range, AttributeCommonInfo::Syntax Syntax";
2594+
OS << ", SourceRange Range";
2595+
if (Header)
2596+
OS << " = {}";
2597+
if (Spellings.size() > 1) {
2598+
OS << ", AttributeCommonInfo::Syntax Syntax";
2599+
if (Header)
2600+
OS << " = AttributeCommonInfo::AS_" << Spellings[0].variety();
2601+
}
26102602
if (!ElideSpelling) {
26112603
OS << ", " << R.getName() << "Attr::Spelling S";
26122604
if (Header)
@@ -2626,7 +2618,13 @@ static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
26262618
else
26272619
OS << "NoSemaHandlerAttribute";
26282620

2629-
OS << ", Syntax";
2621+
if (Spellings.size() == 0)
2622+
OS << ", AttributeCommonInfo::AS_Implicit";
2623+
else if (Spellings.size() == 1)
2624+
OS << ", AttributeCommonInfo::AS_" << Spellings[0].variety();
2625+
else
2626+
OS << ", Syntax";
2627+
26302628
if (!ElideSpelling)
26312629
OS << ", S";
26322630
OS << ");\n";
@@ -2651,19 +2649,9 @@ static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
26512649
OS << "}\n\n";
26522650
};
26532651

2654-
auto emitBothImplicitAndNonCreates = [&](bool DelayedArgsOnly,
2655-
bool emitFake, CreateKind Kind) {
2656-
emitCreate(true, DelayedArgsOnly, emitFake, Kind);
2657-
emitCreate(false, DelayedArgsOnly, emitFake, Kind);
2658-
};
2659-
26602652
auto emitCreates = [&](bool DelayedArgsOnly, bool emitFake) {
2661-
emitBothImplicitAndNonCreates(DelayedArgsOnly, emitFake,
2662-
CreateKind::WithNoArgs);
2663-
emitBothImplicitAndNonCreates(DelayedArgsOnly, emitFake,
2664-
CreateKind::WithAttributeCommonInfo);
2665-
emitBothImplicitAndNonCreates(DelayedArgsOnly, emitFake,
2666-
CreateKind::WithSourceRange);
2653+
emitCreate(true, DelayedArgsOnly, emitFake);
2654+
emitCreate(false, DelayedArgsOnly, emitFake);
26672655
emitCreateNoCI(true, DelayedArgsOnly, emitFake);
26682656
emitCreateNoCI(false, DelayedArgsOnly, emitFake);
26692657
};
@@ -3468,6 +3456,10 @@ void EmitClangAttrHasAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
34683456
OS << "case AttributeCommonInfo::Syntax::AS_ContextSensitiveKeyword:\n";
34693457
OS << " llvm_unreachable(\"hasAttribute not supported for keyword\");\n";
34703458
OS << " return 0;\n";
3459+
OS << "case AttributeCommonInfo::Syntax::AS_Implicit:\n";
3460+
OS << " llvm_unreachable (\"hasAttribute not supported for "
3461+
"AS_Implicit\");\n";
3462+
OS << " return 0;\n";
34713463

34723464
OS << "}\n";
34733465
}

0 commit comments

Comments
 (0)