manmanren added a comment.
In https://reviews.llvm.org/D23125#510632, @rsmith wrote:
> This doesn't seem like quite the right user interface for this feature. The
> module cache is volatile, and it's not really reasonable for people to assume
> they know what is and isn't within it. Instead, it seems like what we want to
> provide is zero or more paths to directories containing prebuilt .pcm files
> whose file names are the same as their top-level module names, completely
> separate from the module cache.
>
> Specifically, rather than spelling this as `-fmodules-use-prebuilt-modules
> -fmodules-cache-path=X`, I'd suggest spelling it as
> `-fprebuilt-module-path=X` or similar (which can be repeated to provide
> multiple search paths). That would then combine orthogonally with
> `-fno-implicit-modules` and `-fno-implicit-module-maps`, where implicit
> modules would still use the module cache. Does that make sense for your use
> case?
What we want is to achieve performance by prebuilding the modules and using
@import to load the corresponding pcm without parsing the module maps. In
ASTReader, we want to treat these modules in the same way as the explicit
modules.
-fprebuilt-module-path sounds reasonable. Let me see how the implementation
goes ...
> You're also missing updates to the modules documentation.
Yes, I will update the document.
Cheers,
Manman
================
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();
+ }
+
----------------
rsmith wrote:
> You requested that `ReadAST` produces diagnostics for the `OutOfDate` and
> `Missing` cases when using prebuilt modules, so this diagnostic is redundant.
Yes, I can remove this diagnostic.
================
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;
+}
+
----------------
rsmith wrote:
> This seems unnecessarily complicated. Perhaps you could make
> `CompilerInstance::loadModule` treat all prebuilt modules as
> `MK_ExplicitModule`, or add a third `ModuleKind` for them?
I tried adding a ModuleKind at some point, but somehow it didn't work out. I
will take another look at that.
https://reviews.llvm.org/D23125
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
