[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.


================
Comment at: clang/lib/Lex/HeaderSearch.cpp:888
     bool InUserSpecifiedSystemFramework = false;
-    bool HasBeenMapped = false;
+    bool CurrentInHeaderMap = false;
     bool IsFrameworkFoundInDir = false;
----------------
Why not name this the same way as the parameter?  (`IsInHeaderMap`)


================
Comment at: clang/lib/Lex/HeaderSearch.cpp:896-897
+      assert(CurrentInHeaderMap && "MappedName should come from a header map");
       CacheLookup.MappedName =
-          copyString(Filename, LookupFileCache.getAllocator());
-      if (IsMapped)
-        *IsMapped = true;
+          copyString(MappedName, LookupFileCache.getAllocator());
+      HasBeenMapped = true;
----------------
It's not obvious to me why this changed from `Filename` to `MappedName`.  Can you explain it?


================
Comment at: clang/lib/Lex/HeaderSearch.cpp:898
+          copyString(MappedName, LookupFileCache.getAllocator());
+      HasBeenMapped = true;
     }
----------------
Do we need `HasBeenMapped` outside of the loop, or can you just use the loop-local variable?


================
Comment at: clang/lib/Lex/HeaderSearch.cpp:908-909
 
+    if (IsMapped)
+      *IsMapped = CurrentInHeaderMap || HasBeenMapped;
+
----------------
Are you intentionally delaying this until after the `if (!File)` check?  If so, why?


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

https://reviews.llvm.org/D58094