[clangd] Handle the new UsingTemplateName.

[hokein]

Summary

Add supports in FindTarget and IncludeCleaner. This would
improve AST-based features on a tempalte which is found via a using
declaration. For example, go-to-def on vect^or<int> v; gives us the
location of using std::vector, which was not previously.

Base on https://reviews.llvm.org/D123127

[kadircet]

thanks a lot for doing this! some drive-by comments

clang-tools-extra/clangd/IncludeCleaner.cpp

91

what's the reason for not doing this in base method instead?

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

90

a note for future:
we probably don't want to consider references to constructor and the underlying decl when figuring out missing includes. as it would imply insertion of underlying header, which might be annoying.
as an example, code patterns in the wild look like:
foo.h:

#include <optional>
namespace famous_library_name {
using std::optional;
}

foo.cc

#include "foo.h"
void foo() {
  famous_library_name::optional x(3); // we were getting complaints around saying `foo.h` is unused, i bet we'll get similar complaints if we said you should insert `<optional>`.
}

[sammccall]

clang-tools-extra/clangd/FindTarget.cpp

449

I don't think this really belongs in the else-if chain, if the previous catch matches (known class specialization) then we still want to recognize the alias.

I suspect this probably belongs right at the top, outside the chain

clang-tools-extra/clangd/IncludeCleaner.cpp

82

TraverseTemplateSpecializationType calls TraverseTemplateName, isn't this redundant with below?

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

90

this wrapping is weird, and the amount of code on one line is hard to read, switch to raw-string?

90

we probably don't want to consider references

Yeah, this is "do member accesses count as references" which we've identified as an important policy knob to add to IWYU-as-a-library

imply insertion

we'll probably replace this impl with said library before starting to offer insertions.

(and for now, the behavior in this test matches the way we currently treat constructors elsewhere)

[hokein]

rebase

[hokein]

clang-tools-extra/clangd/FindTarget.cpp

449

oh, you're right.

clang-tools-extra/clangd/IncludeCleaner.cpp

82

yeah, the add(..FoundDecl) is redundant, but the key point here we do an early return, which means we will not traverse the underlying template decl if this template decl is found via the using decl.

I think this is a policy problem of whether we count the underlying decl as a reference for include-cleaner -- currently I just keep it consistent with the UsingType.

91

Traversing the underlying decl doesn't seem to follow the existing principle of RAV (e.g. the underlying TemplateDecl of TemplateName is not traversed neighter), the base method merely traverses written qualifiers of templateName if any.

[sammccall]

clang-tools-extra/clangd/IncludeCleaner.cpp

82

Makes sense.

But if we're creating a pseudo-VisitTemplateName by overriding its traverse function, can we move the add(getTemplateName() .getAsTemplateDecl()) logic there too?

[sammccall]

(Would be great to land this in some form if you get a chance!)

[hokein]

In D123212#3461557, @sammccall wrote:

(Would be great to land this in some form if you get a chance!)

sure, I plan to land it today.

[hokein]

implement a pseudo-VisitTemplateName locally.

[sammccall]

clang-tools-extra/clangd/IncludeCleaner.cpp

82–84

maybe instead:
"Use of a template through an alias only uses the alias (see VisitTemplateName), not the underlying template"

clang-tools-extra/clangd/FindTarget.cpp

