[AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor

[johannes]

Summary

We need to specialize this because RecursiveASTVisitor visits template
template parameters after the templated declaration, unlike the order in
which they appear in the source code.

[johannes]

use LexicallyOrderedRecursiveASTVisitor

[johannes]

unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp

146

This test also works before the change; what is the best way to enforce an ordering here?

[arphaman]

Awesome, thanks!

unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp

146

You can add a flag to LexicallyOrderedDeclVisitor that emits the ordering index when it's true and turn it on for this test. Then you'd should get something like /f/T@0 and /f/T@1.

Please test a class template as well.

[johannes]

test ordering, class template

[johannes]

This is currently broken, if a user provides a TraverseClassTemplateDecl, then the same method in this class will not be called, I think I will add a flag (probably not user visible) in RecursiveASTVisitor.h to switch the order for templates

[johannes]

fix by adding an option in RecursiveASTVisitor

[arphaman]

include/clang/AST/RecursiveASTVisitor.h

507–508

I don't think you need to make this public with the new patch, right?

[johannes]

undo visibility change

[arphaman]

LGTM

[pdouglas]

This comment was removed by pdouglas.

include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h

for (auto I = DC->decls_begin(), E = DC->decls_end(); I != E;) {
           LexicallyNestedDeclarations.push_back(Sibling);
       }
       if (!BaseType::getDerived().TraverseDecl(Child))
         return false;
     }
     return true;
   }
 
+#define DEF_TRAVERSE_TEMPLATE_DECL(TMPLDECLKIND)                               \
+  bool Traverse##TMPLDECLKIND##TemplateDecl(TMPLDECLKIND##TemplateDecl *TD) {  \
+    if (!BaseType::TraverseTemplateParameterListHelper(                        \
+            TD->getTemplateParameters()))                                      \
+      return false;                                                            \
+    assert(!BaseType::getDerived().shouldVisitTemplateInstantiations() &&      \
+           "It does not make sense for most clients to visit template "        \
+           "instantiations here.");                                            \
+    return BaseType::getDerived().TraverseDecl(TD->getTemplatedDecl());        \
+  }
+  DEF_TRAVERSE_TEMPLATE_DECL(Class)
+  DEF_TRAVERSE_TEMPLATE_DECL(Var)
+  DEF_TRAVERSE_TEMPLATE_DECL(Function)
+#undef DEF_TRAVERSE_TEMPLATE_DECL
+
 private:
   bool TraverseAdditionalLexicallyNestedDeclarations() {
     // FIXME: Ideally the gathered declarations and the declarations in the
     // @implementation should be mixed and sorted to get a true lexical order,
     // but right now we only care about getting the correct lexical parent, so
     // we can traverse the gathered nested declarations after the declarations
     // in the decl context.
     assert(!BaseType::getDerived().shouldTraversePostOrder() &&

include/clang/AST/RecursiveASTVisitor.h

bool WalkUpFrom##CLASS##Decl(CLASS##Decl *D) {                               \
     TRY_TO(Visit##CLASS##Decl(D));                                             \
     return true;                                                               \
   }                                                                            \
   bool Visit##CLASS##Decl(CLASS##Decl *D) { return true; }
 #include "clang/AST/DeclNodes.inc"
 
   bool canIgnoreChildDeclWhileTraversingDeclContext(const Decl *Child);
 
-private:
   // These are helper methods used by more than one Traverse* method.
   bool TraverseTemplateParameterListHelper(TemplateParameterList *TPL);
 
+private:
   // Traverses template parameter lists of either a DeclaratorDecl or TagDecl.
   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,

unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp

for (const Decl *D : TraversalStack) {
     if (isa<TranslationUnitDecl>(D)) {
       OS << "/";
       continue;
     }
     if (const auto *ND = dyn_cast<NamedDecl>(D))
       OS << ND->getNameAsString();
     else
       OS << "???";
-    if (isa<DeclContext>(D))
+    if (isa<DeclContext>(D) or isa<TemplateDecl>(D))
       OS << "/";
   }
   Matcher.match(OS.str(), D);
   return true;
 }
 
 TEST(LexicallyOrderedRecursiveASTVisitor, VisitDeclsInImplementation) {
   StringRef Source = R"(

 )";
   DummyMatchVisitor Visitor;
   Visitor.ExpectMatch("/outerFunction/", 5, 1);
   Visitor.ExpectMatch("/I/nestedFunction1/", 7, 20);
   Visitor.ExpectMatch("/nestedFunction2/", 7, 20);
   EXPECT_TRUE(Visitor.runOver(Source, DummyMatchVisitor::Lang_OBJC));
 }
 
+TEST(LexicallyOrderedRecursiveASTVisitor, VisitTemplateDecl) {
+  StringRef Source = R"(
+template <class T> T f();
+)";
+  DummyMatchVisitor Visitor;
+  Visitor.ExpectMatch("/f/T", 2, 11);
+  Visitor.ExpectMatch("/f/f/", 2, 20);
+  EXPECT_TRUE(Visitor.runOver(Source));
+}
+
 } // end anonymous namespace