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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
