Skip to content

[libc++][doc] Use installed std modules in external projects. #80601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from

Conversation

a858438680
Copy link

The original libc++ document about using std modules in external projects is outdated. Loot at #80231 for more details.

This pull request update the document. The new document tells how to use the current installed std modules without toolchains' support.

@a858438680 a858438680 requested a review from a team as a code owner February 4, 2024 15:54
Copy link

github-actions bot commented Feb 4, 2024

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 4, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 4, 2024

@llvm/pr-subscribers-libcxx

Author: Runzhong Li (a858438680)

Changes

The original libc++ document about using std modules in external projects is outdated. Loot at #80231 for more details.

This pull request update the document. The new document tells how to use the current installed std modules without toolchains' support.


Full diff: https://github.com/llvm/llvm-project/pull/80601.diff

1 Files Affected:

  • (modified) libcxx/docs/Modules.rst (+74-37)
diff --git a/libcxx/docs/Modules.rst b/libcxx/docs/Modules.rst
index 533c3fbd2a1ee..5aeeb0dfc2460 100644
--- a/libcxx/docs/Modules.rst
+++ b/libcxx/docs/Modules.rst
@@ -105,36 +105,36 @@ Users need to be able to build their own BMI files.
    system vendors, with the goal that building the BMI files is done by
    the build system.
 
-Currently this requires a local build of libc++ with modules enabled. Since
-modules are not part of the installation yet, they are used from the build
-directory. First libc++ needs to be build with module support enabled.
+Currently this requires a local build of libc++ with modules installation enabled.
+Since modules are not installed by default. You can build and install the modules
+to ``<install_prefix>`` with the following commands.
 
 .. code-block:: bash
 
-  $ git clone https://github.com/llvm/llvm-project.git
+  $ git clone https://github.com/llvm/llvm-project.git --depth 1
   $ cd llvm-project
   $ mkdir build
-  $ cmake -G Ninja -S runtimes -B build -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind"
-  $ ninja -C build
-
-The above ``build`` directory will be referred to as ``<build>`` in the
-rest of these instructions.
+  $ cmake -G Ninja -S runtimes -B build -DCMAKE_C_COMPILER=<path-to-compiler> -DCMAKE_CXX_COMPILER=<path-to-compiler> -DLIBCXX_INSTALL_MODULES=ON -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind"
+  $ cmake --build build -- -j $(nproc)
+  $ cmake --install build --prefix <install_prefix>
 
 This is a small sample program that uses the module ``std``. It consists of a
-``CMakeLists.txt`` and a ``main.cpp`` file.
+``CMakeLists.txt``, an ``std.cmake``, and a ``main.cpp`` file.
 
 .. code-block:: cpp
 
+  // main.cpp
   import std; // When importing std.compat it's not needed to import std.
   import std.compat;
 
   int main() {
-    std::cout << "Hello modular world\n";
+    std::println("Hello modular world");
     ::printf("Hello compat modular world\n");
   }
 
 .. code-block:: cmake
 
+  # CMakeLists.txt
   cmake_minimum_required(VERSION 3.26.0 FATAL_ERROR)
   project("module"
     LANGUAGES CXX
@@ -168,58 +168,95 @@ This is a small sample program that uses the module ``std``. It consists of a
   #
   # Import the modules from libc++
   #
+  include(std.cmake)
+
+  add_executable(main main.cpp)
+
+.. code-block:: cmake
 
+  # std.cmake
   include(FetchContent)
   FetchContent_Declare(
-    std
-    URL "file://${LIBCXX_BUILD}/modules/c++/v1/"
+    std_module
+    URL "file://${LIBCXX_INSTALLED_DIR}/share/libc++/v1"
     DOWNLOAD_EXTRACT_TIMESTAMP TRUE
     SYSTEM
   )
-  FetchContent_MakeAvailable(std)
+
+  if (NOT std_module_POPULATED)
+    FetchContent_Populate(std_module)
+  endif()
 
   #
-  # Adjust project compiler flags
+  # Add std static library
   #
 
-  add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-fprebuilt-module-path=${std_BINARY_DIR}/CMakeFiles/std.dir/>)
-  add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-fprebuilt-module-path=${std_BINARY_DIR}/CMakeFiles/std.compat.dir/>)
-  add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-nostdinc++>)
-  # The include path needs to be set to be able to use macros from headers.
-  # For example from, the headers <cassert> and <version>.
-  add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-isystem>)
-  add_compile_options($<$<COMPILE_LANGUAGE:CXX>:${LIBCXX_BUILD}/include/c++/v1>)
+  add_library(std)
+
+  target_sources(std
+    PUBLIC FILE_SET cxx_modules TYPE CXX_MODULES FILES
+      ${std_module_SOURCE_DIR}/std.cppm
+      ${std_module_SOURCE_DIR}/std.compat.cppm
+  )
 
   #
