[clang] a6d15d4 - Undo Revert "Ignore template instantiations if not in AsIs mode"
Matt Morehouse via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 3 13:59:57 PST 2020
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) {
More information about the cfe-commits
mailing list