This revision was automatically updated to reflect the committed changes.
Closed by commit rL371655: Fix -Wnonportable-include-path suppression for 
header maps with absolute paths. (authored by vsapsai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58094?vs=219194&id=219788#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58094/new/

https://reviews.llvm.org/D58094

Files:
  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/Inputs/nonportable-hmaps/headers/foo/Bar.h
  cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Baz.h
  cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c

Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===================================================================
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -341,9 +341,10 @@
     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 @@
   if (Dest.empty())
     return None;
 
+  IsInHeaderMap = true;
+
   auto FixupSearchPath = [&]() {
     if (SearchPath) {
       StringRef SearchPathRef(getName());
@@ -393,10 +396,8 @@
   // ("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 @@
   // 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
Index: cfe/trunk/include/clang/Lex/DirectoryLookup.h
===================================================================
--- cfe/trunk/include/clang/Lex/DirectoryLookup.h
+++ cfe/trunk/include/clang/Lex/DirectoryLookup.h
@@ -183,7 +183,7 @@
              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(
Index: cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
===================================================================
--- cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
+++ cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
@@ -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"
     }
 }
Index: cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c
===================================================================
--- cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c
+++ cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c
@@ -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}}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to