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