[cfe-commits] [PATCH] Delayed template instantiation for late-parsed attributes

Richard Smith richard at metafoo.co.uk
Tue Dec 20 15:35:12 PST 2011


Hi Delesley,

On Tue, December 20, 2011 19:55, Delesley Hutchins wrote:
> This patch delays the template instantiation of late-parsed
> attributes, so that such attributes can be used in template code the same way
> they are used in normal code.  Late parsed attributes may refer to
> declarations later in the class, e.g.
>
> class Foo { int a __attribute__((guarded_by(mu))); Mutex mu;
> };
>
>
> If Foo is templatized, then instantiation of the guarded_by attribute
> must be delayed in the same way that parsing is delayed.  During class
> instantiation, we store each delayed attribute in a list, along with a clone
> of the local instantiation scope, and then instantiate and attach the
> attributes at the end of class instantiation.
>
> This patch is built on my previous patch for template instantiation of
> attributes.

Thanks for working on this! It looks like this gives us some nice (and
necessary) infrastructure for eventual support for instantiation of
(late-parsed) exception specifications too. It's a shame we have to allocate
for the LocalInstantiationScopes, but I don't see any way to avoid that.

Some comments below:

diff --git a/include/clang/AST/Attr.h b/include/clang/AST/Attr.h
index 6bb71a4..4620af6 100644
--- a/include/clang/AST/Attr.h
+++ b/include/clang/AST/Attr.h
@@ -111,6 +111,8 @@ public:
   virtual Attr* instantiateFromTemplate(ASTContext &C, Sema &S,
                   const MultiLevelTemplateArgumentList &TemplateArgs) const = 0;

+  virtual bool isLateParsed() const { return false; }
+
   // Pretty print this attribute.
   virtual void printPretty(llvm::raw_ostream &OS, ASTContext &C) const = 0;

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index bd542da..50d8fc4 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -5126,8 +5126,22 @@ public:
                    TemplateSpecializationKind TSK,
                    bool Complain = true);

+  struct LateInstantiatedAttribute {
+    const Attr *TmplAttr;
+    LocalInstantiationScope *Scope;
+    Decl *NewDecl;
+
+    LateInstantiatedAttribute(const Attr *A, LocalInstantiationScope *S,
+                              Decl *D)
+      : TmplAttr(A), Scope(S), NewDecl(D)
+    { }
+  };
+  typedef SmallVector<LateInstantiatedAttribute, 16> LateInstantiatedAttrVec;
+
   void InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs,
