[PATCH] D132502: [clang][modules] Consider M affecting after mapping M.Private to M_Private

[Jan Svoboda via Phabricator via cfe-commits]

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);
----------------
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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132502/new/

https://reviews.llvm.org/D132502