dexonsmith added a subscriber: arphaman. dexonsmith added a comment. In D103930#2820725 <https://reviews.llvm.org/D103930#2820725>, @bruno wrote:
> Thanks for working on this, comments inline. @vsapsai @jansvoboda11 > @dexonsmith any headermap related concerns on your side? @jansvoboda11, I think it'd be prudent for us to test this patch out internally before it's landed, since I don't really trust that the existing unit tests cover all the interactions between header maps and modules. Might you be able to coordinate something with @arphaman? ================ Comment at: clang/test/Modules/implicit-module-header-maps.cpp:27 +#define FOO +// This include will fail if modules weren't used. The include name itself +// doesn't exist and relies on the header map to remap it to the real header. ---------------- Do you really mean if modules aren't used, or do you mean if header maps aren't used? (I think we want to find the same headers on disk whether or not modules are on... if this patch changes that, then I guess I'm not totally understanding why...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103930/new/ https://reviews.llvm.org/D103930 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
