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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
