vsapsai marked 2 inline comments as done.
vsapsai added a comment.
Thanks for the review.
================
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)
----------------
dexonsmith wrote:
> 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.
I've tried the suggested approach but don't find it better. It is a personal
preference but I like how `*IsMapped |= ...` conveys the value is an
"aggregate" of the previous file lookups.
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
