[PATCH] D151855: [clang] Use `{File,Directory}EntryRef` in modular header search (part 2/2)

Wed Jun 7 11:15:08 PDT 2023

jansvoboda11 added a comment.

In D151855#4403879 <https://reviews.llvm.org/D151855#4403879>, @benlangmuir wrote:

>> I think it should be fine to allow dropping the A.framework/Frameworks/B.framework directory from the reproducer VFS
>
> I think technically this is wrong, since if you're missing the symlink, then A might not build -- e.g. it could be doing a relative include that needs the symlink.  But am I understanding correctly that the reproducer was already broken in this case? If so I'm fine with this.

You're right, this would break relative includes, which I believe the current test case would handle correctly. (It only fails to canonicalize the paths to `B.framework` and make it into a top-level framework.)

> The right thing to do would be to capture both the framework and the symlink. I'm not sure how practical that is with the current architecture.

Yeah, I think currently we only record the (as-requested) file names while parsing a module map. I'm not sure how we could record both the canonical and subframework-like paths with the current more precise `FileEntryRefs`. (Long-term, we should probably unify this with the "affecting files and module maps" logic in `clang-scan-deps`.)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151855