[libc++][modules] Removes module testing. (#76083)

mordante · web-flow · commit d06ae33ec321 · 2024-01-17T08:11:25.000+01:00
This removes the entire modules testing infrastructure.

The current infrastructure uses CMake to generate the std and std.compat
module. This requires quite a bit of plumbing and uses CMake. Since
CMake introduced module support in CMake 3.26, modules have a higher
CMake requirement than the rest of the LLVM project. (The LLVM project
requires 3.20.) The main motivation for this approach was how libc++
generated its modules. Every header had its own module partition. This
was changed to improve performance and now only two modules remain. The
code to build these can be manually crafted.

A followup patch will reenable testing modules, using a different
approach.
diff --git a/.github/workflows/libcxx-build-and-test.yaml b/.github/workflows/libcxx-build-and-test.yaml
@@ -35,7 +35,6 @@ concurrency:
 
 
 env:
-  CMAKE: "/opt/bin/cmake"
   # LLVM POST-BRANCH bump version
   # LLVM POST-BRANCH add compiler test for ToT - 1, e.g. "Clang 17"
   # LLVM RELEASE bump remove compiler ToT - 3, e.g. "Clang 15"
@@ -169,24 +168,18 @@ jobs:
           'bootstrapping-build'
         ]
         machine: [ 'libcxx-runners-8-set' ]
-        std_modules: [ 'OFF' ]
         include:
         - config: 'generic-cxx26'
           machine: libcxx-runners-8-set
-          std_modules: 'ON'
         - config: 'generic-asan'
           machine: libcxx-runners-8-set
-          std_modules: 'OFF'
         - config: 'generic-tsan'
           machine: libcxx-runners-8-set
-          std_modules: 'OFF'
         - config: 'generic-ubsan'
           machine: libcxx-runners-8-set
-          std_modules: 'OFF'
         # Use a larger machine for MSAN to avoid timeout and memory allocation issues.
         - config: 'generic-msan'
           machine: libcxx-runners-8-set
-          std_modules: 'OFF'
     runs-on: ${{ matrix.machine }}
     steps:
       - uses: actions/checkout@v4
@@ -196,7 +189,6 @@ jobs:
           CC: clang-18
           CXX: clang++-18
           ENABLE_CLANG_TIDY: "OFF"
