dexonsmith created this revision.

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


https://reviews.llvm.org/D32263

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap
  clang/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Foo.h
  clang/test/Preprocessor/nonportable-include-with-hmap.c

Index: clang/test/Preprocessor/nonportable-include-with-hmap.c
===================================================================
--- /dev/null
+++ clang/test/Preprocessor/nonportable-include-with-hmap.c
@@ -0,0 +1,16 @@
+// 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
Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -758,6 +758,7 @@
     const DirectoryLookup *FromDir,
     const FileEntry *FromFile,
     const DirectoryLookup *&CurDir,
+    bool &IsMapped,
     SmallVectorImpl<char> *SearchPath,
     SmallVectorImpl<char> *RelativePath,
     ModuleMap::KnownHeader *SuggestedModule,
@@ -834,8 +835,8 @@
 
   // 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)
@@ -1783,6 +1784,7 @@
   }
 
   // Search include directories.
+  bool IsMapped = false;
   const DirectoryLookup *CurDir;
   SmallString<1024> SearchPath;
   SmallString<1024> RelativePath;
@@ -1799,7 +1801,7 @@
   }
   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);
 
@@ -1817,7 +1819,7 @@
           File = LookupFile(
               FilenameLoc,
               LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
-              LookupFrom, LookupFromFile, CurDir, nullptr, nullptr,
+              LookupFrom, LookupFromFile, CurDir, IsMapped, nullptr, nullptr,
               &SuggestedModule, /*SkipCache*/ true);
         }
       }
@@ -1831,7 +1833,7 @@
         File = LookupFile(
             FilenameLoc,
             LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, false,
-            LookupFrom, LookupFromFile, CurDir,
+            LookupFrom, LookupFromFile, CurDir, IsMapped,
             Callbacks ? &SearchPath : nullptr,
             Callbacks ? &RelativePath : nullptr,
             &SuggestedModule);
@@ -1964,7 +1966,7 @@
   // 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;
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -621,10 +621,12 @@
 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();
     
@@ -754,8 +756,10 @@
   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
@@ -774,6 +778,7 @@
         SuggestedModule, InUserSpecifiedSystemFramework, HasBeenMapped,
         MappedName);
     if (HasBeenMapped) {
+      IsMapped = true;
       CacheLookup.MappedName =
           copyString(Filename, LookupFileCache.getAllocator());
     }
@@ -838,8 +843,8 @@
 
       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)
Index: clang/include/clang/Lex/Preprocessor.h
===================================================================
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -1682,12 +1682,26 @@
   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. 
   ///
Index: clang/include/clang/Lex/HeaderSearch.h
===================================================================
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -361,6 +361,8 @@
   /// \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.
   ///
@@ -378,11 +380,25 @@
   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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D32263: Pr... Duncan P. N. Exon Smith via Phabricator via cfe-commits

Reply via email to