Author: Matt Morehouse Date: 2020-11-03T13:59:01-08:00 New Revision: a6d15d40701ad38f29e4ff93703b3ffa7b204611
URL: https://github.com/llvm/llvm-project/commit/a6d15d40701ad38f29e4ff93703b3ffa7b204611 DIFF: https://github.com/llvm/llvm-project/commit/a6d15d40701ad38f29e4ff93703b3ffa7b204611.diff LOG: Undo Revert "Ignore template instantiations if not in AsIs mode" MaskRay already fixed the ASan bug. Added: Modified: clang/include/clang/AST/ASTNodeTraverser.h clang/include/clang/AST/RecursiveASTVisitor.h clang/include/clang/ASTMatchers/ASTMatchersInternal.h clang/lib/AST/ASTDumper.cpp clang/lib/ASTMatchers/ASTMatchFinder.cpp clang/lib/ASTMatchers/ASTMatchersInternal.cpp clang/unittests/AST/ASTTraverserTest.cpp clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp clang/unittests/Tooling/TransformerTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/ASTNodeTraverser.h b/clang/include/clang/AST/ASTNodeTraverser.h index b0b1c152db05..1141f514d795 100644 --- a/clang/include/clang/AST/ASTNodeTraverser.h +++ b/clang/include/clang/AST/ASTNodeTraverser.h @@ -82,6 +82,7 @@ class ASTNodeTraverser bool getDeserialize() const { return Deserialize; } void SetTraversalKind(TraversalKind TK) { Traversal = TK; } + TraversalKind GetTraversalKind() const { return Traversal; } void Visit(const Decl *D) { getNodeDelegate().AddChild([=] { @@ -481,8 +482,10 @@ class ASTNodeTraverser Visit(D->getTemplatedDecl()); - for (const auto *Child : D->specializations()) - dumpTemplateDeclSpecialization(Child); + if (Traversal == TK_AsIs) { + for (const auto *Child : D->specializations()) + dumpTemplateDeclSpecialization(Child); + } } void VisitTypeAliasDecl(const TypeAliasDecl *D) { diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h index 5e83cded0652..0a36ec9ad687 100644 --- a/clang/include/clang/AST/RecursiveASTVisitor.h +++ b/clang/include/clang/AST/RecursiveASTVisitor.h @@ -461,6 +461,13 @@ template <typename Derived> class RecursiveASTVisitor { bool canIgnoreChildDeclWhileTraversingDeclContext(const Decl *Child); +#define DEF_TRAVERSE_TMPL_INST(TMPLDECLKIND) \ + bool TraverseTemplateInstantiations(TMPLDECLKIND##TemplateDecl *D); + DEF_TRAVERSE_TMPL_INST(Class) + DEF_TRAVERSE_TMPL_INST(Var) + DEF_TRAVERSE_TMPL_INST(Function) +#undef DEF_TRAVERSE_TMPL_INST + private: // These are helper methods used by more than one Traverse* method. bool TraverseTemplateParameterListHelper(TemplateParameterList *TPL); @@ -469,12 +476,6 @@ template <typename Derived> class RecursiveASTVisitor { template <typename T> bool TraverseDeclTemplateParameterLists(T *D); -#define DEF_TRAVERSE_TMPL_INST(TMPLDECLKIND) \ - bool TraverseTemplateInstantiations(TMPLDECLKIND##TemplateDecl *D); - DEF_TRAVERSE_TMPL_INST(Class) - DEF_TRAVERSE_TMPL_INST(Var) - DEF_TRAVERSE_TMPL_INST(Function) -#undef DEF_TRAVERSE_TMPL_INST bool TraverseTemplateArgumentLocsHelper(const TemplateArgumentLoc *TAL, unsigned Count); bool TraverseArrayTypeLocHelper(ArrayTypeLoc TL); diff --git a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h index 2a3f503f9951..1f5951877f24 100644 --- a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h +++ b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h @@ -586,6 +586,10 @@ class Matcher { return this->InnerMatcher.matches(DynTypedNode::create(*Node), Finder, Builder); } + + llvm::Optional<clang::TraversalKind> TraversalKind() const override { + return this->InnerMatcher.getTraversalKind(); + } }; private: @@ -1056,6 +1060,8 @@ class ASTMatchFinder { virtual ASTContext &getASTContext() const = 0; + virtual bool isMatchingInImplicitTemplateInstantiation() const = 0; + protected: virtual bool matchesChildOf(const DynTypedNode &Node, ASTContext &Ctx, const DynTypedMatcher &Matcher, diff --git a/clang/lib/AST/ASTDumper.cpp b/clang/lib/AST/ASTDumper.cpp index 284e5bdbc6b0..3d368a0a7b63 100644 --- a/clang/lib/AST/ASTDumper.cpp +++ b/clang/lib/AST/ASTDumper.cpp @@ -129,9 +129,11 @@ void ASTDumper::dumpTemplateDecl(const TemplateDecl *D, bool DumpExplicitInst) { Visit(D->getTemplatedDecl()); - for (const auto *Child : D->specializations()) - dumpTemplateDeclSpecialization(Child, DumpExplicitInst, - !D->isCanonicalDecl()); + if (GetTraversalKind() == TK_AsIs) { + for (const auto *Child : D->specializations()) + dumpTemplateDeclSpecialization(Child, DumpExplicitInst, + !D->isCanonicalDecl()); + } } void ASTDumper::VisitFunctionTemplateDecl(const FunctionTemplateDecl *D) { diff --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp b/clang/lib/ASTMatchers/ASTMatchFinder.cpp index b9b38923495a..2fa1a3f71eb5 100644 --- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp +++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp @@ -584,7 +584,45 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>, bool shouldVisitTemplateInstantiations() const { return true; } bool shouldVisitImplicitCode() const { return true; } + bool isMatchingInImplicitTemplateInstantiation() const override { + return TraversingImplicitTemplateInstantiation; + } + + bool TraverseTemplateInstantiations(ClassTemplateDecl *D) { + ImplicitTemplateInstantiationScope RAII(this, true); + return RecursiveASTVisitor<MatchASTVisitor>::TraverseTemplateInstantiations( + D); + } + + bool TraverseTemplateInstantiations(VarTemplateDecl *D) { + ImplicitTemplateInstantiationScope RAII(this, true); + return RecursiveASTVisitor<MatchASTVisitor>::TraverseTemplateInstantiations( + D); + } + + bool TraverseTemplateInstantiations(FunctionTemplateDecl *D) { + ImplicitTemplateInstantiationScope RAII(this, true); + return RecursiveASTVisitor<MatchASTVisitor>::TraverseTemplateInstantiations( + D); + } + private: + bool TraversingImplicitTemplateInstantiation = false; + + struct ImplicitTemplateInstantiationScope { + ImplicitTemplateInstantiationScope(MatchASTVisitor *V, bool B) + : MV(V), MB(V->TraversingImplicitTemplateInstantiation) { + V->TraversingImplicitTemplateInstantiation = B; + } + ~ImplicitTemplateInstantiationScope() { + MV->TraversingImplicitTemplateInstantiation = MB; + } + + private: + MatchASTVisitor *MV; + bool MB; + }; + class TimeBucketRegion { public: TimeBucketRegion() : Bucket(nullptr) {} diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp index 4e4e43b2a94a..c319213d28ab 100644 --- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp +++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp @@ -284,6 +284,11 @@ bool DynTypedMatcher::matches(const DynTypedNode &DynNode, TraversalKindScope RAII(Finder->getASTContext(), Implementation->TraversalKind()); + if (Finder->getASTContext().getParentMapContext().getTraversalKind() == + TK_IgnoreUnlessSpelledInSource && + Finder->isMatchingInImplicitTemplateInstantiation()) + return false; + auto N = Finder->getASTContext().getParentMapContext().traverseIgnored(DynNode); @@ -304,6 +309,11 @@ bool DynTypedMatcher::matchesNoKindCheck(const DynTypedNode &DynNode, TraversalKindScope raii(Finder->getASTContext(), Implementation->TraversalKind()); + if (Finder->getASTContext().getParentMapContext().getTraversalKind() == + TK_IgnoreUnlessSpelledInSource && + Finder->isMatchingInImplicitTemplateInstantiation()) + return false; + auto N = Finder->getASTContext().getParentMapContext().traverseIgnored(DynNode); diff --git a/clang/unittests/AST/ASTTraverserTest.cpp b/clang/unittests/AST/ASTTraverserTest.cpp index c24b43164cc8..6423b4d4cd4d 100644 --- a/clang/unittests/AST/ASTTraverserTest.cpp +++ b/clang/unittests/AST/ASTTraverserTest.cpp @@ -68,6 +68,14 @@ class NodeTreePrinter : public TextTreeStructure { void Visit(const TemplateArgument &A, SourceRange R = {}, const Decl *From = nullptr, const char *Label = nullptr) { OS << "TemplateArgument"; + switch (A.getKind()) { + case TemplateArgument::Type: { + OS << " type " << A.getAsType().getAsString(); + break; + } + default: + break; + } } template <typename... T> void Visit(T...) {} @@ -243,7 +251,7 @@ FullComment verifyWithDynNode(TA, R"cpp( -TemplateArgument +TemplateArgument type int `-BuiltinType )cpp"); @@ -1042,4 +1050,145 @@ LambdaExpr } } +TEST(Traverse, IgnoreUnlessSpelledInSourceTemplateInstantiations) { + + auto AST = buildASTFromCode(R"cpp( +template<typename T> +struct TemplStruct { + TemplStruct() {} + ~TemplStruct() {} + +private: + T m_t; +}; + +template<typename T> +T timesTwo(T input) +{ + return input * 2; +} + +void instantiate() +{ + TemplStruct<int> ti; + TemplStruct<double> td; + (void)timesTwo<int>(2); + (void)timesTwo<double>(2); +} +)cpp"); + { + auto BN = ast_matchers::match( + classTemplateDecl(hasName("TemplStruct")).bind("rec"), + AST->getASTContext()); + EXPECT_EQ(BN.size(), 1u); + + EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource, + BN[0].getNodeAs<Decl>("rec")), + R"cpp( +ClassTemplateDecl 'TemplStruct' +|-TemplateTypeParmDecl 'T' +`-CXXRecordDecl 'TemplStruct' + |-CXXRecordDecl 'TemplStruct' + |-CXXConstructorDecl 'TemplStruct<T>' + | `-CompoundStmt + |-CXXDestructorDecl '~TemplStruct<T>' + | `-CompoundStmt + |-AccessSpecDecl + `-FieldDecl 'm_t' +)cpp"); + + EXPECT_EQ(dumpASTString(TK_AsIs, BN[0].getNodeAs<Decl>("rec")), + R"cpp( +ClassTemplateDecl 'TemplStruct' +|-TemplateTypeParmDecl 'T' +|-CXXRecordDecl 'TemplStruct' +| |-CXXRecordDecl 'TemplStruct' +| |-CXXConstructorDecl 'TemplStruct<T>' +| | `-CompoundStmt +| |-CXXDestructorDecl '~TemplStruct<T>' +| | `-CompoundStmt +| |-AccessSpecDecl +| `-FieldDecl 'm_t' +|-ClassTemplateSpecializationDecl 'TemplStruct' +| |-TemplateArgument type int +| | `-BuiltinType +| |-CXXRecordDecl 'TemplStruct' +| |-CXXConstructorDecl 'TemplStruct' +| | `-CompoundStmt +| |-CXXDestructorDecl '~TemplStruct' +| | `-CompoundStmt +| |-AccessSpecDecl +| |-FieldDecl 'm_t' +| `-CXXConstructorDecl 'TemplStruct' +| `-ParmVarDecl '' +`-ClassTemplateSpecializationDecl 'TemplStruct' + |-TemplateArgument type double + | `-BuiltinType + |-CXXRecordDecl 'TemplStruct' + |-CXXConstructorDecl 'TemplStruct' + | `-CompoundStmt + |-CXXDestructorDecl '~TemplStruct' + | `-CompoundStmt + |-AccessSpecDecl + |-FieldDecl 'm_t' + `-CXXConstructorDecl 'TemplStruct' + `-ParmVarDecl '' +)cpp"); + } + { + auto BN = ast_matchers::match( + functionTemplateDecl(hasName("timesTwo")).bind("fn"), + AST->getASTContext()); + EXPECT_EQ(BN.size(), 1u); + + EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource, + BN[0].getNodeAs<Decl>("fn")), + R"cpp( +FunctionTemplateDecl 'timesTwo' +|-TemplateTypeParmDecl 'T' +`-FunctionDecl 'timesTwo' + |-ParmVarDecl 'input' + `-CompoundStmt + `-ReturnStmt + `-BinaryOperator + |-DeclRefExpr 'input' + `-IntegerLiteral +)cpp"); + + EXPECT_EQ(dumpASTString(TK_AsIs, BN[0].getNodeAs<Decl>("fn")), + R"cpp( +FunctionTemplateDecl 'timesTwo' +|-TemplateTypeParmDecl 'T' +|-FunctionDecl 'timesTwo' +| |-ParmVarDecl 'input' +| `-CompoundStmt +| `-ReturnStmt +| `-BinaryOperator +| |-DeclRefExpr 'input' +| `-IntegerLiteral +|-FunctionDecl 'timesTwo' +| |-TemplateArgument type int +| | `-BuiltinType +| |-ParmVarDecl 'input' +| `-CompoundStmt +| `-ReturnStmt +| `-BinaryOperator +| |-ImplicitCastExpr +| | `-DeclRefExpr 'input' +| `-IntegerLiteral +`-FunctionDecl 'timesTwo' + |-TemplateArgument type double + | `-BuiltinType + |-ParmVarDecl 'input' + `-CompoundStmt + `-ReturnStmt + `-BinaryOperator + |-ImplicitCastExpr + | `-DeclRefExpr 'input' + `-ImplicitCastExpr + `-IntegerLiteral +)cpp"); + } +} + } // namespace clang diff --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp index b4eb8041fcaf..13b5011bfe3e 100644 --- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp +++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp @@ -2085,9 +2085,17 @@ void actual_template_test() { traverse(TK_AsIs, staticAssertDecl(has(implicitCastExpr(has( substNonTypeTemplateParmExpr(has(integerLiteral()))))))))); + EXPECT_TRUE(matches( + Code, traverse(TK_IgnoreUnlessSpelledInSource, + staticAssertDecl(has(declRefExpr( + to(nonTypeTemplateParmDecl(hasName("alignment"))), + hasType(asString("unsigned int")))))))); - EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, - staticAssertDecl(has(integerLiteral()))))); + EXPECT_TRUE(matches(Code, traverse(TK_AsIs, staticAssertDecl(hasDescendant( + integerLiteral()))))); + EXPECT_FALSE(matches( + Code, traverse(TK_IgnoreUnlessSpelledInSource, + staticAssertDecl(hasDescendant(integerLiteral()))))); Code = R"cpp( diff --git a/clang/unittests/Tooling/TransformerTest.cpp b/clang/unittests/Tooling/TransformerTest.cpp index 58405e1f83df..1f0637446a00 100644 --- a/clang/unittests/Tooling/TransformerTest.cpp +++ b/clang/unittests/Tooling/TransformerTest.cpp @@ -1067,6 +1067,70 @@ TEST_F(TransformerTest, ErrorOccurredMatchSkipped) { EXPECT_EQ(ErrorCount, 0); } +TEST_F(TransformerTest, TemplateInstantiation) { + + std::string NonTemplatesInput = R"cpp( +struct S { + int m_i; +}; +)cpp"; + std::string NonTemplatesExpected = R"cpp( +struct S { + safe_int m_i; +}; +)cpp"; + + std::string TemplatesInput = R"cpp( +template<typename T> +struct TemplStruct { + TemplStruct() {} + ~TemplStruct() {} + +private: + T m_t; +}; + +void instantiate() +{ + TemplStruct<int> ti; +} +)cpp"; + + auto MatchedField = fieldDecl(hasType(asString("int"))).bind("theField"); + + // Changes the 'int' in 'S', but not the 'T' in 'TemplStruct': + testRule(makeRule(traverse(TK_IgnoreUnlessSpelledInSource, MatchedField), + changeTo(cat("safe_int ", name("theField")))), + NonTemplatesInput + TemplatesInput, + NonTemplatesExpected + TemplatesInput); + + // In AsIs mode, template instantiations are modified, which is + // often not desired: + + std::string IncorrectTemplatesExpected = R"cpp( +template<typename T> +struct TemplStruct { + TemplStruct() {} + ~TemplStruct() {} + +private: + safe_int m_t; +}; + +void instantiate() +{ + TemplStruct<int> ti; +} +)cpp"; + + // Changes the 'int' in 'S', and (incorrectly) the 'T' in 'TemplStruct': + testRule(makeRule(traverse(TK_AsIs, MatchedField), + changeTo(cat("safe_int ", name("theField")))), + + NonTemplatesInput + TemplatesInput, + NonTemplatesExpected + IncorrectTemplatesExpected); +} + // Transformation of macro source text when the change encompasses the entirety // of the expanded text. TEST_F(TransformerTest, SimpleMacro) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits