Fix provision analysis with public headers

This fixes determination of types provided with type aliases and
function declarations (fn return, "autocast") in the presence of mapping
files. The problem was in that the `PublicHeaderIntendsToProvide`
function considers as provided all the headers transitively included
from a public header, despite they are may be themselves public and not
have the explicit public-to-public mapping specified. It is convenient
to avoid noisy suggestions of the library internals when handling
instantiations of some external library templates, but may hinder
expressing author intent when writing e.g. some own (user's) `typedef`
(non-)providing a type from such a library.

To achieve the more correct behavior, the logic from
`OneUse::SetPublicHeaders()` function has been reused which takes into
account only explicitly specified mappings. The drawback is that the use
site vs. the decl site difference for default template arguments becomes
worse (see `TestDefaultTplArgs()`). The use site handling still uses
`PublicHeaderIntendsToProvide` (which is common way for template
instantiation handling, as stated before). It may end up in that IWYU
may suggest to remove an indirectly providing header from the template
defining file (`-d1.h` from `-def_tpl_arg.h` in the test case)
not suggesting to `#include` the default argument header at the use site
because at the time of analysis `-d1.h` is present in `-def_tpl_arg.h`
and "provides" all its directly and indirectly included stuff. OTOH, it
may be fixed easily by means of including the appropriate header
in the template instantiating file manually, and the IWYU behavior
should become consistent.

The main goal of these changes is to introduce `IsDirectlyIncluded`
function which may be reused when fixing `UnresolvedLookupExpr`-related
bugs (`PublicHeaderIntendsToProvide` doesn't work well in real-world
cases for it).

iwyu.cc

@@ -1436,17 +1436,18 @@ class IwyuBaseAstVisitor : public BaseAstVisitor<Derived> {
       return false;
 
     // Check if the the author is not #including the file with the
-    // definition.  PublicHeaderIntendsToProvide has exactly the
-    // semantics we want.  Note if there's no definition anywhere, we
+    // definition. Note if there's no definition anywhere, we
     // say the author does not want the full type (which is a good
     // thing, since there isn't one!)
     if (const NamedDecl* dfn = GetTagDefinition(decl)) {
       if (IsBeforeInSameFile(dfn, use_loc))
         return false;
-      if (preprocessor_info().PublicHeaderIntendsToProvide(
-              GetFileEntry(use_loc), GetFileEntry(dfn))) {
+      const IwyuFileInfo* use_file =
+          preprocessor_info().FileInfoFor(GetFileEntry(use_loc));
+      if (!use_file)  // Not sure whether this is possible.
+        return true;
+      if (IsDirectlyIncluded(dfn, *use_file))
         return false;
-      }
     }
 
     // OK, looks like the author has stated they don't want the fulll type.

iwyu_output.cc

@@ -65,6 +65,7 @@ using llvm::dyn_cast;
 using llvm::errs;
 using llvm::isa;
 using llvm::raw_string_ostream;
+using std::find_if;
 using std::map;
 using std::multimap;
 using std::pair;
@@ -165,6 +166,19 @@ std::string PrintableUnderlyingType(const EnumDecl* enum_decl) {
   return std::string();
 }
 
+static vector<string> GetMappedPublicHeaders(const string& symbol_name,
+                                             const string& use_path,
+                                             const string& decl_filepath) {
+  const IncludePicker& picker = GlobalIncludePicker();  // short alias
+  // If the symbol has a special mapping, use it, otherwise map its file.
+  vector<string> symbol_headers =
+      picker.GetCandidateHeadersForSymbolUsedFrom(symbol_name, use_path);
+  if (!symbol_headers.empty())
+    return symbol_headers;
+  return picker.GetCandidateHeadersForFilepathIncludedFrom(decl_filepath,
+                                                           use_path);
+}
+
 }  // anonymous namespace
 
 FakeNamedDecl::FakeNamedDecl(const string& kind_name, const string& qual_name,
@@ -335,14 +349,8 @@ void OneUse::SetPublicHeaders() {
   // We should never need to deal with public headers if we already know
   // who we map to.
   CHECK_(suggested_header_.empty() && "Should not need a public header here");
-  const IncludePicker& picker = GlobalIncludePicker();   // short alias
-  const string use_path = GetFilePath(use_loc_);
-  // If the symbol has a special mapping, use it, otherwise map its file.
-  public_headers_ = picker.GetCandidateHeadersForSymbolUsedFrom(
-      symbol_name_, use_path);
-  if (public_headers_.empty())
-    public_headers_ = picker.GetCandidateHeadersForFilepathIncludedFrom(
-        decl_filepath(), use_path);
+  public_headers_ = internal::GetMappedPublicHeaders(
+      symbol_name_, GetFilePath(use_loc_), decl_filepath());
   if (public_headers_.empty())
     public_headers_.push_back(ConvertToQuotedInclude(decl_filepath()));
 }
@@ -2298,4 +2306,19 @@ size_t IwyuFileInfo::CalculateAndReportIwyuViolations() {
   return num_edits;
 }
 
+bool IsDirectlyIncluded(const NamedDecl* decl, const IwyuFileInfo& includer) {
+  const vector<string> mapped_headers = internal::GetMappedPublicHeaders(
+      internal::GetQualifiedNameAsString(decl),
+      GetFilePath(includer.file_entry()), GetFilePath(decl));
+  if (!mapped_headers.empty()) {
+    auto pred = [&direct_includes =
+                     includer.direct_includes()](const string& header) {
+      return direct_includes.count(header);
+    };
+    return find_if(mapped_headers.begin(), mapped_headers.end(), pred) !=
+           mapped_headers.end();
+  }
+  return includer.direct_includes_as_fileentries().count(GetFileEntry(decl));
+}
+
 }  // namespace include_what_you_use

iwyu_output.h

@@ -252,6 +252,14 @@ class IwyuFileInfo {
   }
   void set_pch_in_code() { is_pch_in_code_ = true; }
 
+  clang::OptionalFileEntryRef file_entry() const {
+    return file_;
+  }
+
+  const set<string>& direct_includes() const {
+    return direct_includes_;
+  }
+
   // An 'associated' header is a header that this file #includes
   // (possibly indirectly) that we should treat as being logically
   // part of this file.  In particular, when computing the direct
@@ -340,10 +348,6 @@ class IwyuFileInfo {
   size_t CalculateAndReportIwyuViolations();
 
  private:
-  const set<string>& direct_includes() const {
-    return direct_includes_;
-  }
-
   const set<string>& desired_includes() const {
     CHECK_(desired_includes_have_been_calculated_ &&
            "Must calculate desired includes before calling desired_includes()");
@@ -426,6 +430,10 @@ class IwyuFileInfo {
   bool desired_includes_have_been_calculated_;
 };
 
+// Returns true if a public header (i.e. with respect to mappings) for NamedDecl
+// is directly included.
+bool IsDirectlyIncluded(const clang::NamedDecl*, const IwyuFileInfo& includer);
+
 // Helpers for testing.
 
 namespace internal {

tests/cxx/iwyu_stricter_than_cpp-i1.h

@@ -46,3 +46,5 @@ struct AutocastInPartialSpec<T*> {
   AutocastInPartialSpec(int) {
   }
 };
+
+struct MappedToD1 {};

tests/cxx/iwyu_stricter_than_cpp-typedefs.h

@@ -33,6 +33,10 @@ typedef DirectStruct1 Includes;
 // Essentially the same; should require corresponding header inclusion here.
 typedef struct DirectStruct3 IncludesElaborated;
 
+// The same: the public header for MappedToD1 is directly included.
+struct MappedToD1;
+typedef MappedToD1 IncludesPublicHeader;
+
 // Requires the full type because it does not obey rules (1) *or* (2)
 typedef DirectStruct2 DoesNotForwardDeclareAndIncludes;
 
@@ -99,10 +103,11 @@ tests/cxx/iwyu_stricter_than_cpp-typedefs.h should add these lines:
 
 tests/cxx/iwyu_stricter_than_cpp-typedefs.h should remove these lines:
 - struct DirectStruct1;  // lines XX-XX
+- struct MappedToD1;  // lines XX-XX
 - template <typename T> struct TplDirectStruct1;  // lines XX-XX
 
 The full include-list for tests/cxx/iwyu_stricter_than_cpp-typedefs.h:
-#include "tests/cxx/iwyu_stricter_than_cpp-d1.h"  // for DirectStruct1, DirectStruct2, DirectStruct3, TplDirectStruct1, TplDirectStruct2
+#include "tests/cxx/iwyu_stricter_than_cpp-d1.h"  // for DirectStruct1, DirectStruct2, DirectStruct3, MappedToD1, TplDirectStruct1, TplDirectStruct2
 #include "tests/cxx/iwyu_stricter_than_cpp-d4.h"  // for TplDirectStruct7
 #include "tests/cxx/iwyu_stricter_than_cpp-i1.h"  // for IndirectStruct1, IndirectStructForwardDeclaredInD1, TplIndirectStruct1, TplIndirectStructForwardDeclaredInD1
 struct IndirectStruct2;  // lines XX-XX

tests/cxx/iwyu_stricter_than_cpp.cc

@@ -13,6 +13,7 @@
 //            -Xiwyu --check_also="tests/cxx/*-type_alias.h" \
 //            -Xiwyu --check_also="tests/cxx/*-def_tpl_arg.h" \
 //            -Xiwyu --check_also="tests/cxx/*-d2.h" \
+//            -Xiwyu --mapping_file=tests/cxx/iwyu_stricter_than_cpp.imp \
 //            -I .
 
 // There are a few scenarios where iwyu requires a full type but c++
@@ -86,6 +87,7 @@ void TestTypedefs() {
   DoesNotForwardDeclareProperly dnfdp(2);
   Includes i(3);
   IncludesElaborated ie(3);
+  IncludesPublicHeader ip;
   DoesNotForwardDeclareAndIncludes dnfdai(4);
   // IWYU: IndirectStruct2 is...*iwyu_stricter_than_cpp-i2.h
   DoesEverythingRight dor(5);
@@ -463,10 +465,11 @@ void TestDefaultTplArgs() {
   // regardless of fwd-decl presence or absence. Hence, 'IndirectStruct3' (which
   // is neither directly included nor fwd-declared in the template defn header)
   // is required both here and at the template definition side.
-  // 'IndirectStruct2' is required here because it is fwd-declared and not
-  // included directly in the template defining header.
+  // TODO: 'IndirectStruct2' should better be required here because it is
+  // fwd-declared and not included directly in the template defining header. But
+  // it doesn't because the public header "*-d1.h" includes it transitively,
+  // which means it is considered as provided in template instantiation context.
   // IWYU: IndirectStruct3 is...*iwyu_stricter_than_cpp-i3.h
-  // IWYU: IndirectStruct2 is...*iwyu_stricter_than_cpp-i2.h
   TplWithDefaultArgs<> t;
 }
 
@@ -505,7 +508,7 @@ The full include-list for tests/cxx/iwyu_stricter_than_cpp.cc:
 #include "tests/cxx/iwyu_stricter_than_cpp-i4.h"  // for IndirectStruct4
 #include "tests/cxx/iwyu_stricter_than_cpp-i5.h"  // for TplIndirectStruct3
 #include "tests/cxx/iwyu_stricter_than_cpp-type_alias.h"  // for DoesEverythingRightAl, DoesEverythingRightAlTpl, DoesNotForwardDeclareAl, DoesNotForwardDeclareAlTpl, DoesNotForwardDeclareAndIncludesAl, DoesNotForwardDeclareAndIncludesAlTpl, DoesNotForwardDeclareProperlyAl, IncludesAl, IncludesAlTpl, IncludesElaboratedAl, IndirectStruct3NonProvidingAl, IndirectStruct4NonProvidingAl, TemplateNotProvidedArgumentNotUsed, TemplateNotProvidedArgumentUsed, TemplateProvidedArgumentNotUsed, TemplateProvidedArgumentUsed, TplAllForwardDeclaredAl, TplAllNeededTypesProvidedAl, TplDoesEverythingRightAgainAl, TplDoesEverythingRightAl, TplDoesNotForwardDeclareAl, TplDoesNotForwardDeclareAndIncludesAl, TplDoesNotForwardDeclareProperlyAl, TplIncludesAl, TplOnlyArgumentTypeProvidedAl, TplOnlyTemplateProvidedAl
-#include "tests/cxx/iwyu_stricter_than_cpp-typedefs.h"  // for DoesEverythingRight, DoesNotForwardDeclare, DoesNotForwardDeclareAndIncludes, DoesNotForwardDeclareProperly, Includes, IncludesElaborated, IndirectStruct3NonProvidingTypedef, IndirectStruct4NonProvidingTypedef, TplAllForwardDeclared, TplAllNeededTypesProvided, TplDoesEverythingRight, TplDoesEverythingRightAgain, TplDoesNotForwardDeclare, TplDoesNotForwardDeclareAndIncludes, TplDoesNotForwardDeclareProperly, TplIncludes, TplOnlyArgumentTypeProvided, TplOnlyTemplateProvided
+#include "tests/cxx/iwyu_stricter_than_cpp-typedefs.h"  // for DoesEverythingRight, DoesNotForwardDeclare, DoesNotForwardDeclareAndIncludes, DoesNotForwardDeclareProperly, Includes, IncludesElaborated, IncludesPublicHeader, IndirectStruct3NonProvidingTypedef, IndirectStruct4NonProvidingTypedef, TplAllForwardDeclared, TplAllNeededTypesProvided, TplDoesEverythingRight, TplDoesEverythingRightAgain, TplDoesNotForwardDeclare, TplDoesNotForwardDeclareAndIncludes, TplDoesNotForwardDeclareProperly, TplIncludes, TplOnlyArgumentTypeProvided, TplOnlyTemplateProvided
 struct DirectStruct1;
 struct DirectStruct2;
 struct IndirectStruct1;

tests/cxx/iwyu_stricter_than_cpp.imp

@@ -0,0 +1,6 @@
+# The primary reason to add this file is to mark "-d1.h" header as public.
+# Public headers have special handling in IWYU.
+[
+    { "symbol": ["DirectStruct1", "private", "\"tests/cxx/iwyu_stricter_than_cpp-d1.h\"", "public"] },
+    { "symbol": ["MappedToD1", "private", "\"tests/cxx/iwyu_stricter_than_cpp-d1.h\"", "public"] },
+]