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

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 6 17:44:27 PDT 2019


dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM.  I have one idea for you to consider inline.



================
Comment at: clang/lib/Lex/HeaderSearch.cpp:892-902
+        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)
----------------
I wonder if this would be easier to follow if you refactored like this:

```
if (!MappedName.empty()) {
  // other logic.
  if (IsMapped)
    *IsMapped = true;
} else if (IsInHeaderMap && File) {
  if (IsMapped)
    *IsMapped = true;
}
```

but maybe my aesthetics are being thrown off by all the intervening comments in Phab.  I'll leave it up to you.


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

https://reviews.llvm.org/D58094





More information about the cfe-commits mailing list