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

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 6 05:37:58 PDT 2022

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:
#include <optional>
namespace famous_library_name {
using std::optional;

#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>`.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list