[PATCH] D123212: [clangd] Handle the new UsingTemplateName.

[Haojian Wu via Phabricator via cfe-commits]

hokein marked an inline comment as done.
hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:446
           Outer.add(RD, Flags); // add(Decl) will despecialize if needed.
+        else if (const auto *UTN =
+                     TST->getTemplateName().getAsUsingTemplateName())
----------------
sammccall wrote:
> 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
oh, you're right.


================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:76
   bool VisitTemplateSpecializationType(TemplateSpecializationType *TST) {
+    if (const auto *UTN = TST->getTemplateName().getAsUsingTemplateName()) {
+      add(UTN->getFoundDecl());
----------------
sammccall wrote:
> TraverseTemplateSpecializationType calls TraverseTemplateName, isn't this redundant with below?
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.



================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:87
+  // to handle the UsingTemplateName case.
+  bool TraverseTemplateName(TemplateName TN) {
+    if (const auto *UTN = TN.getAsUsingTemplateName())
----------------
kadircet wrote:
> what's the reason for not doing this in base method instead?
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. 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123212/new/

https://reviews.llvm.org/D123212