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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
