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

Tue Mar 28 20:29:34 PDT 2023

iains marked an inline comment as done.
iains added inline comments.

================
Comment at: clang/lib/Lex/ModuleMap.cpp:935
+  // with any legal user-defined module name).
+  StringRef IName = ".ImplementationUnit";
+  assert(!Modules[IName] && "multiple implementation units?");
----------------
ChuanqiXu wrote:
> nit: It should more consistent to use `<ImplementationUnit>`.
(I do not mind making this change later, if you like - but I did not want to repeat the test cycle today).

In this case I think that `<>` would not mean the same thing as it does in `<global>` and `<private>` modules fragments,  These are never entered into the module map - but are always owned by a parent module.

In the case of the ImplementationUnit - the *name* of the module is unchanged (i.e. it would still be called `M`  when the module line is ` module M;`.

The `.ImplementationUnit` is not the name of the module - but rather it is a key into the module map (which needs to be different from the name of the interface),  Since there can only be one Implementation Unit in a given session, it is safe to use a fixed key. 

However, I do not mind changing the key to `<ImplementationUnit>` if you think that it would be more clear.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126959