[libcxx-commits] [libcxx] Rework Modules CMake to be (more) idiomatic. (PR #84936)

[via libcxx-commits]

https://github.com/EricWF created https://github.com/llvm/llvm-project/pull/84936

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


>From 91fe687ef666e809b3759f93a66fedb311ca28fb Mon Sep 17 00:00:00 2001
From: Eric Fiselier <eric at efcs.ca>
Date: Tue, 12 Mar 2024 11:18:23 -0400
Subject: [PATCH 1/2] Fix bug in documentation regarding dependencies.

The docs for building modules suggest you have to build with -j1 and
build the std modules first. Fortunetly, that's just a bug in the
documentation. By correctly listing the libraries as dependencies,
the build system will order the builds correctly.
---
 libcxx/docs/Modules.rst | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/libcxx/docs/Modules.rst b/libcxx/docs/Modules.rst
index ee2b81d3b9e7ca..fc267cb8761b1e 100644
--- a/libcxx/docs/Modules.rst
+++ b/libcxx/docs/Modules.rst
@@ -206,6 +206,8 @@ This is a small sample program that uses the module ``std``. It consists of a
   #
 
   add_executable(main)
+  add_dependencies(main std)
+  target_link_libraries(std)
   target_sources(main
     PRIVATE
       main.cpp
@@ -218,13 +220,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.
 

>From d0d0104889457429e0f21172ac3bff29d5707bf6 Mon Sep 17 00:00:00 2001
From: Eric Fiselier <eric at efcs.ca>
Date: Tue, 12 Mar 2024 11:47:19 -0400
Subject: [PATCH 2/2] Rework Modules CMake to be (more) idiomatic.

This change adjusts the documentation and module CMake file to
remove unneeded configuration options.

First, in the documentation it states that the user has to set up a
bunch of flags, but they don't actually need to do that. The flags
they need should be part of the PUBLIC flagset provided by the
std.compat and std targets.

Second, CMake knowns how to find and make available the modules it
compiles. We don't need to add extra flags for that, CMake just does it.

I've tested this with Ninja and CMake 3.26, and 3.29 (though not on the
most recent revision)
---
 libcxx/docs/Modules.rst          | 27 ++-------------------------
 libcxx/modules/CMakeLists.txt.in | 16 ++++++++++++----
 2 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/libcxx/docs/Modules.rst b/libcxx/docs/Modules.rst
index fc267cb8761b1e..5b027ed1bd0729 100644
--- a/libcxx/docs/Modules.rst
+++ b/libcxx/docs/Modules.rst
@@ -178,36 +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)
-  target_link_libraries(std)
+  add_dependencies(main std.compat)
+  target_link_libraries(main std.compat)
   target_sources(main
     PRIVATE
       main.cpp
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++)