struct Visitor : public TypeVisitor<Visitor> {
       }
       void VisitDeducedType(const DeducedType *DT) {
         // FIXME: In practice this doesn't work: the AutoType you find inside
         // TypeLoc never has a deduced type. https://llvm.org/PR42914
         Outer.add(DT->getDeducedType(), Flags);
       }
       void VisitDeducedTemplateSpecializationType(
           const DeducedTemplateSpecializationType *DTST) {
+        if (const auto *USD = DTST->getTemplateName().getAsUsingShadowDecl())
+          Outer.add(USD, Flags);
+
         // FIXME: This is a workaround for https://llvm.org/PR42914,
         // which is causing DTST->getDeducedType() to be empty. We
         // fall back to the template pattern and miss the instantiation
         // even when it's known in principle. Once that bug is fixed,
-        // this method can be removed (the existing handling in
+        // the following code can be removed (the existing handling in
         // VisitDeducedType() is sufficient).
         if (auto *TD = DTST->getTemplateName().getAsTemplateDecl())
           Outer.add(TD->getTemplatedDecl(), Flags | Rel::TemplatePattern);
       }
       void VisitDependentNameType(const DependentNameType *DNT) {
         if (Outer.Resolver) {
           for (const NamedDecl *ND :
                Outer.Resolver->resolveDependentNameType(DNT)) {
struct Visitor : public TypeVisitor<Visitor> {
         if (shouldSkipTypedef(TT->getDecl()))
           return;
         Outer.add(TT->getDecl(), Flags);
       }
       void
       VisitTemplateSpecializationType(const TemplateSpecializationType *TST) {
         // Have to handle these case-by-case.
 
+        if (const auto *UTN = TST->getTemplateName().getAsUsingShadowDecl())
+          Outer.add(UTN, Flags);
+
         // templated type aliases: there's no specialized/instantiated using
         // decl to point to. So try to find a decl for the underlying type
         // (after substitution), and failing that point to the (templated) using
         // decl.
         if (TST->isTypeAlias()) {
           Outer.add(TST->getAliasedType(), Flags | Rel::Underlying);
           // Don't *traverse* the alias, which would result in traversing the
           // template of the underlying type.
           Outer.report(
               TST->getTemplateName().getAsTemplateDecl()->getTemplatedDecl(),
               Flags | Rel::Alias | Rel::TemplatePattern);
         }
         // specializations of template template parameters aren't instantiated
         // into decls, so they must refer to the parameter itself.
         else if (const auto *Parm =
                      llvm::dyn_cast_or_null<TemplateTemplateParmDecl>(
                          TST->getTemplateName().getAsTemplateDecl()))
           Outer.add(Parm, Flags);
         // class template specializations have a (specialized) CXXRecordDecl.
         else if (const CXXRecordDecl *RD = TST->getAsCXXRecordDecl())
           Outer.add(RD, Flags); // add(Decl) will despecialize if needed.
         else {
           // fallback: the (un-specialized) declaration from primary template.
           if (auto *TD = TST->getTemplateName().getAsTemplateDecl())
             Outer.add(TD->getTemplatedDecl(), Flags | Rel::TemplatePattern);
         }
       }
       void VisitTemplateTypeParmType(const TemplateTypeParmType *TTPT) {
         Outer.add(TTPT->getDecl(), Flags);
       }
void add(const TemplateArgument &Arg, RelSet Flags) {
     // will hit a more specific node (e.g. a TypeLoc or a
     // DeclRefExpr).
     if (Arg.getKind() == TemplateArgument::Template ||
         Arg.getKind() == TemplateArgument::TemplateExpansion) {
       if (TemplateDecl *TD =
               Arg.getAsTemplateOrTemplatePattern().getAsTemplateDecl()) {
         report(TD, Flags);
       }
+      if (const auto *USD =
+              Arg.getAsTemplateOrTemplatePattern().getAsUsingShadowDecl())
+        add(USD, Flags);
     }
   }
 };
 
 } // namespace
 
 llvm::SmallVector<std::pair<const NamedDecl *, DeclRelationSet>, 1>
 allTargetDecls(const DynTypedNode &N, const HeuristicResolver *Resolver) {

clang-tools-extra/clangd/IncludeCleaner.cpp

public:
   }
 
   bool VisitCXXConstructExpr(CXXConstructExpr *CCE) {
     add(CCE->getConstructor());
     return true;
   }
 
   bool VisitTemplateSpecializationType(TemplateSpecializationType *TST) {
-    add(TST->getTemplateName().getAsTemplateDecl()); // Primary template.
+    // Using templateName case is handled by the override TraverseTemplateName.
+    if (TST->getTemplateName().getKind() == TemplateName::UsingTemplate)
+      return true;
     add(TST->getAsCXXRecordDecl());                  // Specialization
     return true;
   }
 
+  // There is no VisitTemplateName in RAV, thus we override the Traverse version
+  // to handle the Using TemplateName case.
+  bool TraverseTemplateName(TemplateName TN) {
+    VisitTemplateName(TN);
+    return Base::TraverseTemplateName(TN);
+  }
+  // A pseudo VisitTemplateName, dispatched by the above TraverseTemplateName!
+  bool VisitTemplateName(TemplateName TN) {
+    if (const auto *USD = TN.getAsUsingShadowDecl()) {
+      add(USD);
+      return true;
+    }
+    add(TN.getAsTemplateDecl()); // Primary template.
+    return true;
+  }
+
   bool VisitUsingType(UsingType *UT) {
     add(UT->getFoundDecl());
     return true;
   }
 
   bool VisitTypedefType(TypedefType *TT) {
     add(TT->getDecl());
     return true;

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

Code = R"cpp(
       };
       template <typename T>
       struct Derived : Base<T> {
         using Base<T>::[[waldo]];
       };
     )cpp";
   EXPECT_DECLS("UnresolvedUsingValueDecl", {"using Base<T>::waldo", Rel::Alias},
                {"void waldo()"});