-                        const Decl *Pattern, Decl *Inst);
+                        const Decl *Pattern, Decl *Inst,
+                        LateInstantiatedAttrVec *LateAttrs = 0,
+                        LocalInstantiationScope *OuterMostScope = 0);

   bool
   InstantiateClassTemplateSpecialization(SourceLocation PointOfInstantiation,
diff --git a/include/clang/Sema/Template.h b/include/clang/Sema/Template.h
index 78f50fa..1b709bc 100644
--- a/include/clang/Sema/Template.h
+++ b/include/clang/Sema/Template.h
@@ -268,6 +268,48 @@ namespace clang {
       Exited = true;
     }

+    /// \brief Clone this scope, and all outer scopes, down to the given
+    /// outermost scope.
+    LocalInstantiationScope *cloneScopes(LocalInstantiationScope *Outermost) {
+      if (this == Outermost || !this) return this;

Please avoid calling the function on a null pointer, rather than checking
'this' here. Such a call would have undefined behaviour by
[class.mfct.non-static]p2.

+      LocalInstantiationScope *newScope =
+        new LocalInstantiationScope(SemaRef, CombineWithOuterScope);
+
+      newScope->Outer = Outer->cloneScopes(Outermost);
+      newScope->PartiallySubstitutedPack = PartiallySubstitutedPack;
+      newScope->ArgsInPartiallySubstitutedPack = ArgsInPartiallySubstitutedPack;
+      newScope->NumArgsInPartiallySubstitutedPack =
+        NumArgsInPartiallySubstitutedPack;
+
+      for (LocalDeclsMap::iterator I = LocalDecls.begin(), E = LocalDecls.end();
+           I != E; ++I) {
+        const Decl *D = I->first;
+        llvm::PointerUnion<Decl *, DeclArgumentPack *> &Stored =
+          newScope->LocalDecls[D];
+        if (I->second.is<Decl *>()) {
+          Stored = I->second.get<Decl *>();
+        } else {
+          DeclArgumentPack *OldPack = I->second.get<DeclArgumentPack *>();
+          DeclArgumentPack *Pack = new DeclArgumentPack;

new DeclArgumentPack(*OldPack) ?

+          Stored = Pack;
+          ArgumentPacks.push_back(Pack);
+          Pack->append(OldPack->begin(), OldPack->end());
+        }
+      }
+      return newScope;
+    }
+
+    /// \brief deletes the given scope, and all otuer scopes, down to the
+    /// given outermost scope.
+    static void deleteScopes(LocalInstantiationScope *Scope,
+                             LocalInstantiationScope *Outermost) {
+      while (Scope && Scope != Outermost) {
+        LocalInstantiationScope *Out = Scope->Outer;
+        delete Scope;
+        Scope = Out;
+      }
+    }
+
     /// \brief Find the instantiation of the declaration D within the current
     /// instantiation scope.
     ///
@@ -314,6 +356,8 @@ namespace clang {
     Sema::ArgumentPackSubstitutionIndexRAII SubstIndex;
     DeclContext *Owner;
     const MultiLevelTemplateArgumentList &TemplateArgs;
+    Sema::LateInstantiatedAttrVec* LateAttrs;
+    LocalInstantiationScope *StartingScope;

     /// \brief A list of out-of-line class template partial
     /// specializations that will need to be instantiated after the
@@ -326,7 +370,7 @@ namespace clang {
     TemplateDeclInstantiator(Sema &SemaRef, DeclContext *Owner,
                              const MultiLevelTemplateArgumentList &TemplateArgs)
       : SemaRef(SemaRef), SubstIndex(SemaRef, -1), Owner(Owner),
-        TemplateArgs(TemplateArgs) { }
+        TemplateArgs(TemplateArgs), LateAttrs(0), StartingScope(0) { }

     // FIXME: Once we get closer to completion, replace these manually-written
     // declarations with automatically-generated ones from
@@ -382,6 +426,21 @@ namespace clang {
       return 0;
     }

+    // Enable late instantiation of attributes.  Late instantiated attributes
+    // will be stored in LA.
+    void enableLateAttributeInstantiation(Sema::LateInstantiatedAttrVec *LA) {
+      LateAttrs = LA;
+      StartingScope = SemaRef.CurrentInstantiationScope;
+    }
+
+    // Disable late instantiation of attributes.
+    void disableLateAttributeInstantiation() {
+      LateAttrs = 0;
+      StartingScope = 0;
+    }
+
+    LocalInstantiationScope *getStartingScope() const { return StartingScope; }
+
     typedef
       SmallVectorImpl<std::pair<ClassTemplateDecl *,
                                      ClassTemplatePartialSpecializationDecl *> >
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index 9644d62..17177bb 100644
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -857,7 +857,7 @@ void Parser::ParseThreadSafetyAttribute(IdentifierInfo
&AttrName,
     ConsumeToken(); // Eat the comma, move to the next argument
   }
   // Match the ')'.
-  if (ArgExprsOk && !T.consumeClose() && ArgExprs.size() > 0) {
+  if (ArgExprsOk && !T.consumeClose()) {

This appears to be an unrelated change?

     Attrs.addNew(&AttrName, AttrNameLoc, 0, AttrNameLoc, 0, SourceLocation(),
                  ArgExprs.take(), ArgExprs.size());
   }
diff --git a/lib/Sema/SemaTemplateInstantiate.cpp
b/lib/Sema/SemaTemplateInstantiate.cpp
index e86912a..adbba8a 100644
--- a/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1756,6 +1756,10 @@ Sema::InstantiateClass(SourceLocation
PointOfInstantiation,
   SmallVector<Decl*, 4> Fields;
   SmallVector<std::pair<FieldDecl*, FieldDecl*>, 4>
     FieldsWithMemberInitializers;
+  // Delay instantiation of late parsed attributes.
+  LateInstantiatedAttrVec LateAttrs;
+  Instantiator.enableLateAttributeInstantiation(&LateAttrs);
+
   for (RecordDecl::decl_iterator Member = Pattern->decls_begin(),
          MemberEnd = Pattern->decls_end();
        Member != MemberEnd; ++Member) {
@@ -1815,6 +1819,21 @@ Sema::InstantiateClass(SourceLocation
PointOfInstantiation,
     }
   }

+  // Instantiate late parsed attributes, and attach them to their decls.
+  // See Sema::InstantiateAttrs
+  for (LateInstantiatedAttrVec::iterator I = LateAttrs.begin(),
+       E = LateAttrs.end(); I != E; ++I) {
+    assert(CurrentInstantiationScope == Instantiator.getStartingScope());
+    CurrentInstantiationScope = I->Scope;
+    Attr *NewAttr =
+      I->TmplAttr->instantiateFromTemplate(Context, *this, TemplateArgs);
+    I->NewDecl->addAttr(NewAttr);
+    LocalInstantiationScope::deleteScopes(I->Scope,
+                                          Instantiator.getStartingScope());
+  }
+  Instantiator.disableLateAttributeInstantiation();
+  LateAttrs.clear();
+
   if (!FieldsWithMemberInitializers.empty())
     ActOnFinishDelayedMemberInitializers(Instantiation);

diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 63df1e0..8c2c3b7 100644
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -59,10 +59,13 @@ bool TemplateDeclInstantiator::SubstQualifier(const TagDecl
*OldDecl,

 // FIXME: Is this still too simple?
 void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs,
-                            const Decl *Tmpl, Decl *New) {
+                            const Decl *Tmpl, Decl *New,
+                            LateInstantiatedAttrVec *LateAttrs,
+                            LocalInstantiationScope *OuterMostScope) {
   for (AttrVec::const_iterator i = Tmpl->attr_begin(), e = Tmpl->attr_end();
        i != e; ++i) {
     const Attr *TmplAttr = *i;
+
     // FIXME: This should be generalized to more than just the AlignedAttr.
     if (const AlignedAttr *Aligned = dyn_cast<AlignedAttr>(TmplAttr)) {
       if (Aligned->isAlignmentDependent()) {
@@ -88,11 +91,18 @@ void Sema::InstantiateAttrs(const
MultiLevelTemplateArgumentList
&TemplateArgs,
       }
     }

-    // Attr *NewAttr = TmplAttr->clone(Context);
-
-    Attr *NewAttr =
-      TmplAttr->instantiateFromTemplate(Context, *this, TemplateArgs);
-    New->addAttr(NewAttr);
+    if (TmplAttr->isLateParsed() && LateAttrs) {
+      // Late parsed attributes must be instantiated and attached after the
+      // enclosing class has been instantiated.  See Sema::InstantiateClass.
+      LocalInstantiationScope *Saved =
+        CurrentInstantiationScope->cloneScopes(OuterMostScope);
+      LateAttrs->push_back(LateInstantiatedAttribute(TmplAttr, Saved, New));
+    }
+    else {

The prevailing style is to put the } and else on the same line.

+      Attr *NewAttr =
+        TmplAttr->instantiateFromTemplate(Context, *this, TemplateArgs);
+      New->addAttr(NewAttr);
+    }
   }
 }

@@ -491,7 +501,7 @@ Decl *TemplateDeclInstantiator::VisitFieldDecl(FieldDecl
*D) {
     return 0;
   }

-  SemaRef.InstantiateAttrs(TemplateArgs, D, Field);
+  SemaRef.InstantiateAttrs(TemplateArgs, D, Field, LateAttrs, StartingScope);

   if (Invalid)
     Field->setInvalidDecl();
@@ -2373,7 +2383,8 @@
TemplateDeclInstantiator::InitFunctionInstantiation(FunctionDecl *New,
   // Get the definition. Leaves the variable unchanged if undefined.
   Tmpl->isDefined(Definition);

-  SemaRef.InstantiateAttrs(TemplateArgs, Definition, New);
+  SemaRef.InstantiateAttrs(TemplateArgs, Definition, New,
+                           LateAttrs, StartingScope);

   return false;
 }
diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp
b/test/SemaCXX/warn-thread-safety-analysis.cpp
index 3ab3227..535fb61 100644
--- a/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1051,14 +1051,12 @@ class Foo {
  public:
   void func(T x) {
     mu_.Lock();
-    // count_ = x;
+    count_ = x;
     mu_.Unlock();
   }

  private:
-  // FIXME: This test passed earlier only because thread safety was turned
-  // off for templates.
-  // T count_ GUARDED_BY(mu_);
+  T count_ GUARDED_BY(mu_);
   Bar<T> bar_;
   Mutex mu_;
 };
@@ -1670,27 +1668,15 @@ public:
   // Test dependent guarded_by
   T data GUARDED_BY(mu_);

-  void foo() {
-    mu_.Lock();
+  void fooEx() EXCLUSIVE_LOCKS_REQUIRED(mu_) {
     data = 0;
-    mu_.Unlock();
   }
-};
-
-
-template <class T>
-class CellDelayed {
-public:
-  // Test dependent guarded_by
-  T data GUARDED_BY(mu_);

   void foo() {
     mu_.Lock();
     data = 0;
     mu_.Unlock();
   }
-
-  Mutex mu_;
 };

 void test() {
@@ -1722,12 +1708,50 @@ void test() {
   cell.data = 0; // \
     // expected-warning {{writing variable 'data' requires locking 'mu_'
exclusively}}
   cell.foo();
+  cell.mu_.Lock();
+  cell.fooEx();
+  cell.mu_.Unlock();
+}
+

-  // FIXME: This doesn't work yet
-  // CellDelayed<int> celld;
-  // celld.foo();
+template <class T>
+class CellDelayed {
+public:
+  // Test dependent guarded_by
+  T data GUARDED_BY(mu_);
+
+  void fooEx(CellDelayed<T> *other) EXCLUSIVE_LOCKS_REQUIRED(mu_, other->mu_) {
+    this->data = other->data;
+  }
+
+  template <class T2>
+  void fooExT(CellDelayed<T2> *otherT) EXCLUSIVE_LOCKS_REQUIRED(mu_,
otherT->mu_) {
+    this->data = otherT->data;
+  }
+
+  void foo() {
+    mu_.Lock();
+    data = 0;
+    mu_.Unlock();
+  }
+
+  Mutex mu_;
+};
+
+void testDelayed() {
+  CellDelayed<int> celld;
+  CellDelayed<int> celld2;
+  celld.foo();
+  celld.mu_.Lock();
+  celld2.mu_.Lock();
+
+  celld.fooEx(&celld2);
+  celld.fooExT(&celld2);
+
+  celld2.mu_.Unlock();
+  celld.mu_.Unlock();
 }

-};  // end namespace TestTemplateAttributeInstantiation

+};  // end namespace TestTemplateAttributeInstantiation

diff --git a/utils/TableGen/ClangAttrEmitter.cpp
b/utils/TableGen/ClangAttrEmitter.cpp
index 0dc1a72..bddfcbd 100644
--- a/utils/TableGen/ClangAttrEmitter.cpp
+++ b/utils/TableGen/ClangAttrEmitter.cpp
@@ -346,7 +346,7 @@ namespace {
          << "Size;\n";
       OS << "  }\n";
       OS << "  unsigned " << getLowerName() << "_size() const {\n"
-         << "    return " << getLowerName() << "Size;\n;";
+         << "    return " << getLowerName() << "Size;\n";
       OS << "  }";
     }
     void writeCloneArgs(raw_ostream &OS) const {
@@ -695,6 +695,11 @@ void ClangAttrClassEmitter::run(raw_ostream &OS) {
        << "attr::" << R.getName() << "; }\n";
     OS << "  static bool classof(const " << R.getName()
        << "Attr *) { return true; }\n";
+
+    bool LateParsed = R.getValueAsBit("LateParsed");
+    OS << "  virtual bool isLateParsed() const { return "
+       << LateParsed << "; }\n";
+
     OS << "};\n\n";
   }





More information about the cfe-commits mailing list