[clangd] Do not duplicate TemplatedDecls in findExplicitReferences

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

[kadircet]

• Rebase

[hokein]

clang-tools-extra/clangd/FindTarget.cpp

589

the code this is correct right now (since {Class,Function,Var,TypeAlias}TemplateDecls are the decls that derive from RedeclarableTemplateDecl), but I would do this in another way -- just add a filter in the VisitNamedDecl below, like

void VisitNamedDecl(const NamedDecl *ND) {
   ...
   if (ND is one of the  {Class,Function,Var,TypeAlias}TemplateDecls)
     return;
}

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

755

nit: the diff seems not to be the one I have submitted, there should be a FIXME here which should be removed in this patch.

[kadircet]

clang-tools-extra/clangd/FindTarget.cpp

589

these two have a slightly different behaviour though, I deliberately chose to stop this in here.

Eliminating here results in dropping template decl, the one containing template parameters.
Whereas eliminating inside VisitNamedDecl, would result in dropping the described template, the real definition coming after template parameters.

It is always possible to switch between the two using get{TemplatedDecl,DescribedTemplate} but I think eliminating the templatedecl aligns better with
current api, since at the specified position we actually have the DescribedTemplate, not the TemplatedDecl and template parameters are already being
reported independent from this.

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

755

yeah it wasn't rebased

[kadircet]

clang-tools-extra/clangd/FindTarget.cpp

589

yeah actually re-thinking about it they will have the same affect since we can check for specific types, nvm this comment.

[kadircet]

• Filter out templateddecls in VisitNamedDecls, instead of defining a new Visit method.

[merge_guards_bot]

Unit tests: pass. 62057 tests passed, 0 failed and 784 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[merge_guards_bot]

Unit tests: pass. 62057 tests passed, 0 failed and 784 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[hokein]

clang-tools-extra/clangd/FindTarget.cpp

590

It took me a while to understand this comment, I think not everyone is very familiar with TemplatedDecls and DescribedTemplates terms, how about?

Avoid reporting references from {Class,Function,Var,TypeAlias}TemplateDecls to avoid duplicated results, as we will report references from their underlying decls which have the same range.

593

I would check it more elaborately (to reflect the comment above).

if (llvm::isa<ClassTemplateDecl>(ND) || llvm::isa<FunctionTemplateDecl>(ND)..)
  return

or using ND->getKind().

[kadircet]

• Update comments

[merge_guards_bot]

Unit tests: pass. 62094 tests passed, 0 failed and 785 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

clang-tools-extra/clangd/FindTarget.cpp

void VisitNamespaceAliasDecl(const NamespaceAliasDecl *D) {
       // Add a non-declaration reference for Target.
       Refs.push_back(ReferenceLoc{D->getQualifierLoc(),
                                   D->getTargetNameLoc(),
                                   /*IsDecl=*/false,
                                   {D->getAliasedNamespace()}});
     }
 
     void VisitNamedDecl(const NamedDecl *ND) {
+      // We choose to ignore {Class, Function, Var, TypeAlias}TemplateDecls. As
+      // as their underlying decls, covering the same range, will be visited.
+      if (llvm::isa<ClassTemplateDecl>(ND) ||
+          llvm::isa<FunctionTemplateDecl>(ND) ||
+          llvm::isa<VarTemplateDecl>(ND) ||
+          llvm::isa<TypeAliasTemplateDecl>(ND))
+        return;
       // FIXME: decide on how to surface destructors when we need them.
       if (llvm::isa<CXXDestructorDecl>(ND))
         return;
       // Filter anonymous decls, name location will point outside the name token
       // and the clients are not prepared to handle that.
       if (ND->getDeclName().isIdentifier() &&
           !ND->getDeclName().getAsIdentifierInfo())
         return;

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

std::pair</*Code*/ llvm::StringRef, /*References*/ llvm::StringRef> Cases[] =
         "0: targets = {valias}\n"
         "1: targets = {vi}, decl\n"
         "2: targets = {valias}\n"
         "3: targets = {vb}, decl\n"},
        // Injected class name.
        {R"cpp(
             namespace foo {
               template <typename $0^T>
-              class $1^$2^Bar {
-                ~$3^Bar();
-                void $4^f($5^Bar);
+              class $1^Bar {
+                ~$2^Bar();
+                void $3^f($4^Bar);
               };
             }
           )cpp",
         "0: targets = {foo::Bar::T}, decl\n"
-        // FIXME: avoid the 2 duplicated foo::Bar references below, the first
-        // one comes from ClassTemplateDecl; the second comes from the
-        // underlying CXXRecordDecl.
         "1: targets = {foo::Bar}, decl\n"
-        "2: targets = {foo::Bar}, decl\n"
-        "3: targets = {foo::Bar}\n"
-        "4: targets = {foo::Bar::f}, decl\n"
-        "5: targets = {foo::Bar}\n"},
+        "2: targets = {foo::Bar}\n"
+        "3: targets = {foo::Bar::f}, decl\n"
+        "4: targets = {foo::Bar}\n"},
        // MemberExpr should know their using declaration.
        {R"cpp(
             struct X { void func(int); };
             struct Y : X {
               using X::func;
             };
             void foo(Y y) {
               $0^y.$1^func(1);
std::pair</*Code*/ llvm::StringRef, /*References*/ llvm::StringRef> Cases[] =
                 struct Test {
                 Test(T);
               };
               void foo() {
                 $0^Test $1^a(5);
               }
             )cpp",
            "0: targets = {Test}\n"
-           "1: targets = {a}, decl\n"}};
+           "1: targets = {a}, decl\n"},
+       // Templates
+       {R"cpp(
+            namespace foo {
+              template <typename $0^T>
+              class $1^Bar {};
+            }
+          )cpp",
+        "0: targets = {foo::Bar::T}, decl\n"
+        "1: targets = {foo::Bar}, decl\n"},
+       // Templates
+       {R"cpp(
+            namespace foo {
+              template <typename $0^T>
+              void $1^func();
+            }
+          )cpp",
+        "0: targets = {T}, decl\n"
+        "1: targets = {foo::func}, decl\n"},
+       // Templates
+       {R"cpp(
+            namespace foo {
+              template <typename $0^T>
+              $1^T $2^x;
+            }
+          )cpp",
+        "0: targets = {foo::T}, decl\n"
+        "1: targets = {foo::T}\n"
+        "2: targets = {foo::x}, decl\n"},
+       // Templates
+       {R"cpp(
+            template<typename T> class vector {};
+            namespace foo {
+              template <typename $0^T>
+              using $1^V = $2^vector<$3^T>;
+            }
+          )cpp",
+        "0: targets = {foo::T}, decl\n"
+        "1: targets = {foo::V}, decl\n"
+        "2: targets = {vector}\n"
+        "3: targets = {foo::T}\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