+
+  Code = R"cpp(
+    namespace ns {
+    template<typename T> class S {};
+    }
+
+    using ns::S;
+
+    template<typename T>
+    using A = [[S]]<T>;
+  )cpp";
+  EXPECT_DECLS("TemplateSpecializationTypeLoc", {"using ns::S", Rel::Alias},
+               {"template <typename T> class S"},
+               {"class S", Rel::TemplatePattern});
+
+  Code = R"cpp(
+    namespace ns {
+    template<typename T> class S {};
+    }
+
+    using ns::S;
+    template <template <typename> class T> class X {};
+    using B = X<[[S]]>;
+  )cpp";
+  EXPECT_DECLS("TemplateArgumentLoc", {"using ns::S", Rel::Alias},
+               {"template <typename T> class S"});
+
+  Code = R"cpp(
+    namespace ns {
+    template<typename T> class S { public: S(T); };
+    }
+
+    using ns::S;
+    [[S]] s(123);
+  )cpp";
+  Flags.push_back("-std=c++17"); // For CTAD feature.
+  EXPECT_DECLS("DeducedTemplateSpecializationTypeLoc",
+               {"using ns::S", Rel::Alias}, {"template <typename T> class S"},
+               {"class S", Rel::TemplatePattern});
 }
 
 TEST_F(TargetDeclTest, BaseSpecifier) {
   Code = R"cpp(
     struct X {};
     struct Y : [[private]] X {};
   )cpp";
   EXPECT_DECLS("CXXBaseSpecifier", "struct X");

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

TestCase Cases[] = {
           "namespace ns { void ^foo(); void ^foo() {} }",
           "using ns::foo;",
       },
       {
           "namespace ns { void foo(); void foo() {}; }",
           "using namespace ns;",
       },
       {
+          // Refs from UsingTypeLoc and implicit constructor!
           "struct ^A {}; using B = A; using ^C = B;",
           "C a;",
       },
+      {"namespace ns { template<typename T> class A {}; } using ns::^A;",
+       "A<int>* a;"},
+      {"namespace ns { template<typename T> class A {}; } using ns::^A;",
+       R"cpp(
+          template <template <typename> class T> class X {};
+          X<A> x;
+        )cpp"},
+      {R"cpp(
+          namespace ns { template<typename T> struct ^A { ^A(T); }; }
+          using ns::^A;
+       )cpp",
+       "A CATD(123);"},
       {
           "typedef bool ^Y; template <typename T> struct ^X {};",
           "X<Y> x;",
       },
       {
           // https://github.com/clangd/clangd/issues/1036
           R"cpp(
             struct ^Base { void ^base(); };
TestCase Cases[] = {
           "Color *c;",
       },
   };
   for (const TestCase &T : Cases) {
     TestTU TU;
     TU.Code = T.MainCode;
     Annotations Header(T.HeaderCode);
     TU.HeaderCode = Header.code().str();
+    TU.ExtraArgs.push_back("-std=c++17");
     auto AST = TU.build();
 
     std::vector<Position> Points;
     for (const auto &Loc : findReferencedLocations(AST).User) {
       if (AST.getSourceManager().getBufferName(Loc).endswith(
               TU.HeaderFilename)) {
         Points.push_back(offsetToPosition(
             TU.HeaderCode, AST.getSourceManager().getFileOffset(Loc)));