[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

Chuanqi Xu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Jun 5 20:29:33 PDT 2022


ChuanqiXu added a comment.

In D126959#3556115 <https://reviews.llvm.org/D126959#3556115>, @iains wrote:

> In D126959#3556074 <https://reviews.llvm.org/D126959#3556074>, @tahonermann wrote:
>
>>> Implementation modules are never serialized (-emit-module-interface for an implementation unit is diagnosed and rejected).
>>
>> Never? Or just not via -emit-module-interface? I would expect to be able to serialize via -emit-ast.
>
> My current intent is that this is a implementation detail (a mechanism for maintaining the module purview and type for Sema and CG).
>
> As things stand, I do not think we are set up to handle (de-)serialization of two kinds of module with the same name, so I'd prefer to defer
> possible extension to allow this for now (not against the principle, just trying avoid a rabbit hole that's tangential to the current purpose).

I think Iain's "(de-)serialization" means "generating PCM files". And it makes no sense to block users to use `emit-ast` for implementation units. I don't think we have divergence here since I don't find codes which try to emit an error if we are attempting to generate a PCM file for an implementation unit.

---

I feel better if we could add some tests:

- As mentioned above, emit an error when we trying to generate a PCM file for an implementation unit.
- Currently, when we use implementation units, we need to specify `-fmodule-file=/path/to/the/pcm/of/primary/interface` in the command line. It looks like we could omit it after the patch (as long as the PCM file of primary interface lives in the path specified by `-fprebuilt-module-path`)



================
Comment at: clang/lib/Frontend/FrontendActions.cpp:835
+  case Module::ModuleImplementationUnit:
+    return "Implementation Unit (should never be serialized)";
   case Module::ModulePartitionInterface:
----------------
Since `ModuleKindName()` here is used as a helpful for printers, it looks odd to contain the `should never be serialized` part. I think we should check this when we try to generate PCM files for ModuleImplementationUnit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126959/new/

https://reviews.llvm.org/D126959



More information about the cfe-commits mailing list