-  # Adjust project linker flags
+  # Adjust project include directories
   #
 
-  add_link_options($<$<COMPILE_LANGUAGE:CXX>:-nostdlib++>)
-  add_link_options($<$<COMPILE_LANGUAGE:CXX>:-L${LIBCXX_BUILD}/lib>)
-  add_link_options($<$<COMPILE_LANGUAGE:CXX>:-Wl,-rpath,${LIBCXX_BUILD}/lib>)
-  # Linking against the standard c++ library is required for CMake to get the proper dependencies.
-  link_libraries(std c++)
-  link_libraries(std.compat c++)
+  target_include_directories(std SYSTEM PUBLIC ${LIBCXX_INSTALLED_DIR}/include/c++/v1)
 
   #
-  # Add the project
+  # Adjust project compiler flags
   #
 
-  add_executable(main)
-  target_sources(main
+  target_compile_options(std
     PRIVATE
-      main.cpp
+      -Wno-reserved-module-identifier
+      -Wno-reserved-user-defined-literal
+  )
+
+  target_compile_options(std
+    PUBLIC
+      -nostdinc++
+  )
+
+  #
+  # Adjust project linker flags
+  #
+
+  target_link_options(std
+    INTERFACE
+      -nostdlib++
+      -L${LIBCXX_INSTALLED_DIR}/lib
+      -Wl,-rpath,${LIBCXX_INSTALLED_DIR}/lib
+  )
+
+  target_link_libraries(std
+    INTERFACE
+      c++
   )
+  
+  #
+  # Link to the std modules by default
+  #
+
+  link_libraries(std)
 
 Building this project is done with the following steps, assuming the files
-``main.cpp`` and ``CMakeLists.txt`` are copied in the current directory.
+``main.cpp``, ``CMakeLists.txt``, and ``std.cmake`` are copied in the current directory.
 
 .. code-block:: bash
 
   $ mkdir build
-  $ cmake -G Ninja -S . -B build -DCMAKE_CXX_COMPILER=<path-to-compiler> -DLIBCXX_BUILD=<build>
-  $ ninja -C build
-  $ build/main
+  $ cmake -S . -B build -G Ninja -DCMAKE_CXX_COMPILER=<path-to-compiler> -DLIBCXX_INSTALLED_DIR=<install_prefix>
+  $ cmake --build build
+  $ ./build/main
+
+.. warning:: You need more than clang itself to build a project using modules.
+             Specifically, you will need ``clang-scan-deps``. For example, in Ubuntu, you
+             need to use ``sudo ./llvm.sh 17 all`` rather than ``sudo ./llvm.sh 17`` showed
+             in `LLVM Debian/Ubuntu nightly packages <https://apt.llvm.org>`__ to install
+             essential components to build this project.
 
 .. warning:: ``<path-to-compiler>`` should point point to the real binary and
              not to a symlink.

Comment on lines 108 to 110
Currently this requires a local build of libc++ with modules installation enabled.
Since modules are not installed by default. You can build and install the modules
to ``<install_prefix>`` with the following commands.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Currently this requires a local build of libc++ with modules installation enabled.
Since modules are not installed by default. You can build and install the modules
to ``<install_prefix>`` with the following commands.
This requires the system you're distributes the installed std modules. Otherwise...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your review. But I do not understand why you want me to delete these three lines. Could you please explain more in detail?
And also, The line you suggest me to add is not complete and seems to have a grammer error. Could you please tell me what you want to express?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. I mean it may be better to not say "the modules are not installed by default". Since there may be system vendors enable it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, got it. So you mean "requires the system you're using distributes ..." right? And "Otherwise, you need to build and install ..." right? I think this suggestion is fine. But I also want to hear from main author of the original page @mordante.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't change this in this fashion. The current described way for building is broken, this should be fixed. So that way remains a viable option. The goals of this patch is to add a second option based on installed modules.

# For example from, the headers <cassert> and <version>.
add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-isystem>)
add_compile_options($<$<COMPILE_LANGUAGE:CXX>:${LIBCXX_BUILD}/include/c++/v1>)
add_library(std)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing "STATIC"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add it

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch. I really like to keep the existing method and add a new method. I'll make a patch to fix to reintroduce the accidentally removed CMakeLists.txt.


