benlangmuir accepted this revision.
benlangmuir added a comment.
This revision is now accepted and ready to land.
LGTM
================
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:
> 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.
Okay, it sounds like the current patch is less invasive than the alternative.
Thanks for talking it through.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132502/new/
https://reviews.llvm.org/D132502
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
