[clang] 53df3be - Ignore template instantiations if not in AsIs mode
Stephen Kelly via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 2 12:22:23 PST 2020
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) {
More information about the cfe-commits
mailing list