.. code-block:: bash

$ git clone https://github.com/llvm/llvm-project.git
$ git clone https://github.com/llvm/llvm-project.git --depth 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to keep the instructions generic, if users like --depth 1 they are free to do so.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

Comment on lines 108 to 110
Currently this requires a local build of libc++ with modules installation enabled.
Since modules are not installed by default. You can build and install the modules
to ``<install_prefix>`` with the following commands.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't change this in this fashion. The current described way for building is broken, this should be fixed. So that way remains a viable option. The goals of this patch is to add a second option based on installed modules.

import std; // When importing std.compat it's not needed to import std.
import std.compat;

int main() {
std::cout << "Hello modular world\n";
std::println("Hello modular world");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please don't make unrelated changes in the same patch.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will change it back

@mordante
Copy link
Member

mordante commented Mar 3, 2024

@a858438680 are you still working on this patch?

@a858438680
Copy link
Author

@a858438680 are you still working on this patch?

Yes, but I missed your previous comments. I will make some changes according to your comments soon.

@a858438680
Copy link
Author

@mordante Hi, I have changed this commit according the your comments.

@mordante
Copy link
Member

mordante commented Mar 4, 2024

@mordante Hi, I have changed this commit according the your comments.

Thanks! Can you rebase the patch, there are merge conflicts. I prefer to review without these conflicts.

a858438680 and others added 6 commits March 5, 2024 02:37
This CMakeLists.txt is used to build modules without build system
support. This was removed in d06ae33.
This is used in the documentation how to use modules.

Made some minor changes to make it work with the std.compat module using
the std module.

Note the CMakeLists.txt in the build dir should be removed once build
system support is generally available.
I used `class` instead of `struct` for `SymbolMap`.
Fixes the `sanitizer-x86_64-linux-android` buildbot.
@a858438680
Copy link
Author

@mordante Hi, there is no conflict now.

@Endilll Endilll removed their request for review March 4, 2024 19:09
$ ninja -C build
$ cmake -G Ninja -S runtimes -B build -DLIBCXX_INSTALL_MODULES=ON -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind"
$ cmake --build build -- -j $(nproc)
$ cmake --install build --prefix <install_prefix>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really prefer to keep the ninja -C build and add ninja install -C build. This is the style we use in other parts of the libc++ documentation.

std
URL "file://${LIBCXX_BUILD}/modules/c++/v1/"
std_module
URL "file://${LIBCXX_INSTALLED_DIR}/share/libc++/v1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part of the example should use the ${LIBCXX_BUILD} right? Are the other parts of the changes in this part intended?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not intended. I'm not quite good at git. The rebase messed everything and I failed to recheck this. I will fix these things.

nolange added a commit to nolange/llvm-mingw that referenced this pull request Mar 14, 2024
Compilers and Buildsystems have early support for c++ modules,
see [1],[2].
`clang-scan-dep` is necessary for Integration in CMake, potentially
other buildsystems in the future.

Using standard modules like from libc++ is still not solved thoroughly,
and module support from libc++ is marked experimental, documentation
how to use them is WIP [3].

[1]: https://www.kitware.com/import-cmake-the-experiment-is-over
[2]: https://cmake.org/cmake/help/latest/manual/cmake-cxxmodules.7.html
[3]: llvm/llvm-project#80601
Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks I finally had time to have a good look. I tested in locally and the instructions work nicely :-)

