Preprocessor: Suppress -Wnonportable-include-path for header maps

[dexonsmith]

Summary

If a file search involves a header map, suppress
-Wnonportable-include-path. It's firing lots of false positives for
framework authors internally, and it's not trivial to fix.

Consider a framework called "Foo" with a main (installed) framework header
"Foo/Foo.h". It's atypical for "Foo.h" to actually live inside a
directory called "Foo" in the source repository. Instead, the
build system generates a header map while building the framework.
If Foo.h lives at the top-level of the source repository (common), and
the git repo is called ssh://some.url/foo.git, then the header map will
have something like:

Foo/Foo.h -> /Users/myname/code/foo/Foo.h

where "/Users/myname/code/foo" is the clone of ssh://some.url/foo.git.

After #import <Foo/Foo.h>, the current implementation of
-Wnonportable-include-path will falsely assume that Foo.h was found in a
nonportable way, because of the name of the git clone (.../foo/Foo.h).
However, that directory name was not involved in the header search at
all.

This commit adds an extra parameter to Preprocessor::LookupFile and
HeaderSearch::LookupFile to track if the search used a header map,
making it easy to suppress the warning. Longer term, once we find a way
to avoid the false positive, we should turn the warning back on.

rdar://problem/29849743

[eric_niebler]

clang/include/clang/Lex/HeaderSearch.h

401

Why not just add a , bool *IsMapped = nullptr parameter to the existing LookupFile?

[dexonsmith]

Thanks for the review, Eli.

I've changed IsModule to be a required bool* parameter. Setting the default to nullptr looked confusing at call sites.

[dexonsmith]

s/Eli/Eric/

[dexonsmith]

Fixed in r301592.

clang/include/clang/Lex/HeaderSearch.h

public:
   ///
   /// \param IncludeLoc Used for diagnostics if valid.
   ///
   /// \param isAngled indicates whether the file reference is a <> reference.
   ///
   /// \param CurDir If non-null, the file was found in the specified directory
   /// search location.  This is used to implement \#include_next.
   ///
+  /// \param IsMapped If true, the search involved header maps.
+  ///
   /// \param Includers Indicates where the \#including file(s) are, in case
   /// relative searches are needed. In reverse order of inclusion.
   ///
   /// \param SearchPath If non-null, will be set to the search path relative
   /// to which the file was found. If the include path is absolute, SearchPath
   /// will be set to an empty string.
   ///
   /// \param RelativePath If non-null, will be set to the path relative to
   /// SearchPath at which the file was found. This only differs from the
   /// Filename for framework includes.
   ///
   /// \param SuggestedModule If non-null, and the file found is semantically
   /// part of a known module, this will be set to the module that should
   /// be imported instead of preprocessing/parsing the file found.
   const FileEntry *LookupFile(
       StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
       const DirectoryLookup *FromDir, const DirectoryLookup *&CurDir,
+      bool &IsMapped,
       ArrayRef<std::pair<const FileEntry *, const DirectoryEntry *>> Includers,
       SmallVectorImpl<char> *SearchPath, SmallVectorImpl<char> *RelativePath,
       Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
       bool SkipCache = false, bool BuildSystemModule = false);
 
