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

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) {

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);

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,

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) {

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) {}

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);
 

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

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(