There have been some other changes in this file, can you rebase before updating this review?


The above ``build`` directory will be referred to as ``<build>`` in the
rest of these instructions.

This is a small sample program that uses the module ``std``. It consists of a
``CMakeLists.txt`` and a ``main.cpp`` file.
This is a small sample program that uses the module ``std`` from build
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This is a small sample program that uses the module ``std`` from build
This is a small sample program that uses the module ``std`` from the build

Comment on lines +290 to +292
if (NOT std_module_POPULATED)
FetchContent_Populate(std_module)
endif()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the preferred way for CMake to do this.

Suggested change
if (NOT std_module_POPULATED)
FetchContent_Populate(std_module)
endif()
FetchContent_MakeAvailable(std_module)

Comment on lines +235 to +237
installation directory. It consists of a ``CMakeLists.txt``, an
``std.cmake``, and a ``main.cpp`` file. The ``main.cpp`` is the same as
the previous example.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
installation directory. It consists of a ``CMakeLists.txt``, an
``std.cmake``, and a ``main.cpp`` file. The ``main.cpp`` is the same as
the previous example.
the installation directory. It consists of a ``CMakeLists.txt``,
a ``std.cmake``, and the ``main.cpp`` from the previous example.

Comment on lines +359 to +364
.. warning:: You need more than clang itself to build a project using modules.
Specifically, you will need ``clang-scan-deps``. For example, in Ubuntu, you
need to use ``sudo ./llvm.sh 17 all`` rather than ``sudo ./llvm.sh 17`` showed
in `LLVM Debian/Ubuntu nightly packages <https://apt.llvm.org>`__ to install
essential components to build this project.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this part. This is also required when using the build directory.

mstorsjo pushed a commit to nolange/llvm-mingw that referenced this pull request Apr 12, 2024
Compilers and buildsystems have early support for c++ modules,
see [1], [2].
`clang-scan-dep` is necessary for integration in CMake, potentially
other buildsystems in the future.

Using standard modules like from libc++ is still not solved thoroughly,
and module support from libc++ is marked experimental, documentation
how to use them is WIP [3].

[1]: https://www.kitware.com/import-cmake-the-experiment-is-over
[2]: https://cmake.org/cmake/help/latest/manual/cmake-cxxmodules.7.html
[3]: llvm/llvm-project#80601
mstorsjo pushed a commit to mstorsjo/llvm-mingw that referenced this pull request Apr 12, 2024
Compilers and buildsystems have early support for c++ modules,
see [1], [2].
`clang-scan-dep` is necessary for integration in CMake, potentially
other buildsystems in the future.

Using standard modules like from libc++ is still not solved thoroughly,
and module support from libc++ is marked experimental, documentation
how to use them is WIP [3].

[1]: https://www.kitware.com/import-cmake-the-experiment-is-over
[2]: https://cmake.org/cmake/help/latest/manual/cmake-cxxmodules.7.html
[3]: llvm/llvm-project#80601
@mordante
Copy link
Member

CMake currently has experimental support for modules in their master branch. I've update our modules status page. Do you still want to pursue this patch? If not I'll close the patch.

@a858438680
Copy link
Author

a858438680 commented Apr 28, 2024

CMake currently has experimental support for modules in their master branch. I've update our modules status page. Do you still want to pursue this patch? If not I'll close the patch.

Actually I looked the newest document. And I think this patch is not maintainable, giving that cmake now has their official method. So I don't want to pursue this patch. It's completely ok for me to close this.

And Actually I tried to rebase this path to the main branch. But now I don't know how to do it.

@mordante
Copy link
Member

mordante commented May 1, 2024

CMake currently has experimental support for modules in their master branch. I've update our modules status page. Do you still want to pursue this patch? If not I'll close the patch.

Actually I looked the newest document. And I think this patch is not maintainable, giving that cmake now has their official method. So I don't want to pursue this patch. It's completely ok for me to close this.

Thanks then I'll close this.

And Actually I tried to rebase this path to the main branch. But now I don't know how to do it.

You need to resolve the conflicts to be able to rebase. If you want to know more about this, let me know.

@mordante mordante closed this May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants