[PATCH] D156749: [modules] Fix error about the same module being defined in different .pcm files when using VFS overlays.
Volodymyr Sapsai via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 9 18:23:00 PDT 2023
vsapsai added a comment.
In D156749#4565994 <https://reviews.llvm.org/D156749#4565994>, @jansvoboda11 wrote:
> I think your solution is the most pragmatic. If you're confident this doesn't break anything internally, I say go for it. But I think it's good to be aware of the pitfall I mentioned, and make sure the build system doesn't trigger that.
As far as I have tested, it isn't breaking anything.
================
Comment at: clang/lib/Serialization/ASTWriter.cpp:1330
AddPath(WritingModule->PresumedModuleMapFile.empty()
- ? Map.getModuleMapFileForUniquing(WritingModule)->getName()
+ ? Map.getModuleMapFileForUniquing(WritingModule)
+ ->getNameAsRequested()
----------------
jansvoboda11 wrote:
> Can we canonicalize this also? It'd be useful in the scanner.
I'm not sure about that. ASTReader has some complicated logic around reading this value https://github.com/llvm/llvm-project/blob/c1803d5366c794ecade4e4ccd0013690a1976d49/clang/lib/Serialization/ASTReader.cpp#L4005 So if we don't have a proven need for it with a test case, I wouldn't change it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156749/new/
https://reviews.llvm.org/D156749
More information about the cfe-commits
mailing list