Skip to content

Commit

Permalink
[clang] Ensure that Attr::Create(Implicit) chooses a valid syntax
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rsandifo-arm committed Apr 13, 2023
1 parent cafefe0 commit e841d50
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 71 deletions.
2 changes: 1 addition & 1 deletion clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}];
Expand Down
12 changes: 4 additions & 8 deletions clang/include/clang/Basic/AttributeCommonInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand Down
25 changes: 9 additions & 16 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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() &&
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)) {
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 4 additions & 6 deletions clang/lib/Serialization/ASTReaderDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down
8 changes: 2 additions & 6 deletions clang/unittests/AST/DeclTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
56 changes: 24 additions & 32 deletions clang/utils/TableGen/ClangAttrEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 *";
Expand All @@ -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";
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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";
Expand All @@ -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);
};
Expand Down Expand Up @@ -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";
}
Expand Down

0 comments on commit e841d50

Please sign in to comment.