r371655 - Fix -Wnonportable-include-path suppression for header maps with absolute paths.
Volodymyr Sapsai via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 11 13:39:04 PDT 2019
Author: vsapsai
Date: Wed Sep 11 13:39:04 2019
New Revision: 371655
URL: http://llvm.org/viewvc/llvm-project?rev=371655&view=rev
Log:
Fix -Wnonportable-include-path suppression for header maps with absolute paths.
In `DirectoryLookup::LookupFile` parameter `HasBeenMapped` doesn't cover
the case when clang finds a file through a header map but doesn't remap
the lookup filename because the target path is an absolute path. As a
result, -Wnonportable-include-path suppression for header maps
introduced in r301592 wasn't triggered.
Change parameter `HasBeenMapped` to `IsInHeaderMap` and use parameter
`MappedName` to track the filename remapping. This way we can handle
both relative and absolute paths in header maps, and account for their
specific properties, like filename remapping being a property preserved
across lookups in multiple directories.
rdar://problem/39516483
Reviewers: dexonsmith, bruno
Reviewed By: dexonsmith
Subscribers: jkorous, cfe-commits, ributzka
Differential Revision: https://reviews.llvm.org/D58094
Added:
cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Bar.h
cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Baz.h
Modified:
cfe/trunk/include/clang/Lex/DirectoryLookup.h
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c
Modified: cfe/trunk/include/clang/Lex/DirectoryLookup.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/DirectoryLookup.h?rev=371655&r1=371654&r2=371655&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/DirectoryLookup.h (original)
+++ cfe/trunk/include/clang/Lex/DirectoryLookup.h Wed Sep 11 13:39:04 2019
@@ -183,7 +183,7 @@ public:
SmallVectorImpl<char> *RelativePath, Module *RequestingModule,
ModuleMap::KnownHeader *SuggestedModule,
bool &InUserSpecifiedSystemFramework, bool &IsFrameworkFound,
- bool &HasBeenMapped, SmallVectorImpl<char> &MappedName) const;
+ bool &IsInHeaderMap, SmallVectorImpl<char> &MappedName) const;
private:
Optional<FileEntryRef> DoFrameworkLookup(
Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=371655&r1=371654&r2=371655&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Wed Sep 11 13:39:04 2019
@@ -341,9 +341,10 @@ Optional<FileEntryRef> DirectoryLookup::
SmallVectorImpl<char> *SearchPath, SmallVectorImpl<char> *RelativePath,
Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
bool &InUserSpecifiedSystemFramework, bool &IsFrameworkFound,
- bool &HasBeenMapped, SmallVectorImpl<char> &MappedName) const {
+ bool &IsInHeaderMap, SmallVectorImpl<char> &MappedName) const {
InUserSpecifiedSystemFramework = false;
- HasBeenMapped = false;
+ IsInHeaderMap = false;
+ MappedName.clear();
SmallString<1024> TmpDir;
if (isNormalDir()) {
@@ -377,6 +378,8 @@ Optional<FileEntryRef> DirectoryLookup::
if (Dest.empty())
return None;
+ IsInHeaderMap = true;
+
auto FixupSearchPath = [&]() {
if (SearchPath) {
StringRef SearchPathRef(getName());
@@ -393,10 +396,8 @@ Optional<FileEntryRef> DirectoryLookup::
// ("Foo.h" -> "Foo/Foo.h"), in which case continue header lookup using the
// framework include.
if (llvm::sys::path::is_relative(Dest)) {
- MappedName.clear();
MappedName.append(Dest.begin(), Dest.end());
Filename = StringRef(MappedName.begin(), MappedName.size());
- HasBeenMapped = true;
Optional<FileEntryRef> Result = HM->LookupFile(Filename, HS.getFileMgr());
if (Result) {
FixupSearchPath();
@@ -883,18 +884,22 @@ Optional<FileEntryRef> HeaderSearch::Loo
// Check each directory in sequence to see if it contains this file.
for (; i != SearchDirs.size(); ++i) {
bool InUserSpecifiedSystemFramework = false;
- bool HasBeenMapped = false;
+ bool IsInHeaderMap = false;
bool IsFrameworkFoundInDir = false;
Optional<FileEntryRef> File = SearchDirs[i].LookupFile(
Filename, *this, IncludeLoc, SearchPath, RelativePath, RequestingModule,
SuggestedModule, InUserSpecifiedSystemFramework, IsFrameworkFoundInDir,
- HasBeenMapped, MappedName);
- if (HasBeenMapped) {
+ IsInHeaderMap, MappedName);
+ if (!MappedName.empty()) {
+ assert(IsInHeaderMap && "MappedName should come from a header map");
CacheLookup.MappedName =
- copyString(Filename, LookupFileCache.getAllocator());
- if (IsMapped)
- *IsMapped = true;
+ copyString(MappedName, LookupFileCache.getAllocator());
}
+ if (IsMapped)
+ // A filename is mapped when a header map remapped it to a relative path
+ // used in subsequent header search or to an absolute path pointing to an
+ // existing file.
+ *IsMapped |= (!MappedName.empty() || (IsInHeaderMap && File));
if (IsFrameworkFound)
// Because we keep a filename remapped for subsequent search directory
// lookups, ignore IsFrameworkFoundInDir after the first remapping and not
Modified: cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json?rev=371655&r1=371654&r2=371655&view=diff
==============================================================================
--- cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json (original)
+++ cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json Wed Sep 11 13:39:04 2019
@@ -1,6 +1,9 @@
{
"mappings" :
{
- "Foo/Foo.h" : "headers/foo/Foo.h"
+ "Foo/Foo.h" : "headers/foo/Foo.h",
+ "Bar.h" : "headers/foo/Bar.h",
+ "Foo/Bar.h" : "INPUTS_DIR/nonportable-hmaps/headers/foo/Bar.h",
+ "headers/Foo/Baz.h" : "/not/existing/absolute/path/Baz.h"
}
}
Added: cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Bar.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Bar.h?rev=371655&view=auto
==============================================================================
(empty)
Added: cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Baz.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Baz.h?rev=371655&view=auto
==============================================================================
(empty)
Modified: cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c?rev=371655&r1=371654&r2=371655&view=diff
==============================================================================
--- cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c (original)
+++ cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c Wed Sep 11 13:39:04 2019
@@ -1,5 +1,9 @@
+// REQUIRES: shell
+
// RUN: rm -f %t.hmap
-// RUN: %hmaptool write %S/Inputs/nonportable-hmaps/foo.hmap.json %t.hmap
+// RUN: sed -e "s:INPUTS_DIR:%S/Inputs:g" \
+// RUN: %S/Inputs/nonportable-hmaps/foo.hmap.json > %t.hmap.json
+// RUN: %hmaptool write %t.hmap.json %t.hmap
// RUN: %clang_cc1 -Eonly \
// RUN: -I%t.hmap \
// RUN: -I%S/Inputs/nonportable-hmaps \
@@ -15,4 +19,16 @@
// 5. Return.
//
// There is nothing nonportable; -Wnonportable-include-path should not fire.
-#include "Foo/Foo.h" // expected-no-diagnostics
+#include "Foo/Foo.h" // no warning
+
+// Verify files with absolute paths in the header map are handled too.
+// "Bar.h" is included twice to make sure that when we see potentially
+// nonportable path, the file has been already discovered through a relative
+// path which triggers the file to be opened and `FileEntry::RealPathName`
+// to be set.
+#include "Bar.h"
+#include "Foo/Bar.h" // no warning
+
+// But the presence of the absolute path in the header map is not enough. If we
+// didn't use it to discover a file, shouldn't suppress the warning.
+#include "headers/Foo/Baz.h" // expected-warning {{non-portable path}}
More information about the cfe-commits
mailing list