[PATCH] D132502: [clang][modules] Consider M affecting after mapping M.Private to M_Private
Jan Svoboda via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 24 11:52:40 PDT 2022
jansvoboda11 added inline comments.
================
Comment at: clang/lib/Frontend/CompilerInstance.cpp:2017
// migrate off of Foo.Private syntax.
- if (!Sub && PP->getLangOpts().ImplicitModules && Name == "Private" &&
- Module == Module->getTopLevelModule()) {
+ if (!Sub && Name == "Private" && Module == Module->getTopLevelModule()) {
SmallString<128> PrivateModule(Module->Name);
----------------
jansvoboda11 wrote:
> benlangmuir wrote:
> > Not sure if this is a good idea, but have you considered having the scanner provide the module with `Foo.Private=` syntax instead? It would be nice to keep this hack contained to implicit builds if we can.
> Interesting idea, that didn't occur to me. What implementation strategy are you thinking of?
>
> If we want to avoid this hack, it seems like we'd need to pass `ModuleIdPath ImportPath` instead of `StringRef ModuleName` to `findOrCompileModuleAndReadAST()` and friends. That would allow us to look up the most specific `-fmodule-file=` argument. I'm not convinced changing multiple functions (and then changing them back when we don't need the hack) is better than extending the hack to explicit modules.
>
> The upside is that we wouldn't need to report `M` as a dependency. WDYT?
Another thing to consider: implicit build would need to propagate the information that module `Foo_Private` needs to be spelled on the command-line as `Foo.Private` to the scanner.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132502/new/
https://reviews.llvm.org/D132502
More information about the cfe-commits
mailing list