[libcxx-commits] [PATCH] D151030: [libc++][modules] Adds std module cppm files.

[Aaron Siddhartha Mondal via Phabricator via libcxx-commits]

aaronmondal added inline comments.


================
Comment at: libcxx/modules/std/new.cppm:13
+
+export module std:__new; // Note new is a keyword and not a valid identifier
+export namespace std {
----------------
Mordante wrote:
> aaronmondal wrote:
> > I wonder whether it would be better to name this file `__new.cppm`, so that it's more straightforward to generate the module partition name from the filename. Otherwise this needs special treatment in build files.
> > 
> > Then again, that would make the filename inconsistent with the others. Ahh what a dilemma 😅
> For CMake I have a hard-coded list where I can use `__new.cppm` or `new.cppm`. (That CMake file's not part of this review since this review should be about adding these files and no logic.)
> 
> When this makes it easier for you I've no objection against renaming this file.
Hmm thinking about this some more I think renaming would be better. It's not exactly making integration harder, but it does add some avoidable complexity:

```
    interfaces = {
        "modules/std/{}.cppm".format(name): "std:{}".format(name)
        for name in STD_PARTITIONS
    } | {
        "modules/std/new.cppm": "std:__new",
    },
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151030