bruno added inline comments.

================
Comment at: lib/Frontend/CompilerInstance.cpp:1438-1450
@@ -1437,3 +1437,15 @@
     // Search for a module with the given name.
     Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
-    if (!Module) {
+    HeaderSearchOptions &HSOpts =
+        PP->getHeaderSearchInfo().getHeaderSearchOpts();
+
+    std::string ModuleFileName;
+    bool LoadFromPrebuiltModulePath = false;
+    if (Module) {
+      ModuleFileName = PP->getHeaderSearchInfo().getModuleFileName(Module);
+    } else if (!HSOpts.PrebuiltModulePath.empty()) {
+      // Try to load the module from the prebuilt module path.
+      ModuleFileName =
+          PP->getHeaderSearchInfo().getModuleFileName(ModuleName, "", true);
+      LoadFromPrebuiltModulePath = true;
+    } else {
----------------
manmanren wrote:
> rsmith wrote:
> > It looks like the logic here is:
> > 
> >  * if we found the module in a module map, then look for it in the module 
> > cache
> >  * otherwise, if we have a prebuilt module path, then look for it there
> >  * otherwise, error
> > 
> > I think that may be surprising; if I explicitly load a module map, perhaps 
> > via `-fmodule-map-file=`, and supply a prebuilt form of that module in the 
> > prebuilt modules path, I'd expect my prebuilt form to still be used.
> > 
> > Perhaps instead we could first search the prebuilt module path for an 
> > existing .pcm file, and if that fails and we have a `Module` then look for 
> > it in the module cache?
> I was wondering about the priority of loading from prebuilt module path vs. 
> loading from module cache. Your suggestion makes sense.
+1 to Richard's suggestion here.

I wonder about the workflow on generating such modules. Suppose I want to build 
some modules to re-use later with -fprebuilt-module-path=. RIght now, we get a 
module cache with hash in the names unless we disable hash computation via cc1 
option -fdisable-module-hash. Should we assume that the user will have to copy 
out the cache dir and rename the modules OR that it will use 
-fdisable-module-hash instead? Should this option also try to read out modules 
with a hash in the name? Should we promote -fdisable-module-hash to a driver 
option as well?

@Manman, do you have any workflow in mind? @Richard, do you have any feedback 
in this respect?

In the crash reproducer we often have to rebuild the modules when running the 
script in a different machine because of the paths used to hash the modules, it 
would be nice (depending on the resulting design) if the crash reproducer could 
use -fprebuilt-module-path in order to try to re-use the pre-built modules 
resulting from the crash. 


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