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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu 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?");
----------------
iains wrote:
> 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.
> 
Thanks, I think it doesn't matter too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126959