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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to