Rework Modules CMake to be (more) idiomatic.

[EricWF]

• Fix bug in documentation regarding dependencies.
• Rework Modules CMake to be (more) idiomatic.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Eric (EricWF)

Changes

• Fix bug in documentation regarding dependencies.
• Rework Modules CMake to be (more) idiomatic.

---

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

2 Files Affected:

• (modified) libcxx/docs/Modules.rst (+2-27)
• (modified) libcxx/modules/CMakeLists.txt.in (+12-4)

diff --git a/libcxx/docs/Modules.rst b/libcxx/docs/Modules.rst
index ee2b81d3b9e7ca..5b027ed1bd0729 100644
--- a/libcxx/docs/Modules.rst
+++ b/libcxx/docs/Modules.rst
@@ -178,34 +178,13 @@ This is a small sample program that uses the module ``std``. It consists of a
   )
   FetchContent_MakeAvailable(std)
 
-  #
-  # Adjust project compiler flags
-  #
-
-  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>)
-
-  #
-  # Adjust project linker flags
-  #
-
-  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++)
-
   #
   # Add the project
   #
 
   add_executable(main)
+  add_dependencies(main std.compat)
+  target_link_libraries(main std.compat)
   target_sources(main
     PRIVATE
       main.cpp
@@ -218,13 +197,9 @@ Building this project is done with the following steps, assuming the files
 
   $ mkdir build
   $ cmake -G Ninja -S . -B build -DCMAKE_CXX_COMPILER=<path-to-compiler> -DLIBCXX_BUILD=<build>
-  $ ninja -j1 std -C build
   $ ninja -C build
   $ build/main
 
-.. note:: The ``std`` dependencies of ``std.compat`` is not always resolved when
-          building the ``std`` target using multiple jobs.
-
 .. warning:: ``<path-to-compiler>`` should point point to the real binary and
              not to a symlink.
 
diff --git a/libcxx/modules/CMakeLists.txt.in b/libcxx/modules/CMakeLists.txt.in
index e332d70cc16333..459b3937f09325 100644
--- a/libcxx/modules/CMakeLists.txt.in
+++ b/libcxx/modules/CMakeLists.txt.in
@@ -50,10 +50,15 @@ endif()
 target_compile_options(std
   PUBLIC
     -nostdinc++
+    @LIBCXX_COMPILE_FLAGS@
+)
+target_compile_options(std
+  PRIVATE
     -Wno-reserved-module-identifier
     -Wno-reserved-user-defined-literal
-    @LIBCXX_COMPILE_FLAGS@
 )
+target_link_options(std PUBLIC -nostdlib++ -Wl,-rpath,${LIBCXX_BUILD}/lib/x86_64-unknown-linux-gnu/ -L${LIBCXX_BUILD}/lib/x86_64-unknown-linux-gnu)
+target_link_libraries(std c++)
 set_target_properties(std
   PROPERTIES
     OUTPUT_NAME   "c++std"
@@ -67,7 +72,7 @@ target_sources(std.compat
     std.compat.cppm
 )
 
-target_include_directories(std.compat SYSTEM PRIVATE @LIBCXX_CONFIGURED_INCLUDE_DIRS@)
+target_include_directories(std.compat SYSTEM PUBLIC @LIBCXX_CONFIGURED_INCLUDE_DIRS@)
 
 if (NOT @LIBCXX_ENABLE_EXCEPTIONS@)
   target_compile_options(std.compat PUBLIC -fno-exceptions)
@@ -76,13 +81,16 @@ endif()
 target_compile_options(std.compat
   PUBLIC
     -nostdinc++
+    @LIBCXX_COMPILE_FLAGS@
+)
+target_compile_options(std.compat
+ PRIVATE
     -Wno-reserved-module-identifier
     -Wno-reserved-user-defined-literal
-	-fmodule-file=std=${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/std.dir/std.pcm
-    @LIBCXX_COMPILE_FLAGS@
 )
 set_target_properties(std.compat
   PROPERTIES
     OUTPUT_NAME   "c++std.compat"
 )
 add_dependencies(std.compat std)
+target_link_libraries(std.compat PUBLIC std c++)

[mordante]

Thanks for this improvement. Does the current version still work when using modules from the build directory?

[EricWF]

Thanks for this improvement. Does the current version still work when using modules from the build directory?

It should. though now the generated cmake lists isn't dependent on LIBCXX_BUILD, and instead just hardcodes all of the paths to the build directory.

Ideally we would just have the user link and build against a system libc++, so all that's needed is -stdlib=libc++

[mordante]

Thanks this looks like a nice simplification! I've tested it locally and it works.
LGTM!

[EricWF]

Thanks Mark. More to come.

[EricWF]

I think the next step is to actually build a module during the CMake build