+  const FileEntry *LookupFile(
+      StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
+      const DirectoryLookup *FromDir, const DirectoryLookup *&CurDir,
+      ArrayRef<std::pair<const FileEntry *, const DirectoryEntry *>> Includers,
+      SmallVectorImpl<char> *SearchPath, SmallVectorImpl<char> *RelativePath,
+      Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
+      bool SkipCache = false, bool BuildSystemModule = false) {
+    bool IsMapped = false;
+    return LookupFile(Filename, IncludeLoc, isAngled, FromDir, CurDir, IsMapped,
+                      Includers, SearchPath, RelativePath, RequestingModule,
+                      SuggestedModule, SkipCache, BuildSystemModule);
+  }
+
   /// \brief Look up a subframework for the specified \#include file.
   ///
   /// For example, if \#include'ing <HIToolbox/HIToolbox.h> from
   /// within ".../Carbon.framework/Headers/Carbon.h", check to see if
   /// HIToolbox is a subframework within Carbon.framework.  If so, return
   /// the FileEntry for the designated file, otherwise return null.
   const FileEntry *LookupSubframeworkHeader(
       StringRef Filename, const FileEntry *RelativeFileEnt,

clang/include/clang/Lex/Preprocessor.h

public:
 
   /// \brief Given a "foo" or \<foo> reference, look up the indicated file.
   ///
   /// Returns null on failure.  \p isAngled indicates whether the file
   /// reference is for system \#include's or not (i.e. using <> instead of "").
   const FileEntry *LookupFile(SourceLocation FilenameLoc, StringRef Filename,
                               bool isAngled, const DirectoryLookup *FromDir,
                               const FileEntry *FromFile,
-                              const DirectoryLookup *&CurDir,
+                              const DirectoryLookup *&CurDir, bool &IsMapped,
                               SmallVectorImpl<char> *SearchPath,
                               SmallVectorImpl<char> *RelativePath,
                               ModuleMap::KnownHeader *SuggestedModule,
                               bool SkipCache = false);
 
+  const FileEntry *LookupFile(SourceLocation FilenameLoc, StringRef Filename,
+                              bool isAngled, const DirectoryLookup *FromDir,
+                              const FileEntry *FromFile,
+                              const DirectoryLookup *&CurDir,
+                              SmallVectorImpl<char> *SearchPath,
+                              SmallVectorImpl<char> *RelativePath,
+                              ModuleMap::KnownHeader *SuggestedModule,
+                              bool SkipCache = false) {
+    bool IsMapped = false;
+    return LookupFile(FilenameLoc, Filename, isAngled, FromDir, FromFile,
+                      CurDir, IsMapped, SearchPath, RelativePath,
+                      SuggestedModule, SkipCache);
+  }
+
   /// \brief Get the DirectoryLookup structure used to find the current
   /// FileEntry, if CurLexer is non-null and if applicable. 
   ///
   /// This allows us to implement \#include_next and find directory-specific
   /// properties.
   const DirectoryLookup *GetCurDirLookup() { return CurDirLookup; }
 
   /// \brief Return true if we're in the top-level file, not in a \#include.

clang/lib/Lex/HeaderSearch.cpp

 /// LookupFile - Given a "foo" or \<foo> reference, look up the indicated file,
 /// return null on failure.  isAngled indicates whether the file reference is
 /// for system \#include's or not (i.e. using <> instead of ""). Includers, if
 /// non-empty, indicates where the \#including file(s) are, in case a relative
 /// search is needed. Microsoft mode will pass all \#including files.
 const FileEntry *HeaderSearch::LookupFile(
     StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
     const DirectoryLookup *FromDir, const DirectoryLookup *&CurDir,
+    bool &IsMapped,
     ArrayRef<std::pair<const FileEntry *, const DirectoryEntry *>> Includers,
     SmallVectorImpl<char> *SearchPath, SmallVectorImpl<char> *RelativePath,
     Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
     bool SkipCache, bool BuildSystemModule) {
+  IsMapped = false;
   if (SuggestedModule)
     *SuggestedModule = ModuleMap::KnownHeader();
     
   // If 'Filename' is absolute, check to see if it exists and no searching.
   if (llvm::sys::path::is_absolute(Filename)) {
     CurDir = nullptr;
 
     // If this was an #include_next "/absolute/file", fail.
const FileEntry *HeaderSearch::LookupFile(
   LookupFileCacheInfo &CacheLookup = LookupFileCache[Filename];
 
   // If the entry has been previously looked up, the first value will be
   // non-zero.  If the value is equal to i (the start point of our search), then
   // this is a matching hit.
   if (!SkipCache && CacheLookup.StartIdx == i+1) {
     // Skip querying potentially lots of directories for this lookup.
     i = CacheLookup.HitIdx;
-    if (CacheLookup.MappedName)
+    if (CacheLookup.MappedName) {
       Filename = CacheLookup.MappedName;
+      IsMapped = true;
+    }
   } else {
     // Otherwise, this is the first query, or the previous query didn't match
     // our search start.  We will fill in our found location below, so prime the
     // start point value.
     CacheLookup.reset(/*StartIdx=*/i+1);
   }
 
   SmallString<64> MappedName;
 
   // Check each directory in sequence to see if it contains this file.
   for (; i != SearchDirs.size(); ++i) {
     bool InUserSpecifiedSystemFramework = false;
     bool HasBeenMapped = false;
     const FileEntry *FE = SearchDirs[i].LookupFile(
         Filename, *this, IncludeLoc, SearchPath, RelativePath, RequestingModule,
         SuggestedModule, InUserSpecifiedSystemFramework, HasBeenMapped,
         MappedName);
     if (HasBeenMapped) {
+      IsMapped = true;
       CacheLookup.MappedName =
           copyString(Filename, LookupFileCache.getAllocator());
     }
     if (!FE) continue;
 
     CurDir = &SearchDirs[i];
 
     // This file is a system header or C++ unfriendly if the dir is.
if (!Includers.empty() && Includers.front().first && !isAngled &&
     if (IncludingHFI.IndexHeaderMapHeader) {
       SmallString<128> ScratchFilename;
       ScratchFilename += IncludingHFI.Framework;
       ScratchFilename += '/';
       ScratchFilename += Filename;
 
       const FileEntry *FE =
           LookupFile(ScratchFilename, IncludeLoc, /*isAngled=*/true, FromDir,
-                     CurDir, Includers.front(), SearchPath, RelativePath,
-                     RequestingModule, SuggestedModule);
+                     CurDir, IsMapped, Includers.front(), SearchPath,
+                     RelativePath, RequestingModule, SuggestedModule);
 
       if (checkMSVCHeaderSearch(Diags, MSFE, FE, IncludeLoc)) {
         if (SuggestedModule)
           *SuggestedModule = MSSuggestedModule;
         return MSFE;
       }
 
       LookupFileCacheInfo &CacheLookup = LookupFileCache[Filename];

clang/lib/Lex/PPDirectives.cpp

 
 const FileEntry *Preprocessor::LookupFile(
     SourceLocation FilenameLoc,
     StringRef Filename,
     bool isAngled,
     const DirectoryLookup *FromDir,
     const FileEntry *FromFile,
     const DirectoryLookup *&CurDir,
+    bool &IsMapped,
     SmallVectorImpl<char> *SearchPath,
     SmallVectorImpl<char> *RelativePath,
     ModuleMap::KnownHeader *SuggestedModule,
     bool SkipCache) {
   Module *RequestingModule = getModuleForLocation(FilenameLoc);
   bool RequestingModuleIsModuleInterface = !SourceMgr.isInMainFile(FilenameLoc);
 
   // If the header lookup mechanism may be relative to the current inclusion
while (const FileEntry *FE = HeaderInfo.LookupFile(
         CurDir = TmpCurDir;
         break;
       }
     }
   }
 
   // Do a standard file entry lookup.
   const FileEntry *FE = HeaderInfo.LookupFile(
-      Filename, FilenameLoc, isAngled, FromDir, CurDir, Includers, SearchPath,
-      RelativePath, RequestingModule, SuggestedModule, SkipCache,
+      Filename, FilenameLoc, isAngled, FromDir, CurDir, IsMapped, Includers,
+      SearchPath, RelativePath, RequestingModule, SuggestedModule, SkipCache,
       BuildSystemModule);
   if (FE) {
     if (SuggestedModule && !LangOpts.AsmPreprocessor)
       HeaderInfo.getModuleMap().diagnoseHeaderInclusion(
           RequestingModule, RequestingModuleIsModuleInterface, FilenameLoc,
           Filename, FE);
     return FE;
   }
if (HeaderInfo.HasIncludeAliasMap()) {
     // map to anything, fall back on the filename we've already gotten the
     // spelling for.
     StringRef NewName = HeaderInfo.MapHeaderToIncludeAlias(OriginalFilename);
     if (!NewName.empty())
       Filename = NewName;
   }
 
   // Search include directories.
+  bool IsMapped = false;
   const DirectoryLookup *CurDir;
   SmallString<1024> SearchPath;
   SmallString<1024> RelativePath;
   // We get the raw path only if we have 'Callbacks' to which we later pass
   // the path.
   ModuleMap::KnownHeader SuggestedModule;
   SourceLocation FilenameLoc = FilenameTok.getLocation();
   SmallString<128> NormalizedPath;
   if (LangOpts.MSVCCompat) {
     NormalizedPath = Filename.str();
 #ifndef LLVM_ON_WIN32
     llvm::sys::path::native(NormalizedPath);
 #endif
   }
   const FileEntry *File = LookupFile(
       FilenameLoc, LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename,
-      isAngled, LookupFrom, LookupFromFile, CurDir,
+      isAngled, LookupFrom, LookupFromFile, CurDir, IsMapped,
       Callbacks ? &SearchPath : nullptr, Callbacks ? &RelativePath : nullptr,
       &SuggestedModule);
 
   if (!File) {
     if (Callbacks) {
       // Give the clients a chance to recover.
       SmallString<128> RecoveryPath;
       if (Callbacks->FileNotFound(Filename, RecoveryPath)) {
         if (const DirectoryEntry *DE = FileMgr.getDirectory(RecoveryPath)) {
           // Add the recovery path to the list of search paths.
           DirectoryLookup DL(DE, SrcMgr::C_User, false);
           HeaderInfo.AddSearchPath(DL, isAngled);
 
           // Try the lookup again, skipping the cache.
           File = LookupFile(
               FilenameLoc,
               LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
-              LookupFrom, LookupFromFile, CurDir, nullptr, nullptr,
+              LookupFrom, LookupFromFile, CurDir, IsMapped, nullptr, nullptr,
               &SuggestedModule, /*SkipCache*/ true);
         }
       }
     }
 
     if (!SuppressIncludeNotFoundError) {
       // If the file could not be located and it was included via angle
       // brackets, we can attempt a lookup as though it were a quoted path to
       // provide the user with a possible fixit.
       if (isAngled) {
         File = LookupFile(
             FilenameLoc,
             LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, false,
-            LookupFrom, LookupFromFile, CurDir,
+            LookupFrom, LookupFromFile, CurDir, IsMapped,
             Callbacks ? &SearchPath : nullptr,
             Callbacks ? &RelativePath : nullptr,
             &SuggestedModule);
         if (File) {
           SourceRange Range(FilenameTok.getLocation(), CharEnd);
           Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
             Filename <<
             FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\"");
std::max(HeaderInfo.getFileDirFlavor(File),
              SourceMgr.getFileCharacteristic(FilenameTok.getLocation()));
 
   // FIXME: If we have a suggested module, and we've already visited this file,
   // don't bother entering it again. We know it has no further effect.
 
   // Issue a diagnostic if the name of the file on disk has a different case
   // than the one we're about to open.
   const bool CheckIncludePathPortability =
-    File && !File->tryGetRealPathName().empty();
+      !IsMapped && File && !File->tryGetRealPathName().empty();
 
   if (CheckIncludePathPortability) {
     StringRef Name = LangOpts.MSVCCompat ? NormalizedPath.str() : Filename;
     StringRef RealPathName = File->tryGetRealPathName();
     SmallVector<StringRef, 16> Components(llvm::sys::path::begin(Name),
                                           llvm::sys::path::end(Name));
 
     if (trySimplifyPath(Components, RealPathName)) {

clang/test/Preprocessor/nonportable-include-with-hmap.c

+// RUN: %clang_cc1 -Eonly                        \
+// RUN:   -I%S/Inputs/nonportable-hmaps/foo.hmap \
+// RUN:   -I%S/Inputs/nonportable-hmaps          \
+// RUN:   %s -verify
+//
+// foo.hmap contains: Foo/Foo.h -> headers/foo/Foo.h
+//
+// Header search of "Foo/Foo.h" follows this path:
+//  1. Look for "Foo/Foo.h".
+//  2. Find "Foo/Foo.h" in "nonportable-hmaps/foo.hmap".
+//  3. Look for "headers/foo/Foo.h".
+//  4. Find "headers/foo/Foo.h" in "nonportable-hmaps".
+//  5. Return.
+//
+// There is nothing nonportable; -Wnonportable-include-path should not fire.
+#include "Foo/Foo.h" // expected-no-diagnostics