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