[libcxx-commits] [PATCH] D151814: [libc++][modules] Adds the C++23 std module.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked an inline comment as done.
Mordante added a comment.

In D151814#4390263 <https://reviews.llvm.org/D151814#4390263>, @Arthapz wrote:

> Hello, i'll add the support of this patch in XMake today :)

Great, thanks :-)

In D151814#4390266 <https://reviews.llvm.org/D151814#4390266>, @ChuanqiXu wrote:

> (BTW, we lack a `[` in the title.)

Thanks, fixed.



================
Comment at: libcxx/docs/Contributing.rst:38
 - Did you mark all functions and type declarations with the :ref:`proper visibility macro <visibility-macros>`?
+- Did you add all new named declarations to the ``std`` module?
 - If you added a header:
----------------
ChuanqiXu wrote:
> nit: in my imagination, in the future, this can be solved automatically:
> 
> ```
> #if ...
> import std;
> #else
> #include <stl_headers>
> #endif
> 
> use of new decl... // error if the author forget to touch std modules.
> ```
> 
> (this is not a requirement to change the note)
This is about adding new declarations to the `.cppm` files. For example when somebody implements parts of mdspan these new classes need to be added to the `.cppm` files. If you forget to do that, the CI will fail.


================
Comment at: libcxx/docs/Modules.rst:133-134
+  set(CMAKE_CXX_STANDARD_REQUIRED YES)
+  # Clang doesn't support compiler extensions for modules.
+  set(CMAKE_CXX_EXTENSIONS OFF)
+
----------------
ChuanqiXu wrote:
> This was mitigated recently: https://github.com/llvm/llvm-project/blob/15a719de01b92da7de4b8381660525b622c2c292/clang/lib/Driver/ToolChains/Clang.cpp#L3688-L3697. But personally I prefer `-std=c++xx` instead of `-std=gnu++xx`.
Thanks for the info. I too prefer to disable extensions. I'll remove these two lines.


================
Comment at: libcxx/docs/Modules.rst:164
+
+  # Implicit paths are not allowed in the future.
+  add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-fprebuilt-module-path=${CMAKE_BINARY_DIR}/_deps/std-build/CMakeFiles/std.dir/>)
----------------
ChuanqiXu wrote:
> This is not what we talked about implicit paths. The implicit paths is that the BMI will contain paths to their dependency implicitly. e.g,
> 
> 
> ```
> // b.cppm
> export module b;
> export int b() {
>     return 43;
> }
> 
> // a.cppm
> export module a;
> import b;
> export int a() {
>     return b() + 43;
> }
> 
> // user.cpp
> import a;
> int use() {
>     return a();
> }
> ```
> 
> Now it is ok to compiler `user.cpp` without specify the path to the BMI of module b. It is different from disabling `-fprebuilt-module-path`.
Interesting. When I removed the `fprebuilt-module-path` I got these warnings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151814