[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 a comment.

Just a couple of things, but this basically LGTM.


================
Comment at: docs/Modules.rst:217
@@ +216,3 @@
+``-fprebuilt-module-path=<directory>``
+  Specify the path to the prebuilt modules. If specified, we will look for modules in this directory for a given top-level module name. We don't need a module map for loading prebuilt modules in this directory and the compiler will not try to rebuild these modules.
+
----------------
Please document that the option can be given multiple times.

================
Comment at: lib/Frontend/CompilerInstance.cpp:1502
@@ +1501,3 @@
+        Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
+        assert(Module && "ReadAST should construct a Module instance");
+      }
----------------
This should have a real diagnostic rather than an assertion. This would happen if the pcm file in the prebuilt module path had the wrong filename, for instance.

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.


https://reviews.llvm.org/D23125