[clangd] IncludeCleaner: Mark possible expr resolutions as used

[kbobyrev]

Summary

Fixes: https://github.com/clangd/clangd/issues/934

[sammccall]

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{});

[kbobyrev]

Address review comments.

clang-tools-extra/clangd/IncludeCleaner.cpp

 //===--- IncludeCleaner.cpp - Unused/Missing Headers Analysis ---*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "IncludeCleaner.h"
 #include "Config.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 
public:
   // an underlying type. In this case, the definition should see the declaration
   // so they can be checked for compatibility.
   bool VisitEnumDecl(EnumDecl *D) {
     if (D->isThisDeclarationADefinition() && D->getIntegerTypeSourceInfo())
       add(D);
     return true;
   }
 
+  // Some expressions might be not resolved yet due to ADL: conservatively mark
+  // all resolution candidates as used.
+  bool VisitOverloadExpr(OverloadExpr *E) {
+    for (const auto *ResolutionDecl : E->decls())
+      add(ResolutionDecl);
+    return true;
+  }
+
 private:
   using Base = RecursiveASTVisitor<ReferencedLocationCrawler>;
 
   void add(const Decl *D) {
     if (!D || !isNew(D->getCanonicalDecl()))
       return;
     for (const Decl *Redecl : D->redecls())
       Result.insert(Redecl->getLocation());

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

TestCase Cases[] = {
       {
           "void foo() {}",
           "void foo();",
       },
       {
           "inline void ^foo() {}",
           "void bar() { foo(); }",
       },
+      {
+          "template <typename T> void ^foo(T &&F) {}",
+          "template <typename U> void bar() { foo([](){}); }",
+      },
       // Static function
       {
           "struct ^X { static bool ^foo(); }; bool X::^foo() {}",
           "auto b = X::foo();",
       },
       // TemplateRecordDecl
       {
           "template <typename> class ^X;",