[PATCH] D114287: [clangd] IncludeCleaner: Traverse unresolved expressions

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks for the fix!



================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:108
 
+  // Some expressions might be not resolved yet due to ADL: conservatively mark
+  // all resolution candidates as used.
----------------
The impl is correct but the comment is not.
The primary reason we haven't resolved the overload is simply that we have dependent arguments, and you need to know the types to resolve overloads. This is true even if ADL doesn't apply (usually because the name is qualified).

If ADL applies, this heuristic isn't conservative: a function in *any* namespace with the right name is a candidate, and we can't identify them all here.
However if the template expects to find the callee via ADL, then the callee is quite likely not even visible here and the instantiation site rather than the template is the real user of it, so there's nothing for us to do here.

(We're going to have a hard time identifying such cases at the instantiation site, but that's the way it goes)


================
Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:101
+      {
+          "template <typename T> void ^foo(T &&F) {}",
+          "template <typename U> void bar() { foo([](){}); }",
----------------
Neither the lambda or the foo template are essential here.
Maybe clearer:
```
int ^foo(char);
int ^foo(float);
template<class T> int x = foo(T{});
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114287