From e98443e62f3e4babe81953165389072ee1780fce Mon Sep 17 00:00:00 2001 From: Bolshakov Date: Fri, 29 Mar 2024 05:19:34 +0300 Subject: [PATCH] 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 #636, #823, #1111, #1293, and, partially, #1250. --- iwyu.cc | 41 ++++++++++++++++++++++++++++---- tests/cxx/overload_expr-d1.h | 31 ++++++++++++++++++++++++ tests/cxx/overload_expr-d2.h | 39 ++++++++++++++++++++++++++++++ tests/cxx/overload_expr-i1.h | 13 ++++++++++ tests/cxx/overload_expr-i2.h | 12 ++++++++++ tests/cxx/overload_expr-i3.h | 10 ++++++++ tests/cxx/overload_expr.cc | 39 ++++++++++++++++++++++++++++++ tests/cxx/overload_expr.imp | 4 ++++ tests/cxx/using_overload-float.h | 4 ++++ tests/cxx/using_overload-int.h | 4 ++++ 10 files changed, 192 insertions(+), 5 deletions(-) create mode 100644 tests/cxx/overload_expr-d1.h create mode 100644 tests/cxx/overload_expr-d2.h create mode 100644 tests/cxx/overload_expr-i1.h create mode 100644 tests/cxx/overload_expr-i2.h create mode 100644 tests/cxx/overload_expr-i3.h create mode 100644 tests/cxx/overload_expr.cc create mode 100644 tests/cxx/overload_expr.imp diff --git a/iwyu.cc b/iwyu.cc index cac1b7e04..75050a368 100644 --- a/iwyu.cc +++ b/iwyu.cc @@ -2200,12 +2200,11 @@ class IwyuBaseAstVisitor : public BaseAstVisitor { // 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 '#include'd 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. @@ -2213,11 +2212,43 @@ class IwyuBaseAstVisitor : public BaseAstVisitor { } 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 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; diff --git a/tests/cxx/overload_expr-d1.h b/tests/cxx/overload_expr-d1.h new file mode 100644 index 000000000..72689aa3f --- /dev/null +++ b/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 +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 */ diff --git a/tests/cxx/overload_expr-d2.h b/tests/cxx/overload_expr-d2.h new file mode 100644 index 000000000..56b459c64 --- /dev/null +++ b/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 +T TplFn2() { + // IWYU: ns::add is...*using_overload-int.h + return ns::add(T{}, T{}); +} + +template +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 */ diff --git a/tests/cxx/overload_expr-i1.h b/tests/cxx/overload_expr-i1.h new file mode 100644 index 000000000..2321489e7 --- /dev/null +++ b/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" diff --git a/tests/cxx/overload_expr-i2.h b/tests/cxx/overload_expr-i2.h new file mode 100644 index 000000000..5c197467b --- /dev/null +++ b/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); diff --git a/tests/cxx/overload_expr-i3.h b/tests/cxx/overload_expr-i3.h new file mode 100644 index 000000000..cd1977cf3 --- /dev/null +++ b/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); diff --git a/tests/cxx/overload_expr.cc b/tests/cxx/overload_expr.cc new file mode 100644 index 000000000..9fd2d9162 --- /dev/null +++ b/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(); + // IWYU: OverloadedFn is...*overload_expr-i2.h + TplFn3(); +} + +/**** 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 */ diff --git a/tests/cxx/overload_expr.imp b/tests/cxx/overload_expr.imp new file mode 100644 index 000000000..6ebd6964e --- /dev/null +++ b/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"] }, +] diff --git a/tests/cxx/using_overload-float.h b/tests/cxx/using_overload-float.h index 49c83da38..e1513b343 100644 --- a/tests/cxx/using_overload-float.h +++ b/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_ diff --git a/tests/cxx/using_overload-int.h b/tests/cxx/using_overload-int.h index 857e4c586..01c78e702 100644 --- a/tests/cxx/using_overload-int.h +++ b/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_