[clangd] Handle template arguments in findExplicitReferences

[ilya-biryukov]

clang-tools-extra/clangd/FindTarget.cpp

576

This comment is a bit unclear, I'll have to change it.
The idea is that this is handled by VisitType and VisitExpression

[kadircet]

clang-tools-extra/clangd/FindTarget.cpp

556

... and we need it because, template template decls are visited through it ?

567

I suppose these corresponds to non-type template paramters, don't they? (which should make it similar to integral case)

618

can we keep dlog ?

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

751

can you also name the template param, add a reference in wrapper and check for it ?

[ilya-biryukov]

clang-tools-extra/clangd/FindTarget.cpp

556

Yes, exactly. Amended this to the comment.

567

Yes, e.g. see the wrapper<func> example from the tests.
However, these arguments seem to be stored and visited as expressions in the TemplateArgumentLoc.

IIUC, Declaration can only happen in template arguments outside TemplateArgumentLoc.

618

I've initially removed it to avoid passing DynTypedNode to reportReference, but I also agree this dlog is useful.

So making the function signature a bit clunky for the sake of debug logging

[ilya-biryukov]

• Restore dlog()
• Check reference to function pointer non-type template parameters in tests
• Extend comment of TraverseTemplateArgumentLoc

[kadircet]

LGTM, thanks!

clang-tools-extra/clangd/FindTarget.cpp

 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtVisitor.h"
+#include "clang/AST/TemplateBase.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeLocVisitor.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
bool TraverseElaboratedTypeLoc(ElaboratedTypeLoc L) {
     return RecursiveASTVisitor::TraverseElaboratedTypeLoc(L);
   }
 
   bool VisitExpr(Expr *E) {
     visitNode(DynTypedNode::create(*E));
     return true;
   }
 
+  // We re-define Traverse*, since there's no corresponding Visit*.
+  bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) {
+    switch (A.getArgument().getKind()) {
+    case TemplateArgument::Template:
+    case TemplateArgument::TemplateExpansion:
+      reportReference(ReferenceLoc{A.getTemplateQualifierLoc(),
+                                   A.getTemplateNameLoc(),
+                                   {A.getArgument()
+                                        .getAsTemplateOrTemplatePattern()
+                                        .getAsTemplateDecl()}});
+      break;
+    case TemplateArgument::Declaration:
+      break; // FIXME: can this actually happen in TemplateArgumentLoc?
+    case TemplateArgument::Integral:
+    case TemplateArgument::Null:
+    case TemplateArgument::NullPtr:
+      break; // no references.
+    case TemplateArgument::Pack:
+    case TemplateArgument::Type:
+    case TemplateArgument::Expression:
+      break; // Handled by visited functions.
+    };
+    return RecursiveASTVisitor::TraverseTemplateArgumentLoc(A);
+  }
+
   bool VisitDecl(Decl *D) {
     visitNode(DynTypedNode::create(*D));
     return true;
   }
 
   // We have to use Traverse* because there is no corresponding Visit*.
   bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc L) {
     if (!L.getNestedNameSpecifier())
llvm::Optional<ReferenceLoc> explicitReference(DynTypedNode N) {
     // We do not have location information for other nodes (QualType, etc)
     return llvm::None;
   }
 
   void visitNode(DynTypedNode N) {
     auto Ref = explicitReference(N);
     if (!Ref)
       return;
+    reportReference(*Ref);
+  }
+
+  void reportReference(const ReferenceLoc &Ref) {
     // Our promise is to return only references from the source code. If we lack
     // location information, skip these nodes.
     // Normally this should not happen in practice, unless there are bugs in the
     // traversals or users started the traversal at an implicit node.
-    if (Ref->NameLoc.isInvalid()) {
-      dlog("invalid location at node {0}", nodeToString(N));
+    if (Ref.NameLoc.isInvalid())
       return;
-    }
-    Out(*Ref);
+    Out(Ref);
   }
 
   llvm::function_ref<void(ReferenceLoc)> Out;
   /// TypeLocs starting at these locations must be skipped, see
   /// TraverseElaboratedTypeSpecifierLoc for details.
   llvm::DenseSet</*SourceLocation*/ unsigned> TypeLocsToSkip;
 };
 } // namespace

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

std::pair</*Code*/ llvm::StringRef, /*References*/ llvm::StringRef> Cases[] =
           // Non-type template parameters.
           {R"cpp(
             template <int I>
             void foo() {
               int x = $0^I;
             }
         )cpp",
            "0: targets = {I}\n"},
+          // Template template parameters.
+          {R"cpp(
+            template <class T> struct vector {};
+
+            template <template<class> class TT, template<class> class ...TP>
+            void foo() {
+              $0^TT<int> x;
+              $1^foo<$2^TT>();
+              $3^foo<$4^vector>()
+              $5^foo<$6^TP...>();
+            }
+        )cpp",
+           "0: targets = {TT}\n"
+           "1: targets = {foo}\n"
+           "2: targets = {TT}\n"
+           "3: targets = {foo}\n"
+           "4: targets = {vector}\n"
+           "5: targets = {foo}\n"
+           "6: targets = {TP}\n"},
+          // Non-type template parameters with declarations.
+          {R"cpp(
+            int func();
+            template <int(*)()> struct wrapper {};
+
+            void foo() {
+              $0^wrapper<$1^func> w;
+            }
+        )cpp",
+           "0: targets = {wrapper<&func>}\n"
+           "1: targets = {func}\n"},
       };
 
   for (const auto &C : Cases) {
     llvm::StringRef ExpectedCode = C.first;
     llvm::StringRef ExpectedRefs = C.second;
 
     auto Actual =
         annotateReferencesInFoo(llvm::Annotations(ExpectedCode).code());
     EXPECT_EQ(ExpectedCode, Actual.AnnotatedCode);
     EXPECT_EQ(ExpectedRefs, Actual.DumpedReferences) << ExpectedCode;
   }
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang