rsmith created this revision.
rsmith added reviewers: aaron.ballman, erichkeane.
rsmith added a project: clang.
Herald added a subscriber: sanjoy.
Attribute instantiation would previously default to instantiating each kind of
attribute only once. This was overridden by a flag whose intended purpose was
to permit attributes from a prior declaration to be inherited onto a new
declaration even if that new declaration had its own copy of the attribute.
This appears to be the wrong behavior: when instantiating attributes from a
template, we should always instantiate all the attributes that were written on
that template.
This patch renames the flag in the Attr class (and TableGen sources) to more
clearly identify what it's actually for, and removes the usage of the flag from
template instantiation. I also removed the flag from AlignedAttr, which was
only added to work around the seemingly-incorrect suppression of duplicate
attribute instantiation.
Repository:
rC Clang
https://reviews.llvm.org/D41736
Files:
include/clang/AST/Attr.h
include/clang/Basic/Attr.td
lib/Sema/SemaDecl.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -2065,10 +2065,13 @@
ArrayRef<std::pair<Record *, SMRange>> Supers = R.getSuperClasses();
assert(!Supers.empty() && "Forgot to specify a superclass for the attr");
std::string SuperName;
+ bool Inheritable = false;
for (const auto &Super : llvm::reverse(Supers)) {
const Record *R = Super.first;
if (R->getName() != "TargetSpecificAttr" && SuperName.empty())
SuperName = R->getName();
+ if (R->getName() == "InheritableAttr")
+ Inheritable = true;
}
OS << "class " << R.getName() << "Attr : public " << SuperName << " {\n";
@@ -2162,8 +2165,13 @@
OS << " )\n";
OS << " : " << SuperName << "(attr::" << R.getName() << ", R, SI, "
- << ( R.getValueAsBit("LateParsed") ? "true" : "false" ) << ", "
- << ( R.getValueAsBit("DuplicatesAllowedWhileMerging") ? "true" : "false" ) << ")\n";
+ << ( R.getValueAsBit("LateParsed") ? "true" : "false" );
+ if (Inheritable) {
+ OS << ", "
+ << (R.getValueAsBit("InheritEvenIfAlreadyPresent") ? "true"
+ : "false");
+ }
+ OS << ")\n";
for (auto const &ai : Args) {
OS << " , ";
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -343,14 +343,6 @@
Attr.getRange());
}
-static bool DeclContainsAttr(const Decl *D, const Attr *NewAttr) {
- if (!D->hasAttrs() || NewAttr->duplicatesAllowed())
- return false;
- return llvm::find_if(D->getAttrs(), [NewAttr](const Attr *Attr) {
- return Attr->getKind() == NewAttr->getKind();
- }) != D->getAttrs().end();
-}
-
void Sema::InstantiateAttrsForDecl(
const MultiLevelTemplateArgumentList &TemplateArgs, const Decl *Tmpl,
Decl *New, LateInstantiatedAttrVec *LateAttrs,
@@ -365,7 +357,7 @@
Attr *NewAttr = sema::instantiateTemplateAttributeForDecl(
TmplAttr, Context, *this, TemplateArgs);
- if (NewAttr && !DeclContainsAttr(New, NewAttr))
+ if (NewAttr)
New->addAttr(NewAttr);
}
}
@@ -470,8 +462,7 @@
Attr *NewAttr = sema::instantiateTemplateAttribute(TmplAttr, Context,
*this, TemplateArgs);
-
- if (NewAttr && !DeclContainsAttr(New, NewAttr))
+ if (NewAttr)
New->addAttr(NewAttr);
}
}
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -2495,7 +2495,7 @@
else if (const auto *UA = dyn_cast<UuidAttr>(Attr))
NewAttr = S.mergeUuidAttr(D, UA->getRange(), AttrSpellingListIndex,
UA->getGuid());
- else if (Attr->duplicatesAllowed() || !DeclHasAttr(D, Attr))
+ else if (Attr->isInheritEvenIfAlreadyPresent() || !DeclHasAttr(D, Attr))
NewAttr = cast<InheritableAttr>(Attr->clone(S.Context));
if (NewAttr) {
Index: include/clang/Basic/Attr.td
===================================================================
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -441,9 +441,6 @@
// Set to true if all of the attribute's arguments should be parsed in an
// unevaluated context.
bit ParseArgumentsAsUnevaluated = 0;
- // Set to true if this attribute can be duplicated on a subject when merging
- // attributes. By default, attributes are not merged.
- bit DuplicatesAllowedWhileMerging = 0;
// Set to true if this attribute meaningful when applied to or inherited
// in a class template definition.
bit MeaningfulToClassTemplateDefinition = 0;
@@ -478,7 +475,11 @@
class StmtAttr : Attr;
/// An inheritable attribute is inherited by later redeclarations.
-class InheritableAttr : Attr;
+class InheritableAttr : Attr {
+ // Set to true if this attribute can be duplicated on a subject when inheriting
+ // attributes from prior declarations.
+ bit InheritEvenIfAlreadyPresent = 0;
+}
/// A target-specific attribute. This class is meant to be used as a mixin
/// with InheritableAttr or Attr depending on the attribute's needs.
@@ -552,7 +553,6 @@
Keyword<"_Alignas">]>,
Accessor<"isDeclspec",[Declspec<"align">]>];
let Documentation = [Undocumented];
- let DuplicatesAllowedWhileMerging = 1;
}
def AlignValue : Attr {
@@ -710,7 +710,7 @@
.Default(Platform);
} }];
let HasCustomParsing = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Named]>;
let Documentation = [AvailabilityDocs];
}
@@ -1380,7 +1380,7 @@
return false;
} }];
// FIXME: We should merge duplicates into a single nonnull attribute.
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Documentation = [NonNullDocs];
}
@@ -1917,7 +1917,7 @@
["DT_Error", "DT_Warning"]>,
BoolArgument<"ArgDependent", 0, /*fake*/ 1>,
NamedArgument<"Parent", 0, /*fake*/ 1>];
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let LateParsed = 1;
let AdditionalMembers = [{
bool isError() const { return diagnosticType == DT_Error; }
@@ -2160,7 +2160,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Args = [VariadicExprArgument<"Args">];
let Accessors = [Accessor<"isShared",
[Clang<"assert_shared_capability">]>];
@@ -2176,7 +2176,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Args = [VariadicExprArgument<"Args">];
let Accessors = [Accessor<"isShared",
[Clang<"acquire_shared_capability">,
@@ -2192,7 +2192,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Args = [ExprArgument<"SuccessValue">, VariadicExprArgument<"Args">];
let Accessors = [Accessor<"isShared",
[Clang<"try_acquire_shared_capability">]>];
@@ -2208,7 +2208,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Args = [VariadicExprArgument<"Args">];
let Accessors = [Accessor<"isShared",
[Clang<"release_shared_capability">]>,
@@ -2227,7 +2227,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Function]>;
let Accessors = [Accessor<"isShared", [Clang<"requires_shared_capability">,
Clang<"shared_locks_required">]>];
@@ -2246,7 +2246,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Field, SharedVar]>;
let Documentation = [Undocumented];
}
@@ -2257,7 +2257,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Field, SharedVar]>;
let Documentation = [Undocumented];
}
@@ -2268,7 +2268,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Field, SharedVar]>;
let Documentation = [Undocumented];
}
@@ -2279,7 +2279,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Field, SharedVar]>;
let Documentation = [Undocumented];
}
@@ -2290,7 +2290,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Function]>;
let Documentation = [Undocumented];
}
@@ -2301,7 +2301,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Function]>;
let Documentation = [Undocumented];
}
@@ -2314,7 +2314,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Function]>;
let Documentation = [Undocumented];
}
@@ -2327,7 +2327,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Function]>;
let Documentation = [Undocumented];
}
@@ -2348,7 +2348,7 @@
let LateParsed = 1;
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
- let DuplicatesAllowedWhileMerging = 1;
+ let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Function]>;
let Documentation = [Undocumented];
}
Index: include/clang/AST/Attr.h
===================================================================
--- include/clang/AST/Attr.h
+++ include/clang/AST/Attr.h
@@ -52,8 +52,10 @@
unsigned Inherited : 1;
unsigned IsPackExpansion : 1;
unsigned Implicit : 1;
+ // FIXME: These are properties of the attribute kind, not state for this
+ // instance of the attribute.
unsigned IsLateParsed : 1;
- unsigned DuplicatesAllowed : 1;
+ unsigned InheritEvenIfAlreadyPresent : 1;
void *operator new(size_t bytes) noexcept {
llvm_unreachable("Attrs cannot be allocated with regular 'new'.");
@@ -74,10 +76,10 @@
protected:
Attr(attr::Kind AK, SourceRange R, unsigned SpellingListIndex,
- bool IsLateParsed, bool DuplicatesAllowed)
+ bool IsLateParsed)
: Range(R), AttrKind(AK), SpellingListIndex(SpellingListIndex),
Inherited(false), IsPackExpansion(false), Implicit(false),
- IsLateParsed(IsLateParsed), DuplicatesAllowed(DuplicatesAllowed) {}
+ IsLateParsed(IsLateParsed), InheritEvenIfAlreadyPresent(false) {}
public:
@@ -109,18 +111,13 @@
// Pretty print this attribute.
void printPretty(raw_ostream &OS, const PrintingPolicy &Policy) const;
-
- /// \brief By default, attributes cannot be duplicated when being merged;
- /// however, an attribute can override this. Returns true if the attribute
- /// can be duplicated when merging.
- bool duplicatesAllowed() const { return DuplicatesAllowed; }
};
class StmtAttr : public Attr {
protected:
StmtAttr(attr::Kind AK, SourceRange R, unsigned SpellingListIndex,
- bool IsLateParsed, bool DuplicatesAllowed)
- : Attr(AK, R, SpellingListIndex, IsLateParsed, DuplicatesAllowed) {}
+ bool IsLateParsed)
+ : Attr(AK, R, SpellingListIndex, IsLateParsed) {}
public:
static bool classof(const Attr *A) {
@@ -132,12 +129,20 @@
class InheritableAttr : public Attr {
protected:
InheritableAttr(attr::Kind AK, SourceRange R, unsigned SpellingListIndex,
- bool IsLateParsed, bool DuplicatesAllowed)
- : Attr(AK, R, SpellingListIndex, IsLateParsed, DuplicatesAllowed) {}
+ bool IsLateParsed, bool InheritEvenIfAlreadyPresent)
+ : Attr(AK, R, SpellingListIndex, IsLateParsed) {
+ this->InheritEvenIfAlreadyPresent = InheritEvenIfAlreadyPresent;
+ }
public:
void setInherited(bool I) { Inherited = I; }
+ /// Should this attribute be inherited from a prior declaration even if it's
+ /// explicitly provided in the current declaration?
+ bool isInheritEvenIfAlreadyPresent() const {
+ return InheritEvenIfAlreadyPresent;
+ }
+
// Implement isa/cast/dyncast/etc.
static bool classof(const Attr *A) {
return A->getKind() >= attr::FirstInheritableAttr &&
@@ -148,9 +153,9 @@
class InheritableParamAttr : public InheritableAttr {
protected:
InheritableParamAttr(attr::Kind AK, SourceRange R, unsigned SpellingListIndex,
- bool IsLateParsed, bool DuplicatesAllowed)
+ bool IsLateParsed, bool InheritEvenIfAlreadyPresent)
: InheritableAttr(AK, R, SpellingListIndex, IsLateParsed,
- DuplicatesAllowed) {}
+ InheritEvenIfAlreadyPresent) {}
public:
// Implement isa/cast/dyncast/etc.
@@ -166,9 +171,9 @@
protected:
ParameterABIAttr(attr::Kind AK, SourceRange R,
unsigned SpellingListIndex, bool IsLateParsed,
- bool DuplicatesAllowed)
+ bool InheritEvenIfAlreadyPresent)
: InheritableParamAttr(AK, R, SpellingListIndex, IsLateParsed,
- DuplicatesAllowed) {}
+ InheritEvenIfAlreadyPresent) {}
public:
ParameterABI getABI() const {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
