Make attribute instantiation instantiate all attributes, not just the first of

each kind.

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 is 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 incorrect suppression of duplicate attribute
instantiation.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@321834 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/Attr.h b/include/clang/AST/Attr.h
index bbe320c28a3b5afab9a752f752b1970b46c38540..a7c766a65ce1151ca7dcee7c53d2bd16c372b696 100644 (file)
--- a/include/clang/AST/Attr.h
+++ b/include/clang/AST/Attr.h
@@ -52,8 +52,10 @@ protected:
   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 @@ public:
 
 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 @@ public:
 
   // 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 @@ public:
 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 shouldInheritEvenIfAlreadyPresent() const {
+    return InheritEvenIfAlreadyPresent;
+  }
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Attr *A) {
     return A->getKind() >= attr::FirstInheritableAttr &&
@@ -148,9 +153,9 @@ public:
 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 @@ class ParameterABIAttr : public InheritableParamAttr {
 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 {

diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
index a0dc1a2b9a2e449aaa644fe5f7fa92d04c604d47..633c5977a5fa4dbd13dcb8f8945ea734d28cef21 100644 (file)
--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -441,9 +441,6 @@ class Attr {
   // 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 TypeAttr : Attr {
 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 @@ def Aligned : InheritableAttr {
                                           Keyword<"_Alignas">]>,
                    Accessor<"isDeclspec",[Declspec<"align">]>];
   let Documentation = [Undocumented];
-  let DuplicatesAllowedWhileMerging = 1;
 }
 
 def AlignValue : Attr {
@@ -710,7 +710,7 @@ static llvm::StringRef canonicalizePlatformName(llvm::StringRef Platform) {
              .Default(Platform);
 } }];
   let HasCustomParsing = 1;
-  let DuplicatesAllowedWhileMerging = 1;
+  let InheritEvenIfAlreadyPresent = 1;
   let Subjects = SubjectList<[Named]>;
   let Documentation = [AvailabilityDocs];
 }
