[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