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

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 9 16:38:02 PDT 2016


rsmith added inline comments.

================
Comment at: lib/Frontend/CompilerInstance.cpp:1483
@@ +1482,3 @@
+                                   HSOpts.ModulesUsePrebuiltModules ?
+                                   ASTReader::ARR_ConfigurationMismatch :
+                                   ARRFlags)) {
----------------
This looks wrong: you're asking `ReadAST` to not diagnose a configuration mismatch, and you also don't diagnose it yourself.

================
Comment at: lib/Frontend/CompilerInstance.cpp:1502-1510
@@ -1477,1 +1501,11 @@
     case ASTReader::Missing: {
+      if (HSOpts.ModulesUsePrebuiltModules) {
+        // We can't rebuild the module without a module map. Throw diagnostics
+        // here if using prebuilt modules.
+        getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
+        << ModuleName
+        << SourceRange(ImportLoc, ModuleNameLoc);
+        ModuleBuildFailed = true;
+        return ModuleLoadResult();
+      }
+
----------------
You requested that `ReadAST` produces diagnostics for the `OutOfDate` and `Missing` cases when using prebuilt modules, so this diagnostic is redundant.

================
Comment at: lib/Serialization/ASTReader.cpp:495-498
@@ -494,1 +494,6 @@
 
+static bool isPrebuiltModule(Preprocessor &PP, ModuleKind MK) {
+  return (MK == MK_ExplicitModule || MK == MK_ImplicitModule) &&
+      PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesUsePrebuiltModules;
+}
+
----------------
This seems unnecessarily complicated. Perhaps you could make `CompilerInstance::loadModule` treat all prebuilt modules as `MK_ExplicitModule`, or add a third `ModuleKind` for them?


https://reviews.llvm.org/D23125





More information about the cfe-commits mailing list