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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

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



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


================
Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:89
+       "template <template <typename> class T> class X {}; X<A> x;"},
+      {"namespace ns { template<typename T> struct ^A { ^A(T); }; } using "
+       "ns::^A;",
----------------
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>`.
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123212