Author: Stephen Kelly Date: 2020-11-02T20:21:48Z New Revision: 53df3beb624989ed32d87697d0c17601d7871465
URL: https://github.com/llvm/llvm-project/commit/53df3beb624989ed32d87697d0c17601d7871465 DIFF: https://github.com/llvm/llvm-project/commit/53df3beb624989ed32d87697d0c17601d7871465.diff LOG: Ignore template instantiations if not in AsIs mode Summary: IgnoreUnlessSpelledInSource mode should ignore these because they are not written in the source. This matters for example when trying to replace types or values which are templated. The new test in TransformerTest.cpp in this commit demonstrates the problem. In existing matcher code, users can write `unless(isInTemplateInstantiation())` or `unless(isInstantiated())` (the user must know which to use). The point of the TK_IgnoreUnlessSpelledInSource mode is to allow the novice to avoid such details. This patch changes the IgnoreUnlessSpelledInSource mode to skip over implicit template instantiations. This patch does not change the TK_AsIs mode. Note: An obvious attempt at an alternative implementation would simply change the shouldVisitTemplateInstantiations() in ASTMatchFinder.cpp to return something conditional on the operational TraversalKind. That does not work because shouldVisitTemplateInstantiations() is called before a possible top-level traverse() matcher changes the operational TraversalKind. Reviewers: sammccall, aaron.ballman, gribozavr2, ymandel, klimek Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D80961 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 ee4fb032dd51..092747e4387d 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 ff735cd73198..46f1d63752d8 100644 --- a/clang/unittests/Tooling/TransformerTest.cpp +++ b/clang/unittests/Tooling/TransformerTest.cpp @@ -1064,6 +1064,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
