[libcxx-commits] [PATCH] D144994: [Draft][libc++][modules] Adds std module.

[Aaron Siddhartha Mondal via Phabricator via libcxx-commits]

aaronmondal added inline comments.


================
Comment at: libcxx/CMakeLists.txt:422
   set(LIBCXX_GENERATED_INCLUDE_DIR "${LLVM_BINARY_DIR}/include/c++/v1")
+  set(LIBCXX_GENERATED_MODULE_DIR "${LLVM_BINARY_DIR}/include/c++/modules")
   set(LIBCXX_GENERATED_INCLUDE_TARGET_DIR "${LLVM_BINARY_DIR}/include/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
----------------
This seems like a good opportunity to finally give `v1` a friend 😊

Is there some resource on why/how the `v1` path came to be in the first place?

I'd expect this to {break/be incompatible with} quite a few {distros/setups/build sytems} as the path to `v1` is probably one of the most often hardcoded paths in history 😅.

Whatever the final name of this `modules` directory is, it would probably be a good idea to have some kind of "reasoning" behind it so that adopters (and other c++ standard libraries?) don't feel like this name was chosen arbitrarily.


================
Comment at: libcxx/docs/Modules.rst:36
+ * The ``std`` module is incomplete.
+ * There is no ``std.compat`` module is incomplete.
+ * Requires CMake 3.26 (soon to be released)
----------------



================
Comment at: libcxx/docs/Modules.rst:43
+ * There is no `P1689 style output <https://wg21.link/P1689>`_ yet.
+ * The experimental library (read <format>) does not work as expected
+
----------------



================
Comment at: libcxx/docs/Modules.rst:65
+   to discuss that approach before spending effort in it.
+ * Currently the ``cppm`` file and the generated ``CMakeLists.txt`` are not
+   installed. Before doing that it would be good to determine what the best
----------------
We intend to distribute all `.cppm` files, i.e. the partitions as well, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144994