Keep directly included overloads

I've decided to report only that overloaded function decls which are
already directly `#include`d, as opposed to the suggestion
in the removed TODO, so as to avoid lots of unwanted `#include`s
in real-world STL cases (things like `std::begin` may be defined in many
headers). When no overload is included directly but the name is
qualified (i.e. can not be found via ADL), an arbitrary (the first one)
overloaded declaration is suggested to keep the code compilable.

Filtering of suitable overloads has not been done.

This fixes include-what-you-use#636, include-what-you-use#823, include-what-you-use#1111, include-what-you-use#1293, and, partially, include-what-you-use#1250.

iwyu.cc

@@ -2201,24 +2201,54 @@ class IwyuBaseAstVisitor : public BaseAstVisitor<Derived> {
   // an iwyu warning now, even without knowing the exact overload.
   // In that case, we store the fact we warned, so we won't warn again
   // when the template is instantiated.
-  // TODO(csilvers): to be really correct, we should report *every*
-  // overload that callers couldn't match via ADL.
+  // Otherwise, all directly included overloads are reported just to be kept.
   bool VisitUnresolvedLookupExpr(UnresolvedLookupExpr* expr) {
     // No CanIgnoreCurrentASTNode() check here!  It's later in the function.
 
-    // Make sure all overloads are in the same file.
     if (expr->decls_begin() == expr->decls_end()) {
       // This can occur when there are no overloads before the template
       // definition, and a callee may be found only via ADL.
       return true;
     }
     const NamedDecl* first_decl = *expr->decls_begin();
     OptionalFileEntryRef first_decl_file_entry = GetFileEntry(first_decl);
+    OptionalFileEntryRef current_file_entry = GetFileEntry(CurrentLoc());
+    const IwyuFileInfo* current_file_info =
+        preprocessor_info().FileInfoFor(current_file_entry);
+    if (!current_file_info)
+      return true;
+    vector<const NamedDecl*> directly_included;
+    bool same_file = true;
     for (const NamedDecl* decl : expr->decls()) {
-      if (GetFileEntry(decl) != first_decl_file_entry)
-        return true;
+      // TODO(bolshakov): filter overloads suitable by number and type
+      // of parameters. Take into account default arguments, ellipsis, explicit
+      // template arguments, and pack expansions. See
+      // clang::Sema::AddOverloadedCallCandidates for guidance.
+      if (IsDirectlyIncluded(decl, *current_file_info))
+        directly_included.push_back(decl);
+      same_file &= GetFileEntry(decl) == first_decl_file_entry;
     }
 
+    if (!same_file) {
+      if (CanIgnoreCurrentASTNode())
+        return true;
+      if (!directly_included.empty()) {
+        for (const NamedDecl* decl : directly_included)
+          ReportDeclUse(CurrentLoc(), decl);
+        // No AddProcessedOverloadLoc here: PublicHeaderIntendsToProvide should
+        // already work well at the instantiation site.
+      } else if (!expr->requiresADL()) {
+        // Report any of the decls so as to keep the code compilable. If ADL is
+        // to be applied (i.e. the function name is unqualified), it is allowed
+        // to have no declaration.
+        ReportDeclUse(CurrentLoc(), first_decl);
+        // No AddProcessedOverloadLoc here: even first_decl may be different
+        // when scanning different translation units. It's better to
+        // double-report it on template-defn and instantiation sites than not
+        // to report an actually used decl at all.
+      }
+      return true;
+    }
     // For now, we're only worried about function calls.
     // TODO(csilvers): are there other kinds of overloads we need to check?
     const FunctionDecl* arbitrary_fn_decl = nullptr;

tests/cxx/overload_expr-d1.h

@@ -0,0 +1,31 @@
+//===--- overload_expr-d1.h - test input file for iwyu --------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "tests/cxx/overload_expr-i1.h"
+#include "tests/cxx/using_overload-int.h"
+
+// 'using_overload-int.h' should be kept because it is directly included
+// (in contrast to '...-float.h').
+
+template <typename T>
+T TplFn1() {
+  return ns::add(T{}, T{});
+}
+
+/**** IWYU_SUMMARY
+
+tests/cxx/overload_expr-d1.h should add these lines:
+
+tests/cxx/overload_expr-d1.h should remove these lines:
+- #include "tests/cxx/overload_expr-i1.h"  // lines XX-XX
+
+The full include-list for tests/cxx/overload_expr-d1.h:
+#include "tests/cxx/using_overload-int.h"  // for add
+
+***** IWYU_SUMMARY */

tests/cxx/overload_expr-d2.h

@@ -0,0 +1,39 @@
+//===--- overload_expr-d2.h - test input file for iwyu --------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "tests/cxx/overload_expr-i1.h"
+
+// IWYU should report any of the files declaring 'ns::add' because it is
+// a qualified name.
+
+template <typename T>
+T TplFn2() {
+  // IWYU: ns::add is...*using_overload-int.h
+  return ns::add(T{}, T{});
+}
+
+template <typename T>
+void TplFn3() {
+  // It is allowed not to include any definition of 'OverloadedFn' at this
+  // point.
+  return OverloadedFn(T{});
+}
+
+/**** IWYU_SUMMARY
+
+tests/cxx/overload_expr-d2.h should add these lines:
+#include "tests/cxx/using_overload-int.h"
+
+tests/cxx/overload_expr-d2.h should remove these lines:
+- #include "tests/cxx/overload_expr-i1.h"  // lines XX-XX
+
+The full include-list for tests/cxx/overload_expr-d2.h:
+#include "tests/cxx/using_overload-int.h"  // for add
+
+***** IWYU_SUMMARY */

tests/cxx/overload_expr-i1.h

@@ -0,0 +1,13 @@
+//===--- overload_expr-i1.h - test input file for iwyu --------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "tests/cxx/using_overload-int.h"
+#include "tests/cxx/using_overload-float.h"
+#include "tests/cxx/overload_expr-i2.h"
+#include "tests/cxx/overload_expr-i3.h"

tests/cxx/overload_expr-i2.h

@@ -0,0 +1,12 @@
+//===--- overload_expr-i2.h - test input file for iwyu --------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+struct A;
+
+void OverloadedFn(A);

tests/cxx/overload_expr-i3.h

@@ -0,0 +1,10 @@
+//===--- overload_expr-i3.h - test input file for iwyu --------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+void OverloadedFn(char);

tests/cxx/overload_expr.cc

@@ -0,0 +1,39 @@
+//===--- overload_expr.cc - test input file for iwyu ----------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// IWYU_ARGS: -Xiwyu --check_also="tests/cxx/overload_expr-d?.h" \
+//            -Xiwyu --mapping_file=tests/cxx/overload_expr.imp \
+//            -I .
+
+// Tests various cases of unresolved lookup expression handling.
+
+#include "tests/cxx/overload_expr-d1.h"
+#include "tests/cxx/overload_expr-d2.h"
+
+struct A {};
+
+int main() {
+  TplFn1<int>();
+  // IWYU: OverloadedFn is...*overload_expr-i2.h
+  TplFn3<A>();
+}
+
+/**** IWYU_SUMMARY
+
+tests/cxx/overload_expr.cc should add these lines:
+#include "tests/cxx/overload_expr-i2.h"
+
+tests/cxx/overload_expr.cc should remove these lines:
+
+The full include-list for tests/cxx/overload_expr.cc:
+#include "tests/cxx/overload_expr-d1.h"  // for TplFn1
+#include "tests/cxx/overload_expr-d2.h"  // for TplFn3
+#include "tests/cxx/overload_expr-i2.h"  // for OverloadedFn
+
+***** IWYU_SUMMARY */

tests/cxx/overload_expr.imp

@@ -0,0 +1,4 @@
+# The presence of the mapping file should not spoil IWYU analysis.
+[
+    { "symbol": ["TplFn1", "private", "\"tests/cxx/overload_expr-d1.h\"", "public"] },
+]

tests/cxx/using_overload-float.h

@@ -7,7 +7,11 @@
 //
 //===----------------------------------------------------------------------===//
 
+#ifndef INCLUDE_WHAT_YOU_USE_TESTS_CXX_USING_OVERLOAD_FLOAT_H_
+#define INCLUDE_WHAT_YOU_USE_TESTS_CXX_USING_OVERLOAD_FLOAT_H_
+
 namespace ns {
   float add(float, float);
 }
 
+#endif  // INCLUDE_WHAT_YOU_USE_TESTS_CXX_USING_OVERLOAD_FLOAT_H_

tests/cxx/using_overload-int.h

@@ -7,7 +7,11 @@
 //
 //===----------------------------------------------------------------------===//
 
+#ifndef INCLUDE_WHAT_YOU_USE_TESTS_CXX_USING_OVERLOAD_INT_H_
+#define INCLUDE_WHAT_YOU_USE_TESTS_CXX_USING_OVERLOAD_INT_H_
+
 namespace ns {
   int add(int, int);
 }
 
+#endif  // INCLUDE_WHAT_YOU_USE_TESTS_CXX_USING_OVERLOAD_INT_H_