[PATCH] D23125: Modules: add command line option to support loading prebuilt modules on demand, without parsing any module map

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: lib/Frontend/CompilerInstance.cpp:1502
@@ +1501,3 @@
+        Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
+        if (!Module) {
+          getDiagnostics().Report(ModuleNameLoc, diag::err_module_prebuilt)
----------------
manmanren wrote:
> Updated the patch to use error diagnostics instead of assertion.
> 
> <<<
> It'd also be good to check that Module->ASTFile actually refers to the file that we found in the prebuilt module path, and not (say) to one of its transitive dependencies that ReadAST also loaded.
> 
> Did we already check this when creating the module in ASTReader?
>       if (!ParentModule) {
>         if (const FileEntry *CurFile = CurrentModule->getASTFile()) {
>           if (CurFile != F.File) {
>             ...
>           }
>         }
> 
>         CurrentModule->setASTFile(F.File);
>       }
> 
The AST reader has no idea which module we're trying to load a module file for, so it can't check this. The check you quoted is ensuring that we don't have two different module files providing the same module.

A testcase for this would be:

  module A is in prebuilt-module-cache/x.pcm
  module B is in prebuilt-module-cache/A.pcm and depends on x.pcm

When trying to load module A, ReadAST will load A.pcm, and we'll find that we now have a definition for module A, but we still didn't get the module from the expected file.


https://reviews.llvm.org/D23125