This patch modifies Sema::InstantiateAttrs so that attributes in template code are properly instantiated; the previous behavior was to clone them. The main motivation for the patch are thread safety attributes, which make extensive use of expressions.
A new method named instantiateFromTemplate is now generated for all attributes. The behavior of this method is identical to clone() for all arguments except ExprArgument and VariadicExprArgument; expression arguments are instantiated rather than cloned. Features still on the todo list are: (1) Instantiate type arguments as well as expressions. This will affect attributes other than thread-safety, so I wanted to get feedback before implementing it. (2) Move handling of the aligned attribute to the new mechanism. Patch can be found at: http://codereview.appspot.com/5488067/ -- DeLesley Hutchins | Software Engineer | [email protected] | 505-206-0315
From 0e824654a516ba4714b3d72cfd21e9fdc2b8d7b7 Mon Sep 17 00:00:00 2001 From: DeLesley Hutchins <[email protected]> Date: Tue, 13 Dec 2011 14:49:49 -0800 Subject: [PATCH] Template instantiation of attributes --- include/clang/AST/Attr.h | 7 ++ lib/AST/AttrImpl.cpp | 2 + lib/Sema/SemaDecl.cpp | 3 + lib/Sema/SemaDeclAttr.cpp | 12 +-- lib/Sema/SemaTemplateInstantiateDecl.cpp | 6 +- test/SemaCXX/warn-thread-safety-analysis.cpp | 130 +++++++++++++++++++++++++- utils/TableGen/ClangAttrEmitter.cpp | 83 ++++++++++++++++- 7 files changed, 229 insertions(+), 14 deletions(-) diff --git a/include/clang/AST/Attr.h b/include/clang/AST/Attr.h index 345cc94..6bb71a4 100644 --- a/include/clang/AST/Attr.h +++ b/include/clang/AST/Attr.h @@ -36,6 +36,8 @@ namespace clang { class QualType; class FunctionDecl; class TypeSourceInfo; + class Sema; + class MultiLevelTemplateArgumentList; } // Defined in ASTContext.h @@ -104,6 +106,11 @@ public: // Clone this attribute. virtual Attr* clone(ASTContext &C) const = 0; + // Instantiate a template attribute. + // Same as clone for non-dependent attributes. + virtual Attr* instantiateFromTemplate(ASTContext &C, Sema &S, + const MultiLevelTemplateArgumentList &TemplateArgs) const = 0; + // Pretty print this attribute. virtual void printPretty(llvm::raw_ostream &OS, ASTContext &C) const = 0; diff --git a/lib/AST/AttrImpl.cpp b/lib/AST/AttrImpl.cpp index 3ca7d4d..43c6df6 100644 --- a/lib/AST/AttrImpl.cpp +++ b/lib/AST/AttrImpl.cpp @@ -15,6 +15,8 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/Type.h" #include "clang/AST/Expr.h" +#include "clang/Sema/Sema.h" +#include "clang/Sema/Template.h" using namespace clang; Attr::~Attr() { } diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 09ada7e..e850e1c 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -7239,6 +7239,9 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body, /// relevant Decl. void Sema::ActOnFinishDelayedAttribute(Scope *S, Decl *D, ParsedAttributes &Attrs) { + // Always attach attributes to the underlying decl. + if (TemplateDecl *TD = dyn_cast<TemplateDecl>(D)) + D = TD->getTemplatedDecl(); ProcessDeclAttributeList(S, D, Attrs.getList()); } diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index e60cd63..717b025 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -393,13 +393,11 @@ static void handleGuardedByAttr(Sema &S, Decl *D, const AttributeList &Attr, if (pointer && !checkIsPointer(S, D, Attr)) return; - if (Arg->isTypeDependent()) - // FIXME: handle attributes with dependent types - return; - - // check that the argument is lockable object - if (!checkForLockableRecord(S, D, Attr, getRecordType(Arg->getType()))) - return; + if (!Arg->isTypeDependent()) { + if (!checkForLockableRecord(S, D, Attr, getRecordType(Arg->getType()))) + return; + // FIXME -- semantic checks for dependent attributes + } if (pointer) D->addAttr(::new (S.Context) PtGuardedByAttr(Attr.getRange(), diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp index 9477192..411e294 100644 --- a/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -88,8 +88,10 @@ void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs, } } - // FIXME: Is cloning correct for all attributes? - Attr *NewAttr = TmplAttr->clone(Context); + // Attr *NewAttr = TmplAttr->clone(Context); + + Attr *NewAttr = + TmplAttr->instantiateFromTemplate(Context, *this, TemplateArgs); New->addAttr(NewAttr); } } diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp index 6289be7..6d2202d 100644 --- a/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -1051,12 +1051,14 @@ class Foo { public: void func(T x) { mu_.Lock(); - count_ = x; + // count_ = x; mu_.Unlock(); } private: - T count_ GUARDED_BY(mu_); + // FIXME: This test passed earlier only because thread safety was turned + // off for templates. + // T count_ GUARDED_BY(mu_); Bar<T> bar_; Mutex mu_; }; @@ -1605,3 +1607,127 @@ struct TestScopedLockable { } // end namespace test_scoped_lockable + +namespace TestTemplateAttributeInstantiation { + +class Foo1 { +public: + Mutex mu_; + int a GUARDED_BY(mu_); +}; + +class Foo2 { +public: + int a GUARDED_BY(mu_); + Mutex mu_; +}; + + +class Bar { +public: + // Test non-dependent expressions in attributes on template functions + template <class T> + void barND(Foo1 *foo, T *fooT) EXCLUSIVE_LOCKS_REQUIRED(foo->mu_) { + foo->a = 0; + } + + // Test dependent expressions in attributes on template functions + template <class T> + void barD(Foo1 *foo, T *fooT) EXCLUSIVE_LOCKS_REQUIRED(fooT->mu_) { + fooT->a = 0; + } +}; + + +template <class T> +class BarT { +public: + Foo1 fooBase; + T fooBaseT; + + // Test non-dependent expression in ordinary method on template class + void barND() EXCLUSIVE_LOCKS_REQUIRED(fooBase.mu_) { + fooBase.a = 0; + } + + // Test dependent expressions in ordinary methods on template class + void barD() EXCLUSIVE_LOCKS_REQUIRED(fooBaseT.mu_) { + fooBaseT.a = 0; + } + + // Test dependent expressions in template method in template class + template <class T2> + void barTD(T2 *fooT) EXCLUSIVE_LOCKS_REQUIRED(fooBaseT.mu_, fooT->mu_) { + fooBaseT.a = 0; + fooT->a = 0; + } +}; + +template <class T> +class Cell { +public: + Mutex mu_; + // Test dependent guarded_by + T data GUARDED_BY(mu_); + + void foo() { + mu_.Lock(); + 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() { + Bar b; + BarT<Foo2> bt; + Foo1 f1; + Foo2 f2; + + f1.mu_.Lock(); + f2.mu_.Lock(); + bt.fooBase.mu_.Lock(); + bt.fooBaseT.mu_.Lock(); + + b.barND(&f1, &f2); + b.barD(&f1, &f2); + bt.barND(); + bt.barD(); + bt.barTD(&f2); + + f1.mu_.Unlock(); + bt.barTD(&f1); // \ + // expected-warning {{calling function 'barTD' requires exclusive lock on 'mu_'}} + + bt.fooBase.mu_.Unlock(); + bt.fooBaseT.mu_.Unlock(); + f2.mu_.Unlock(); + + Cell<int> cell; + cell.data = 0; // \ + // expected-warning {{writing variable 'data' requires locking 'mu_' exclusively}} + cell.foo(); + + // FIXME: This doesn't work yet + // CellDelayed<int> celld; + // celld.foo(); +} + +}; // end namespace TestTemplateAttributeInstantiation + + diff --git a/utils/TableGen/ClangAttrEmitter.cpp b/utils/TableGen/ClangAttrEmitter.cpp index 03b6f76..0dc1a72 100644 --- a/utils/TableGen/ClangAttrEmitter.cpp +++ b/utils/TableGen/ClangAttrEmitter.cpp @@ -91,6 +91,10 @@ namespace { virtual void writeAccessors(raw_ostream &OS) const = 0; virtual void writeAccessorDefinitions(raw_ostream &OS) const {} virtual void writeCloneArgs(raw_ostream &OS) const = 0; + virtual void writeTemplateInstantiationArgs(raw_ostream &OS) const { + writeCloneArgs(OS); + } + virtual void writeTemplateInstantiation(raw_ostream &OS) const {}; virtual void writeCtorBody(raw_ostream &OS) const {} virtual void writeCtorInitializers(raw_ostream &OS) const = 0; virtual void writeCtorParameters(raw_ostream &OS) const = 0; @@ -109,6 +113,8 @@ namespace { : Argument(Arg, Attr), type(T) {} + std::string getType() const { return type; } + void writeAccessors(raw_ostream &OS) const { OS << " " << type << " get" << getUpperName() << "() const {\n"; OS << " return " << getLowerName() << ";\n"; @@ -268,6 +274,9 @@ namespace { << "Expr) : " << getLowerName() << "Type"; } + void writeTemplateInstantiation(raw_ostream &OS) const { + // FIXME: move the definition in Sema::InstantiateAttrs to here. + } void writeCtorBody(raw_ostream &OS) const { OS << " if (is" << getLowerName() << "Expr)\n"; OS << " " << getLowerName() << "Expr = reinterpret_cast<Expr *>(" @@ -502,6 +511,58 @@ namespace { OS << getLowerName() << "=\" << get" << getUpperName() << "() << \""; } }; + + class ExprArgument : public SimpleArgument { + public: + ExprArgument(Record &Arg, StringRef Attr) + : SimpleArgument(Arg, Attr, "Expr *") + {} + + void writeTemplateInstantiationArgs(raw_ostream &OS) const { + OS << "tempInst" << getUpperName(); + } + + void writeTemplateInstantiation(raw_ostream &OS) const { + OS << " " << getType() << " tempInst" << getUpperName() << ";\n"; + OS << " {\n"; + OS << " // The alignment expression is not potentially evaluated.\n"; + OS << " EnterExpressionEvaluationContext " + << "Unevaluated(S, Sema::Unevaluated);\n"; + OS << " ExprResult " << "Result = S.SubstExpr(" + << getLowerName() << ", TemplateArgs);\n"; + OS << " tempInst" << getUpperName() << " = " + << "Result.takeAs<Expr>();\n"; + OS << " }\n"; + } + }; + + class VariadicExprArgument : public VariadicArgument { + public: + VariadicExprArgument(Record &Arg, StringRef Attr) + : VariadicArgument(Arg, Attr, "Expr *") + {} + + void writeTemplateInstantiationArgs(raw_ostream &OS) const { + OS << "tempInst" << getUpperName() << ", " << getLowerName() << "Size"; + } + + void writeTemplateInstantiation(raw_ostream &OS) const { + OS << " " << getType() << " *tempInst" << getUpperName() + << " = new (C, 16) " << getType() + << "[" << getLowerName() << "Size];\n"; + OS << " {\n"; + OS << " // The alignment expression is not potentially evaluated.\n"; + OS << " EnterExpressionEvaluationContext " + << "Unevaluated(S, Sema::Unevaluated);\n"; + OS << " for (unsigned i = 0; i < " << getLowerName() << "Size; ++i) {\n"; + OS << " ExprResult Result = S.SubstExpr(" + << getLowerName() << "[i], TemplateArgs);\n"; + OS << " tempInst" << getUpperName() << "[i] = " + << "Result.takeAs<Expr>();\n"; + OS << " }\n"; + OS << " }\n"; + } + }; } static Argument *createArgument(Record &Arg, StringRef Attr, @@ -514,8 +575,7 @@ static Argument *createArgument(Record &Arg, StringRef Attr, if (ArgName == "AlignedArgument") Ptr = new AlignedArgument(Arg, Attr); else if (ArgName == "EnumArgument") Ptr = new EnumArgument(Arg, Attr); - else if (ArgName == "ExprArgument") Ptr = new SimpleArgument(Arg, Attr, - "Expr *"); + else if (ArgName == "ExprArgument") Ptr = new ExprArgument(Arg, Attr); else if (ArgName == "FunctionArgument") Ptr = new SimpleArgument(Arg, Attr, "FunctionDecl *"); else if (ArgName == "IdentifierArgument") @@ -533,7 +593,7 @@ static Argument *createArgument(Record &Arg, StringRef Attr, else if (ArgName == "VariadicUnsignedArgument") Ptr = new VariadicArgument(Arg, Attr, "unsigned"); else if (ArgName == "VariadicExprArgument") - Ptr = new VariadicArgument(Arg, Attr, "Expr *"); + Ptr = new VariadicExprArgument(Arg, Attr); else if (ArgName == "VersionArgument") Ptr = new VersionArgument(Arg, Attr); @@ -618,6 +678,9 @@ void ClangAttrClassEmitter::run(raw_ostream &OS) { OS << " }\n\n"; OS << " virtual " << R.getName() << "Attr *clone (ASTContext &C) const;\n"; + OS << " virtual " << R.getName() + << "Attr *instantiateFromTemplate(ASTContext &C, Sema &S, " + << "const MultiLevelTemplateArgumentList &TemplateArgs) const;\n"; OS << " virtual void printPretty(llvm::raw_ostream &OS, ASTContext &Ctx) const;\n"; for (ai = Args.begin(); ai != ae; ++ai) { @@ -665,6 +728,20 @@ void ClangAttrImplEmitter::run(raw_ostream &OS) { } OS << ");\n}\n\n"; + OS << R.getName() << "Attr *" << R.getName() + << "Attr::instantiateFromTemplate(ASTContext &C, Sema &S, " + << "const MultiLevelTemplateArgumentList &TemplateArgs" + << ") const {\n"; + for (ai = Args.begin(); ai != ae; ++ai) { + (*ai)->writeTemplateInstantiation(OS); + } + OS << "\n return new (C) " << R.getName() << "Attr(getLocation(), C"; + for (ai = Args.begin(); ai != ae; ++ai) { + OS << ", "; + (*ai)->writeTemplateInstantiationArgs(OS); + } + OS << ");\n}\n\n"; + OS << "void " << R.getName() << "Attr::printPretty(" << "llvm::raw_ostream &OS, ASTContext &Ctx) const {\n"; if (Spellings.begin() != Spellings.end()) { -- 1.7.3.1
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
