[clangd] Handle template arguments in findExplicitReferences

[ilya-biryukov]

clang-tools-extra/clangd/FindTarget.cpp

576 ↗

(On Diff #222157)

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 ↗

(On Diff #222157)

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

567 ↗

(On Diff #222157)

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

618 ↗

(On Diff #222157)

can we keep dlog ?

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

751 ↗

(On Diff #222157)

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 ↗

(On Diff #222157)

Yes, exactly. Amended this to the comment.

567 ↗

(On Diff #222157)

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 ↗

(On Diff #222157)

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/trunk/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*.
+  // TemplateArgumentLoc is the only way to get locations for references to
+  // template template parameters.
+  bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) {
+    switch (A.getArgument().getKind()) {
+    case TemplateArgument::Template:
+    case TemplateArgument::TemplateExpansion:
+      reportReference(ReferenceLoc{A.getTemplateQualifierLoc(),
+                                   A.getTemplateNameLoc(),
+                                   {A.getArgument()
+                                        .getAsTemplateOrTemplatePattern()
+                                        .getAsTemplateDecl()}},
+                      DynTypedNode::create(A.getArgument()));
+      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 VisitType and VisitExpression.
+    };
+    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, N);
+  }
+
+  void reportReference(const ReferenceLoc &Ref, DynTypedNode N) {
     // 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()) {
+    if (Ref.NameLoc.isInvalid()) {
       dlog("invalid location at node {0}", nodeToString(N));
       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/trunk/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 {};
+
+            template <int(*FuncParam)()>
+            void foo() {
+              $0^wrapper<$1^func> w;
+              $2^FuncParam();
+            }
+        )cpp",
+           "0: targets = {wrapper<&func>}\n"
+           "1: targets = {func}\n"
+           "2: targets = {FuncParam}\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