@@ -1380,7 +1380,7 @@ def NonNull : InheritableParamAttr {
     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 @@ def DiagnoseIf : InheritableAttr {
                            ["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 @@ def AssertCapability : InheritableAttr {
   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 @@ def AcquireCapability : InheritableAttr {
   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 @@ def TryAcquireCapability : InheritableAttr {
   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 @@ def ReleaseCapability : InheritableAttr {
   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 @@ def RequiresCapability : InheritableAttr {
   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 @@ def GuardedBy : InheritableAttr {
   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 @@ def PtGuardedBy : InheritableAttr {
   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 @@ def AcquiredAfter : InheritableAttr {
   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 @@ def AcquiredBefore : InheritableAttr {
   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 @@ def AssertExclusiveLock : InheritableAttr {
   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 @@ def AssertSharedLock : InheritableAttr {
   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 @@ def ExclusiveTrylockFunction : InheritableAttr {
   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 @@ def SharedTrylockFunction : InheritableAttr {
   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 @@ def LocksExcluded : InheritableAttr {
   let LateParsed = 1;
   let TemplateDependent = 1;
   let ParseArgumentsAsUnevaluated = 1;
-  let DuplicatesAllowedWhileMerging = 1;
+  let InheritEvenIfAlreadyPresent = 1;
   let Subjects = SubjectList<[Function]>;
   let Documentation = [Undocumented];
 }

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 73952ad38af7c603ab188ac93c2e6afc3d5b3931..2b2ab42427bd59ad94f3026933bfd3979a1a1801 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -2495,7 +2495,7 @@ static bool mergeDeclAttribute(Sema &S, NamedDecl *D,
   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->shouldInheritEvenIfAlreadyPresent() || !DeclHasAttr(D, Attr))
     NewAttr = cast<InheritableAttr>(Attr->clone(S.Context));
 
   if (NewAttr) {

diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp
index ab68e7e671de631bd0bf0ab0fad141af64cc3bd7..0d45835742fd7e16c3d02ffa4e4182fe8f95f330 100644 (file)
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -343,14 +343,6 @@ static void instantiateOMPDeclareSimdDeclAttr(
       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 @@ void Sema::InstantiateAttrsForDecl(
 
       Attr *NewAttr = sema::instantiateTemplateAttributeForDecl(
           TmplAttr, Context, *this, TemplateArgs);
-      if (NewAttr && !DeclContainsAttr(New, NewAttr))
+      if (NewAttr)
         New->addAttr(NewAttr);
     }
   }
@@ -470,8 +462,7 @@ void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs,
 
       Attr *NewAttr = sema::instantiateTemplateAttribute(TmplAttr, Context,
                                                          *this, TemplateArgs);
-
-      if (NewAttr && !DeclContainsAttr(New, NewAttr))
+      if (NewAttr)
         New->addAttr(NewAttr);
     }
   }

diff --git a/test/SemaTemplate/attributes.cpp b/test/SemaTemplate/attributes.cpp
index 1d46058b0191adad300c2661a370af81db415e96..7634b937c906896c1b1829d3801bb902ba3a566d 100644 (file)
--- a/test/SemaTemplate/attributes.cpp
+++ b/test/SemaTemplate/attributes.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=gnu++11 -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=gnu++11 -ast-dump %s | FileCheck %s
 
 namespace attribute_aligned {
   template<int N>
@@ -52,3 +53,13 @@ namespace PR9049 {
   template<typename T>
   inline void WBCFRelease(__attribute__((cf_consumed)) T aValue) { if(aValue) CFRelease(aValue); }
 }
+
+// CHECK: FunctionTemplateDecl {{.*}} HasAnnotations
+// CHECK:   AnnotateAttr {{.*}} "ANNOTATE_BAR"
+// CHECK:   AnnotateAttr {{.*}} "ANNOTATE_FOO"
+// CHECK: FunctionDecl {{.*}} HasAnnotations
+// CHECK:   TemplateArgument type 'int'
+// CHECK:   AnnotateAttr {{.*}} "ANNOTATE_BAR"
+// CHECK:   AnnotateAttr {{.*}} "ANNOTATE_FOO"
+template<typename T> [[clang::annotate("ANNOTATE_FOO"), clang::annotate("ANNOTATE_BAR")]] void HasAnnotations();
+void UseAnnotations() { HasAnnotations<int>(); }

diff --git a/test/SemaTemplate/warn-thread-safety-analysis.cpp b/test/SemaTemplate/warn-thread-safety-analysis.cpp
new file mode 100644 (file)
index 0000000..03bae7a
--- /dev/null
+++ b/test/SemaTemplate/warn-thread-safety-analysis.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -std=c++11 %s -verify -Wthread-safety-analysis
+
+class Mutex {
+public:
+  void Lock() __attribute__((exclusive_lock_function()));
+  void Unlock() __attribute__((unlock_function()));
+};
+
+class A {
+public:
+  Mutex mu1, mu2;
+
+  void foo() __attribute__((exclusive_locks_required(mu1))) __attribute__((exclusive_locks_required(mu2))) {}
+
+  template <class T> void bar() __attribute__((exclusive_locks_required(mu1))) __attribute__((exclusive_locks_required(mu2))) {
+    foo();
+  }
+};
+
+void f() {
+  A a;
+  a.mu1.Lock();
+  a.mu2.Lock();
+  a.bar<int>();
+  a.mu2.Unlock();
+  a.bar<int>(); // expected-warning {{calling function 'bar' requires holding mutex 'a.mu2' exclusively}}
+  a.mu1.Unlock();
+  a.bar<int>(); // expected-warning {{calling function 'bar' requires holding mutex 'a.mu1' exclusively}} \
+                   expected-warning {{calling function 'bar' requires holding mutex 'a.mu2' exclusively}}
+}

diff --git a/utils/TableGen/ClangAttrEmitter.cpp b/utils/TableGen/ClangAttrEmitter.cpp
index 2a8b891e64b52fc67c8e596c82c147979ba7387a..c7e3d31001bfcf5963345ea68676be4a87e30a9a 100644 (file)
--- a/utils/TableGen/ClangAttrEmitter.cpp
+++ b/utils/TableGen/ClangAttrEmitter.cpp
@@ -2065,10 +2065,13 @@ void EmitClangAttrClass(RecordKeeper &Records, raw_ostream &OS) {
     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 @@ void EmitClangAttrClass(RecordKeeper &Records, raw_ostream &OS) {
 
       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 << "              , ";