This is an archive of the discontinued LLVM Phabricator instance.

[clangd] IncludeCleaner: Mark possible expr resolutions as used
ClosedPublic

Authored by kbobyrev on Nov 19 2021, 2:22 PM.

Diff Detail

Event Timeline

kbobyrev created this revision.Nov 19 2021, 2:22 PM
kbobyrev requested review of this revision.Nov 19 2021, 2:22 PM
kbobyrev edited the summary of this revision. (Show Details)Nov 19 2021, 2:23 PM
sammccall accepted this revision.Nov 19 2021, 2:38 PM

Thanks for the fix!

clang-tools-extra/clangd/IncludeCleaner.cpp
108

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)

clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
101

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{});
This revision is now accepted and ready to land.Nov 19 2021, 2:38 PM
kbobyrev updated this revision to Diff 388816.Nov 22 2021, 1:41 AM
kbobyrev marked 2 inline comments as done.

Address review comments.

kbobyrev retitled this revision from [clangd] IncludeCleaner: Traverse unresolved expressions to [clangd] IncludeCleaner: Mark possible expr resolutions as used.Nov 22 2021, 1:44 AM
This revision was landed with ongoing or failed builds.Nov 22 2021, 1:44 AM
This revision was automatically updated to reflect the committed changes.