-          ENABLE_STD_MODULES: ${{ matrix.std_modules }}
       - uses: actions/upload-artifact@v3
         if: always()
         with:
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
@@ -117,12 +117,6 @@ option(LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS
    the shared library they shipped should turn this on and see `include/__availability`
    for more details." OFF)
 option(LIBCXX_ENABLE_CLANG_TIDY "Whether to compile and run clang-tidy checks" OFF)
-# TODO MODULES Remove this option and test for the requirements (CMake/Clang) instead.
-option(LIBCXX_ENABLE_STD_MODULES
-   "Whether to enable the building the C++23 `std` module. This feature is
-    experimental and has additional dependencies. Only enable this when
-    interested in testing or developing this module. See
-    https://libcxx.llvm.org/Modules.html for more information." OFF)
 
 if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
   set(LIBCXX_DEFAULT_TEST_CONFIG "llvm-libc++-shared-gcc.cfg.in")
@@ -772,7 +766,6 @@ config_define_if_not(LIBCXX_ENABLE_RANDOM_DEVICE _LIBCPP_HAS_NO_RANDOM_DEVICE)
 config_define_if_not(LIBCXX_ENABLE_LOCALIZATION _LIBCPP_HAS_NO_LOCALIZATION)
 config_define_if_not(LIBCXX_ENABLE_UNICODE _LIBCPP_HAS_NO_UNICODE)
 config_define_if_not(LIBCXX_ENABLE_WIDE_CHARACTERS _LIBCPP_HAS_NO_WIDE_CHARACTERS)
-config_define_if_not(LIBCXX_ENABLE_STD_MODULES _LIBCPP_HAS_NO_STD_MODULES)
 config_define_if_not(LIBCXX_ENABLE_TIME_ZONE_DATABASE _LIBCPP_HAS_NO_TIME_ZONE_DATABASE)
 config_define_if_not(LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS _LIBCPP_HAS_NO_VENDOR_AVAILABILITY_ANNOTATIONS)
 
@@ -856,19 +849,15 @@ endfunction()
 add_subdirectory(include)
 add_subdirectory(src)
 add_subdirectory(utils)
-if (LIBCXX_ENABLE_STD_MODULES)
-  add_subdirectory(modules)
-endif()
+add_subdirectory(modules)
 
 set(LIBCXX_TEST_DEPS "cxx_experimental")
 
 if (LIBCXX_ENABLE_CLANG_TIDY)
   list(APPEND LIBCXX_TEST_DEPS cxx-tidy)
 endif()
 
-if (LIBCXX_ENABLE_STD_MODULES)
-  list(APPEND LIBCXX_TEST_DEPS generate-cxx-modules generate-test-module-std)
-endif()
+list(APPEND LIBCXX_TEST_DEPS generate-cxx-modules)
 
 if (LIBCXX_INCLUDE_BENCHMARKS)
   add_subdirectory(benchmarks)
diff --git a/libcxx/cmake/caches/Generic-cxx26.cmake b/libcxx/cmake/caches/Generic-cxx26.cmake
@@ -1,3 +1,2 @@
-set(LIBCXX_ENABLE_STD_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
@@ -1,2 +1 @@
-set(LIBCXX_ENABLE_STD_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
@@ -1,3 +1,2 @@
-set(LIBCXX_ENABLE_STD_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
@@ -1,3 +1,2 @@
-set(LIBCXX_ENABLE_STD_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
@@ -1,2 +1 @@
-set(LIBCXX_ENABLE_STD_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
@@ -1,2 +1 @@
-set(LIBCXX_ENABLE_STD_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
@@ -1,2 +1 @@
-set(LIBCXX_ENABLE_STD_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
@@ -1,4 +1,3 @@
-set(LIBCXX_ENABLE_STD_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-tzdb.cmake b/libcxx/cmake/caches/Generic-no-tzdb.cmake
@@ -1,2 +1 @@
-set(LIBCXX_ENABLE_STD_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_ENABLE_TIME_ZONE_DATABASE OFF CACHE BOOL "")
diff --git a/libcxx/cmake/caches/Generic-no-unicode.cmake b/libcxx/cmake/caches/Generic-no-unicode.cmake
@@ -1,2 +1 @@
-set(LIBCXX_ENABLE_STD_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
@@ -1,2 +1 @@
-set(LIBCXX_ENABLE_STD_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.
 set(LIBCXX_ENABLE_WIDE_CHARACTERS OFF CACHE BOOL "")
diff --git a/libcxx/docs/Modules.rst b/libcxx/docs/Modules.rst
@@ -115,7 +115,7 @@ directory. First libc++ needs to be build with module support enabled.
   $ git clone https://github.com/llvm/llvm-project.git
   $ cd llvm-project
   $ mkdir build
-  $ cmake -G Ninja -S runtimes -B build -DLIBCXX_ENABLE_STD_MODULES=ON -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind"
+  $ 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
diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
@@ -89,6 +89,10 @@ Improvements and New Features
 - The ``_LIBCPP_ENABLE_CXX20_REMOVED_SHARED_PTR_UNIQUE`` macro has been added to make
   the function ``std::shared_ptr<...>::unique()`` available.
 
+- The cmake option ``LIBCXX_ENABLE_STD_MODULES`` has been removed. The test
+  infrastructure no longer depends on a modern CMake, it works with the minimal
+  required LLVM version (3.20.0).
+
 
 Deprecations and Removals
 -------------------------
diff --git a/libcxx/modules/CMakeLists.txt b/libcxx/modules/CMakeLists.txt
@@ -1,8 +1,3 @@
-if (CMAKE_VERSION VERSION_LESS 3.26)
-  message(WARNING "The libc++ modules won't be available because the CMake version is too old. Update to CMake 3.26 or later.")
-  return()
-endif()
-
 # The headers of Table 24: C++ library headers [tab:headers.cpp]
 # and the headers of Table 25: C++ headers for C library facilities [tab:headers.cpp.c]
 set(LIBCXX_MODULE_STD_SOURCES
@@ -142,28 +137,6 @@ set(LIBCXX_MODULE_STD_COMPAT_SOURCES
   std.compat/cwctype.inc
 )
 
-# TODO MODULES the CMakeLists.txt in the install directory is only temporary
-# When that is removed the configured file can use the substitution
-# LIBCXX_GENERATED_INCLUDE_TARGET_DIR avoiding this set.
-# Also clean up the parts needed to generate the install version.
-# - LIBCXX_GENERATED_INCLUDE_DIR contains the libc++ headers
-# - LIBCXX_GENERATED_INCLUDE_TARGET_DIR contains the libc++ site config
-if ("${LIBCXX_GENERATED_INCLUDE_DIR}" STREQUAL "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}")
-  # This typically happens when the target is not installed.
-  set(LIBCXX_CONFIGURED_INCLUDE_DIRS "${LIBCXX_GENERATED_INCLUDE_DIR}")
-else()
-  # It's important that the arch directory be included first so that its header files
-  # which interpose on the default include dir be included instead of the default ones.
-  set(LIBCXX_CONFIGURED_INCLUDE_DIRS
-    "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR};${LIBCXX_GENERATED_INCLUDE_DIR}"
-  )
-endif()
-configure_file(
-  "CMakeLists.txt.in"
-  "${LIBCXX_GENERATED_MODULE_DIR}/CMakeLists.txt"
-  @ONLY
-)
-
 set(LIBCXX_MODULE_STD_INCLUDE_SOURCES)
 foreach(file ${LIBCXX_MODULE_STD_SOURCES})
   set(
@@ -193,7 +166,6 @@ configure_file(
 )
 
 set(_all_modules)
-list(APPEND _all_modules "${LIBCXX_GENERATED_MODULE_DIR}/CMakeLists.txt")
 list(APPEND _all_modules "${LIBCXX_GENERATED_MODULE_DIR}/std.cppm")
 list(APPEND _all_modules "${LIBCXX_GENERATED_MODULE_DIR}/std.compat.cppm")
 foreach(file ${LIBCXX_MODULE_STD_SOURCES} ${LIBCXX_MODULE_STD_COMPAT_SOURCES})
diff --git a/libcxx/modules/CMakeLists.txt.in b/libcxx/modules/CMakeLists.txt.in
diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
@@ -87,31 +87,6 @@ if (LIBCXX_INCLUDE_TESTS)
     ${CMAKE_CURRENT_BINARY_DIR}
     DEPENDS cxx-test-depends)
 
-  if(LIBCXX_ENABLE_STD_MODULES)
-    # Generates the modules used in the test.
-    # Note the test will regenerate this with the proper setting
-    # - the right DCMAKE_CXX_STANDARD
-    # - the right test compilation flags
-    # Since modules depend on these flags there currently is no way to
-    # avoid generating these for the tests. The advantage of the
-    # pre generation is that less build information needs to be shared
-    # in the bridge.
-    add_custom_command(
-        OUTPUT "${CMAKE_BINARY_DIR}/test/__config_module__/CMakeCache.txt"
-        COMMAND
-        ${CMAKE_COMMAND}
-            "-G${CMAKE_GENERATOR}"
-            "-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}"
-            "-B${CMAKE_BINARY_DIR}/test/__config_module__"
-            "-H${LIBCXX_GENERATED_MODULE_DIR}"
-            "-DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}"
-            "-DCMAKE_CXX_STANDARD=23"
-            "-DCMAKE_EXPORT_COMPILE_COMMANDS=ON"
-            )
-  add_custom_target(generate-test-module-std
-      DEPENDS "${CMAKE_BINARY_DIR}/test/__config_module__/CMakeCache.txt"
-      COMMENT "Builds generic module std.")
-  endif()
 endif()
 
 if (LIBCXX_GENERATE_COVERAGE)
diff --git a/libcxx/test/configs/cmake-bridge.cfg.in b/libcxx/test/configs/cmake-bridge.cfg.in
@@ -31,10 +31,3 @@ config.substitutions.append(('%{target-include}', '@LIBCXX_GENERATED_INCLUDE_TAR
 config.substitutions.append(('%{lib}', '@LIBCXX_LIBRARY_DIR@'))
 config.substitutions.append(('%{module}', '@LIBCXX_GENERATED_MODULE_DIR@'))
 config.substitutions.append(('%{test-tools}', '@LIBCXX_TEST_TOOLS_PATH@'))
-
-# The test needs to manually rebuild the module. The compiler flags used in the
-# test need to be the same as the compiler flags used to generate the module.
-# In the future, when CMake can generated modules this may no longer be
-# necessary.
-# TODO MODULES whether it's possible to remove this substitution.
-config.substitutions.append(('%{cmake}', '@CMAKE_COMMAND@'))
diff --git a/libcxx/test/lit.local.cfg b/libcxx/test/lit.local.cfg
diff --git a/libcxx/test/std/modules/std.compat.pass.cpp b/libcxx/test/std/modules/std.compat.pass.cpp
diff --git a/libcxx/test/std/modules/std.pass.cpp b/libcxx/test/std/modules/std.pass.cpp
diff --git a/libcxx/utils/ci/Dockerfile b/libcxx/utils/ci/Dockerfile
diff --git a/libcxx/utils/ci/buildkite-pipeline.yml b/libcxx/utils/ci/buildkite-pipeline.yml
diff --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
diff --git a/libcxx/utils/libcxx/test/modules.py b/libcxx/utils/libcxx/test/modules.py

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,2 @@`
`1`		`-set(LIBCXX_ENABLE_STD_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.`
`2`	`1`	`set(LIBCXX_TEST_PARAMS "std=c++26" CACHE STRING "")`
`3`	`2`	`set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1 @@`
`1`		`-set(LIBCXX_ENABLE_STD_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.`
`2`	`1`	`set(LIBCXX_HARDENING_MODE "extensive" CACHE STRING "")`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,2 @@`
`1`		`-set(LIBCXX_ENABLE_STD_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.`
`2`	`1`	`set(LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "")`
`3`	`2`	`set(LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL "")`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,2 @@`
`1`		`-set(LIBCXX_ENABLE_STD_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.`
`2`	`1`	`set(LIBCXX_TEST_PARAMS "enable_experimental=False" CACHE STRING "")`
`3`	`2`	`set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1 @@`
`1`		`-set(LIBCXX_ENABLE_STD_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.`
`2`	`1`	`set(LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1 @@`
`1`		`-set(LIBCXX_ENABLE_STD_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.`
`2`	`1`	`set(LIBCXX_ENABLE_LOCALIZATION OFF CACHE BOOL "")`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1 @@`
`1`		`-set(LIBCXX_ENABLE_STD_MODULES ON CACHE BOOL "") # TODO MODULES Remove when enabled automatically.`
`2`	`1`	`set(LIBCXX_ENABLE_RANDOM_DEVICE OFF CACHE BOOL "")`