]> granicus.if.org Git - clang/commitdiff
Preprocessor: Suppress -Wnonportable-include-path for header maps
authorDuncan P. N. Exon Smith <dexonsmith@apple.com>
Thu, 27 Apr 2017 21:41:51 +0000 (21:41 +0000)
committerDuncan P. N. Exon Smith <dexonsmith@apple.com>
Thu, 27 Apr 2017 21:41:51 +0000 (21:41 +0000)
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/28863903

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@301592 91177308-0d34-0410-b5e6-96231b3b80d8

12 files changed:
include/clang/Lex/HeaderSearch.h
include/clang/Lex/Preprocessor.h
lib/Frontend/CompilerInstance.cpp
lib/Frontend/Rewrite/InclusionRewriter.cpp
lib/Frontend/VerifyDiagnosticConsumer.cpp
lib/Lex/HeaderSearch.cpp
lib/Lex/PPDirectives.cpp
lib/Lex/PPMacroExpansion.cpp
lib/Lex/Pragma.cpp
test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap [new file with mode: 0644]
test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Foo.h [new file with mode: 0644]
test/Preprocessor/nonportable-include-with-hmap.c [new file with mode: 0644]

index 51983b9ab5d8e5a4637e0a5edea63b7c6a26e462..64c6f2a03f2b8890d9102974f4e5707ed8cff408 100644 (file)
@@ -375,13 +375,16 @@ public:
   /// \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.
+  ///
+  /// \param IsMapped If non-null, and the search involved header maps, set to
+  /// true.
   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, bool SkipCache = false, bool BuildSystemModule = false);
 
   /// \brief Look up a subframework for the specified \#include file.
   ///
index 3efe914daaeef71398840338f56688348ca8a1f0..f48cab68375229f096fd4131e4efe1b0ce80c7cd 100644 (file)
@@ -1686,7 +1686,7 @@ public:
                               SmallVectorImpl<char> *SearchPath,
                               SmallVectorImpl<char> *RelativePath,
                               ModuleMap::KnownHeader *SuggestedModule,
-                              bool SkipCache = false);
+                              bool *IsMapped, bool SkipCache = false);
 
   /// \brief Get the DirectoryLookup structure used to find the current
   /// FileEntry, if CurLexer is non-null and if applicable. 
index dc309837312c7923dbc0fd4dcb46d6fc262912df..1f85f6811888a91aad2b876b6649a7f8a61c827f 100644 (file)
@@ -858,7 +858,8 @@ bool CompilerInstance::InitializeSourceManager(
                             /*SearchPath=*/nullptr,
                             /*RelativePath=*/nullptr,
                             /*RequestingModule=*/nullptr,
-                            /*SuggestedModule=*/nullptr, /*SkipCache=*/true);
+                            /*SuggestedModule=*/nullptr, /*IsMapped=*/nullptr,
+                            /*SkipCache=*/true);
       // Also add the header to /showIncludes output.
       if (File)
         DepOpts.ShowIncludesPretendHeader = File->getName();
index d953da2e4fd2946916abc42c5905cb391eb40465..ca14da3bca3910c76bf69602ea63adce444813d3 100644 (file)
@@ -392,7 +392,7 @@ bool InclusionRewriter::HandleHasInclude(
   // FIXME: Why don't we call PP.LookupFile here?
   const FileEntry *File = PP.getHeaderSearchInfo().LookupFile(
       Filename, SourceLocation(), isAngled, nullptr, CurDir, Includers, nullptr,
-      nullptr, nullptr, nullptr, false);
+      nullptr, nullptr, nullptr, nullptr);
 
   FileExists = File != nullptr;
   return true;
