Skip to content
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

[libc++][modules] Enable installation by default. #90094

Merged
merged 1 commit into from Apr 28, 2024

Conversation

mordante
Copy link
Member

This was suggested during the review of
#89413

This does not change the experimental state of modules.

@mordante mordante requested a review from a team as a code owner April 25, 2024 17:35
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Apr 25, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 25, 2024

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

This was suggested during the review of
#89413

This does not change the experimental state of modules.


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

15 Files Affected:

  • (modified) libcxx/CMakeLists.txt (+1-1)
  • (modified) libcxx/cmake/caches/Generic-cxx20.cmake (-1)
  • (modified) libcxx/cmake/caches/Generic-cxx23.cmake (-1)
  • (modified) libcxx/cmake/caches/Generic-cxx26.cmake (-1)
  • (modified) libcxx/cmake/caches/Generic-hardening-mode-extensive.cmake (-1)
  • (modified) libcxx/cmake/caches/Generic-no-exceptions.cmake (-1)
  • (modified) libcxx/cmake/caches/Generic-no-experimental.cmake (-1)
  • (modified) libcxx/cmake/caches/Generic-no-filesystem.cmake (-1)
  • (modified) libcxx/cmake/caches/Generic-no-localization.cmake (-1)
  • (modified) libcxx/cmake/caches/Generic-no-random_device.cmake (-1)
  • (modified) libcxx/cmake/caches/Generic-no-threads.cmake (-1)
  • (modified) libcxx/cmake/caches/Generic-no-unicode.cmake (-1)
  • (modified) libcxx/cmake/caches/Generic-no-wide-characters.cmake (-1)
  • (modified) libcxx/docs/BuildingLibcxx.rst (+1-1)
  • (modified) libcxx/docs/ReleaseNotes/19.rst (+2)
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 2977c26646cb2e..f34cb178e076e9 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -178,7 +178,7 @@ set(LIBCXX_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}" CACHE STRING
 option(LIBCXX_INSTALL_HEADERS "Install the libc++ headers." ON)
 option(LIBCXX_INSTALL_LIBRARY "Install the libc++ library." ON)
 option(LIBCXX_INSTALL_MODULES
-  "Install the libc++ C++20 module source files (experimental)." OFF
+  "Install the libc++ C++20 module source files (experimental)." ON
 )
 cmake_dependent_option(LIBCXX_INSTALL_STATIC_LIBRARY
   "Install the static libc++ library." ON
diff --git a/libcxx/cmake/caches/Generic-cxx20.cmake b/libcxx/cmake/caches/Generic-cxx20.cmake
index 641c131a737b18..3c44fdaf0e4253 100644
--- a/libcxx/cmake/caches/Generic-cxx20.cmake
+++ b/libcxx/cmake/caches/Generic-cxx20.cmake
@@ -1,3 +1,2 @@
-set(LIBCXX_INSTALL_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_TEST_PARAMS "std=c++20" CACHE STRING "")
 set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")
diff --git a/libcxx/cmake/caches/Generic-cxx23.cmake b/libcxx/cmake/caches/Generic-cxx23.cmake
index f5409e4652e429..bf88abf56ca6a4 100644
--- a/libcxx/cmake/caches/Generic-cxx23.cmake
+++ b/libcxx/cmake/caches/Generic-cxx23.cmake
@@ -1,3 +1,2 @@
-set(LIBCXX_INSTALL_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_TEST_PARAMS "std=c++23" CACHE STRING "")
 set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")
diff --git a/libcxx/cmake/caches/Generic-cxx26.cmake b/libcxx/cmake/caches/Generic-cxx26.cmake
index 2d9c018a4ff540..6ba9482af57851 100644
--- a/libcxx/cmake/caches/Generic-cxx26.cmake
+++ b/libcxx/cmake/caches/Generic-cxx26.cmake
@@ -1,3 +1,2 @@
-set(LIBCXX_INSTALL_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_TEST_PARAMS "std=c++26" CACHE STRING "")
 set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")
diff --git a/libcxx/cmake/caches/Generic-hardening-mode-extensive.cmake b/libcxx/cmake/caches/Generic-hardening-mode-extensive.cmake
index 9542dcdbf77834..72263dfd84635b 100644
--- a/libcxx/cmake/caches/Generic-hardening-mode-extensive.cmake
+++ b/libcxx/cmake/caches/Generic-hardening-mode-extensive.cmake
@@ -1,2 +1 @@
-set(LIBCXX_INSTALL_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_HARDENING_MODE "extensive" CACHE STRING "")
diff --git a/libcxx/cmake/caches/Generic-no-exceptions.cmake b/libcxx/cmake/caches/Generic-no-exceptions.cmake
index c68adfc1276b55..f0dffef60dba08 100644
--- a/libcxx/cmake/caches/Generic-no-exceptions.cmake
+++ b/libcxx/cmake/caches/Generic-no-exceptions.cmake
@@ -1,3 +1,2 @@
-set(LIBCXX_INSTALL_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
 set(LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
diff --git a/libcxx/cmake/caches/Generic-no-experimental.cmake b/libcxx/cmake/caches/Generic-no-experimental.cmake
index 62b7d7373d4478..f33ed01418990b 100644
--- a/libcxx/cmake/caches/Generic-no-experimental.cmake
+++ b/libcxx/cmake/caches/Generic-no-experimental.cmake
@@ -1,3 +1,2 @@
-set(LIBCXX_INSTALL_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_TEST_PARAMS "enable_experimental=False" CACHE STRING "")
 set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")
diff --git a/libcxx/cmake/caches/Generic-no-filesystem.cmake b/libcxx/cmake/caches/Generic-no-filesystem.cmake
index 01ae7e68f12c94..4000f3a3e8ef23 100644
--- a/libcxx/cmake/caches/Generic-no-filesystem.cmake
+++ b/libcxx/cmake/caches/Generic-no-filesystem.cmake
@@ -1,2 +1 @@
-set(LIBCXX_INSTALL_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
diff --git a/libcxx/cmake/caches/Generic-no-localization.cmake b/libcxx/cmake/caches/Generic-no-localization.cmake
index fc4957b2d53a71..79d6b44c7139aa 100644
--- a/libcxx/cmake/caches/Generic-no-localization.cmake
+++ b/libcxx/cmake/caches/Generic-no-localization.cmake
@@ -1,2 +1 @@
-set(LIBCXX_INSTALL_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_ENABLE_LOCALIZATION OFF CACHE BOOL "")
diff --git a/libcxx/cmake/caches/Generic-no-random_device.cmake b/libcxx/cmake/caches/Generic-no-random_device.cmake
index ddf479add6269c..e9b4cc60cc80ea 100644
--- a/libcxx/cmake/caches/Generic-no-random_device.cmake
+++ b/libcxx/cmake/caches/Generic-no-random_device.cmake
@@ -1,2 +1 @@
-set(LIBCXX_INSTALL_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_ENABLE_RANDOM_DEVICE OFF CACHE BOOL "")
diff --git a/libcxx/cmake/caches/Generic-no-threads.cmake b/libcxx/cmake/caches/Generic-no-threads.cmake
index 724fbc466b58a8..616baef1be7bef 100644
--- a/libcxx/cmake/caches/Generic-no-threads.cmake
+++ b/libcxx/cmake/caches/Generic-no-threads.cmake
@@ -1,4 +1,3 @@
-set(LIBCXX_INSTALL_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_ENABLE_THREADS OFF CACHE BOOL "")
 set(LIBCXXABI_ENABLE_THREADS OFF CACHE BOOL "")
 set(LIBCXX_ENABLE_MONOTONIC_CLOCK OFF CACHE BOOL "")
diff --git a/libcxx/cmake/caches/Generic-no-unicode.cmake b/libcxx/cmake/caches/Generic-no-unicode.cmake
index a4cf7dd73772b4..01160bf218981a 100644
--- a/libcxx/cmake/caches/Generic-no-unicode.cmake
+++ b/libcxx/cmake/caches/Generic-no-unicode.cmake
@@ -1,2 +1 @@
-set(LIBCXX_INSTALL_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_ENABLE_UNICODE OFF CACHE BOOL "")
diff --git a/libcxx/cmake/caches/Generic-no-wide-characters.cmake b/libcxx/cmake/caches/Generic-no-wide-characters.cmake
index dc19389bb5ae21..728d41086a3867 100644
--- a/libcxx/cmake/caches/Generic-no-wide-characters.cmake
+++ b/libcxx/cmake/caches/Generic-no-wide-characters.cmake
@@ -1,2 +1 @@
-set(LIBCXX_INSTALL_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_ENABLE_WIDE_CHARACTERS OFF CACHE BOOL "")
diff --git a/libcxx/docs/BuildingLibcxx.rst b/libcxx/docs/BuildingLibcxx.rst
index a0a0cdb4339749..e425b9dadfe7d1 100644
--- a/libcxx/docs/BuildingLibcxx.rst
+++ b/libcxx/docs/BuildingLibcxx.rst
@@ -208,7 +208,7 @@ libc++ specific options
 
 .. option:: LIBCXX_INSTALL_MODULES:BOOL
 
-  **Default**: ``OFF``
+  **Default**: ``ON``
 
   Toggle the installation of the experimental libc++ module sources.
 
diff --git a/libcxx/docs/ReleaseNotes/19.rst b/libcxx/docs/ReleaseNotes/19.rst
index 938ab76c6ecbdd..ac4fd0ecc122bd 100644
--- a/libcxx/docs/ReleaseNotes/19.rst
+++ b/libcxx/docs/ReleaseNotes/19.rst
@@ -149,3 +149,5 @@ Build System Changes
 
 - The Cmake variable ``LIBCXX_ENABLE_CLANG_TIDY`` has been removed. The build system has been changed
   to automatically detect the presence of ``clang-tidy`` and the required ``Clang`` libraries.
+
+- The CMake options ``LIBCXX_INSTALL_MODULES`` now defaults to ``ON``.

@mordante
Copy link
Member Author

@llvm/libcxx-vendors a heads-up

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

Does the documentation on Modules need to be updated?

This LGTM. It doesn't mean that vendors will start shipping modules by default, but I think it makes sense for the LLVM package to include them by default. This will allow build system providers to get the ball rolling on supporting this, like CMake did.

@ldionne ldionne self-assigned this Apr 26, 2024
@mordante
Copy link
Member Author

Does the documentation on Modules need to be updated?

This LGTM. It doesn't mean that vendors will start shipping modules by default, but I think it makes sense for the LLVM package to include them by default. This will allow build system providers to get the ball rolling on supporting this, like CMake did.

There is no need to update the documentation; the availability still depends on vendors to ship the newly installed files. I'd like to overhaul the documentation closer to the LLVM 19 release; for now I like to keep the information correct for LLVM 18 and 19. I see people reference the ToT modules document in various places (like reddit/slack) and not reference the LLVM 18 specific documents.

This was suggested during the review of
llvm#89413

This does not change the experimental state of modules.
@mordante mordante force-pushed the review/install_modules_by_default branch from b869e84 to facc526 Compare April 28, 2024 09:54
@mordante
Copy link
Member Author

The CI errors seem not to be related and the Buildkite build passed before rebasing.

@mordante mordante merged commit 19d2d3f into llvm:main Apr 28, 2024
51 of 52 checks passed
@mordante mordante deleted the review/install_modules_by_default branch April 28, 2024 12:12
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.

None yet

3 participants