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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to