index ae16ea177ffedef6948bb9ec23f07f1a248971d3..427d15ed703aea7bb2bf3b2f8d5dc6670bc5e781 100644 (file)
@@ -400,7 +400,7 @@ static bool ParseDirective(StringRef S, ExpectedData *ED, SourceManager &SM,
         const DirectoryLookup *CurDir;
         const FileEntry *FE =
             PP->LookupFile(Pos, Filename, false, nullptr, nullptr, CurDir,
-                           nullptr, nullptr, nullptr);
+                           nullptr, nullptr, nullptr, nullptr);
         if (!FE) {
           Diags.Report(Pos.getLocWithOffset(PH.C-PH.Begin),
                        diag::err_verify_missing_file) << Filename << KindStr;
index 4ee38719289bcb61c6c303341c3d4d0aa1e16c97..bd425a07c33a658b8c23a9650d765a73d7d724e8 100644 (file)
@@ -624,7 +624,10 @@ const FileEntry *HeaderSearch::LookupFile(
     ArrayRef<std::pair<const FileEntry *, const DirectoryEntry *>> Includers,
     SmallVectorImpl<char> *SearchPath, SmallVectorImpl<char> *RelativePath,
     Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
-    bool SkipCache, bool BuildSystemModule) {
+    bool *IsMapped, bool SkipCache, bool BuildSystemModule) {
+  if (IsMapped)
+    *IsMapped = false;
+
   if (SuggestedModule)
     *SuggestedModule = ModuleMap::KnownHeader();
     
@@ -754,8 +757,11 @@ const FileEntry *HeaderSearch::LookupFile(
   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;
+      if (IsMapped)
+        *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
@@ -776,6 +782,8 @@ const FileEntry *HeaderSearch::LookupFile(
     if (HasBeenMapped) {
       CacheLookup.MappedName =
           copyString(Filename, LookupFileCache.getAllocator());
+      if (IsMapped)
+        *IsMapped = true;
     }
     if (!FE) continue;
 
@@ -839,7 +847,7 @@ const FileEntry *HeaderSearch::LookupFile(
       const FileEntry *FE =
           LookupFile(ScratchFilename, IncludeLoc, /*isAngled=*/true, FromDir,
                      CurDir, Includers.front(), SearchPath, RelativePath,
-                     RequestingModule, SuggestedModule);
+                     RequestingModule, SuggestedModule, IsMapped);
 
       if (checkMSVCHeaderSearch(Diags, MSFE, FE, IncludeLoc)) {
         if (SuggestedModule)
index fd4e6d30def964429f6f77a82aa7766987da23fd..54578e8fb9fcc45045ec30469d02d04a1795f877 100644 (file)
@@ -752,16 +752,11 @@ Preprocessor::getModuleHeaderToIncludeForDiagnostics(SourceLocation IncLoc,
 }
 
 const FileEntry *Preprocessor::LookupFile(
-    SourceLocation FilenameLoc,
-    StringRef Filename,
-    bool isAngled,
-    const DirectoryLookup *FromDir,
-    const FileEntry *FromFile,
-    const DirectoryLookup *&CurDir,
-    SmallVectorImpl<char> *SearchPath,
+    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) {
+    ModuleMap::KnownHeader *SuggestedModule, bool *IsMapped, bool SkipCache) {
   Module *RequestingModule = getModuleForLocation(FilenameLoc);
   bool RequestingModuleIsModuleInterface = !SourceMgr.isInMainFile(FilenameLoc);
 
@@ -819,7 +814,7 @@ const FileEntry *Preprocessor::LookupFile(
     while (const FileEntry *FE = HeaderInfo.LookupFile(
                Filename, FilenameLoc, isAngled, TmpFromDir, TmpCurDir,
                Includers, SearchPath, RelativePath, RequestingModule,
-               SuggestedModule, SkipCache)) {
+               SuggestedModule, /*IsMapped=*/nullptr, SkipCache)) {
       // Keep looking as if this file did a #include_next.
       TmpFromDir = TmpCurDir;
       ++TmpFromDir;
@@ -835,7 +830,7 @@ const FileEntry *Preprocessor::LookupFile(
   // Do a standard file entry lookup.
   const FileEntry *FE = HeaderInfo.LookupFile(
       Filename, FilenameLoc, isAngled, FromDir, CurDir, Includers, SearchPath,
-      RelativePath, RequestingModule, SuggestedModule, SkipCache,
+      RelativePath, RequestingModule, SuggestedModule, IsMapped, SkipCache,
       BuildSystemModule);
   if (FE) {
     if (SuggestedModule && !LangOpts.AsmPreprocessor)
@@ -1783,6 +1778,7 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
   }
 
   // Search include directories.
+  bool IsMapped = false;
   const DirectoryLookup *CurDir;
   SmallString<1024> SearchPath;
   SmallString<1024> RelativePath;
@@ -1801,7 +1797,7 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
       FilenameLoc, LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename,
       isAngled, LookupFrom, LookupFromFile, CurDir,
       Callbacks ? &SearchPath : nullptr, Callbacks ? &RelativePath : nullptr,
-      &SuggestedModule);
+      &SuggestedModule, &IsMapped);
 
   if (!File) {
     if (Callbacks) {
@@ -1818,7 +1814,7 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
               FilenameLoc,
               LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
               LookupFrom, LookupFromFile, CurDir, nullptr, nullptr,
-              &SuggestedModule, /*SkipCache*/ true);
+              &SuggestedModule, &IsMapped, /*SkipCache*/ true);
         }
       }
     }
@@ -1833,8 +1829,7 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
             LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, false,
             LookupFrom, LookupFromFile, CurDir,
             Callbacks ? &SearchPath : nullptr,
-            Callbacks ? &RelativePath : nullptr,
-            &SuggestedModule);
+            Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
         if (File) {
           SourceRange Range(FilenameTok.getLocation(), CharEnd);
           Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
@@ -1964,7 +1959,7 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
   // 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 358c96a783009e5887ac44d4a20a5f0b662f66c7..196223981d74b261dac7f7d77ac05160a2984a86 100644 (file)
@@ -1422,7 +1422,7 @@ static bool EvaluateHasIncludeCommon(Token &Tok,
   const DirectoryLookup *CurDir;
   const FileEntry *File =
       PP.LookupFile(FilenameLoc, Filename, isAngled, LookupFrom, LookupFromFile,
-                    CurDir, nullptr, nullptr, nullptr);
+                    CurDir, nullptr, nullptr, nullptr, nullptr);
 
   // Get the result value.  A result of true means the file exists.
   return File != nullptr;
index 87e105d1d03d49fd226b6f3ba6b71a7e574d2d31..fcaaa2d416ef048420a3dadd31f0742968919a0a 100644 (file)
@@ -508,7 +508,7 @@ void Preprocessor::HandlePragmaDependency(Token &DependencyTok) {
   const DirectoryLookup *CurDir;
   const FileEntry *File =
       LookupFile(FilenameTok.getLocation(), Filename, isAngled, nullptr,
-                 nullptr, CurDir, nullptr, nullptr, nullptr);
+                 nullptr, CurDir, nullptr, nullptr, nullptr, nullptr);
   if (!File) {
     if (!SuppressIncludeNotFoundError)
       Diag(FilenameTok, diag::err_pp_file_not_found) << Filename;
diff --git a/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap b/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap
new file mode 100644 (file)
index 0000000..9036f20
Binary files /dev/null and b/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap differ
diff --git a/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Foo.h b/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Foo.h
new file mode 100644 (file)
index 0000000..e69de29
diff --git a/test/Preprocessor/nonportable-include-with-hmap.c b/test/Preprocessor/nonportable-include-with-hmap.c
new file mode 100644 (file)
index 0000000..fc958e7
--- /dev/null
@@ -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