[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.

Fri Aug 4 13:52:03 PDT 2023

jansvoboda11 added a comment.

In D156749#4552590 <https://reviews.llvm.org/D156749#4552590>, @vsapsai wrote:

> In D156749#4551596 <https://reviews.llvm.org/D156749#4551596>, @jansvoboda11 wrote:
>
>> Alternatively, we could keep VFS overlays out of the context hash but create `<hash2>` from the on-disk real path of the defining module map and make the whole PCM VFS-agnostic. Then it'd be correct to import that PCM regardless of the specific VFS overlay setup, as long as all VFS queries of the importer resolve the same way they resolved within the instance that built the PCM. Maybe we can force the importer to recompile the PCM if that's not the case, similar to what we do for diagnostic options.
>
> I'm not sure I understand your proposal. Before this change we were calculating hash from the on-disk real path of the defining module map. And due to different VFS/no-VFS options defining module map is at different locations on-disk.

My suggestion is to use the actual real on-disk path. Not `FileEntryRef::getName()`, but something that always behaves as if `use-external-name` was set to `true`. I believe this would handle your VFS/VFS-use-external-name-true/VFS-use-external-name-false problem. It would also handle another pitfall: two compilations with distinct VFS overlays that redirect two different as-requested module map paths into the same on-disk path.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156749