[libc++][modules] Adds module testing. #76246
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
libc++
|
@llvm/pr-subscribers-github-workflow Author: Mark de Wever (mordante) ChangesPatch is 38.03 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/76246.diff 35 Files Affected:
diff --git a/.github/workflows/libcxx-build-and-test.yaml b/.github/workflows/libcxx-build-and-test.yaml
index 370cf830a60cf8..a7aeeb1b0d59d2 100644
--- 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-32-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
index 75cb63222da35c..0ab900e7df3c51 100644
--- 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,9 +849,7 @@ 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")
@@ -866,9 +857,7 @@ 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
index f48d72d493c2f5..6ba9482af57851 100644
--- 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
index 0487377d4e9ba2..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_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
index f405f7fe993752..f0dffef60dba08 100644
--- 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
index fe14e7afed7b96..f33ed01418990b 100644
--- 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
index db62f86854d941..4000f3a3e8ef23 100644
--- 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
index 54a7ec3f1f5b36..79d6b44c7139aa 100644
--- 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
index adfa2458a8edf6..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_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
index 2aeab22915e00c..616baef1be7bef 100644
--- 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
index c5dc882e584428..27c826edfecffb 100644
--- 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
index 880e2d502ad91b..01160bf218981a 100644
--- 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
index 5036f6abd52e83..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_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
index 5099e6095582cf..1998cd9d1d267e 100644
--- 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
index 79608c631f1e62..45e12cec44e510 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -83,6 +83,10 @@ Improvements and New Features
- The ``_LIBCPP_ENABLE_CXX26_REMOVED_STRING_RESERVE`` macro has been added to make
the function ``std::basic_string<...>::reserve()`` 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
index fae6448a7eec84..31fbadf449f773 100644
--- 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
deleted file mode 100644
index 98168673ebfe9c..00000000000000
--- a/libcxx/modules/CMakeLists.txt.in
+++ /dev/null
@@ -1,86 +0,0 @@
-cmake_minimum_required(VERSION 3.26)
-
-project(libc++-modules LANGUAGES CXX)
-
-# Enable CMake's module support
-if(CMAKE_VERSION VERSION_LESS "3.28.0")
- if(CMAKE_VERSION VERSION_LESS "3.27.0")
- set(CMAKE_EXPERIMENTAL_CXX_MODULE_CMAKE_API "2182bf5c-ef0d-489a-91da-49dbc3090d2a")
- else()
- set(CMAKE_EXPERIMENTAL_CXX_MODULE_CMAKE_API "aa1f7df0-828a-4fcd-9afc-2dc80491aca7")
- endif()
- set(CMAKE_EXPERIMENTAL_CXX_MODULE_DYNDEP 1)
-else()
- cmake_policy(VERSION 3.28)
-endif()
-
-# Default to C++ extensions being off. Libc++'s modules support have trouble
-# with extensions right now.
-set(CMAKE_CXX_EXTENSIONS OFF)
-
-# Propagates the CMake options to the modules.
-#
-# This uses the std module hard-coded since the std.compat module does not
-# depend on these flags.
-macro(compile_define_if_not condition def)
- if (NOT ${condition})
- target_compile_definitions(std PRIVATE ${def})
- endif()
-endmacro()
-macro(compile_define_if condition def)
- if (${condition})
- target_compile_definitions(std PRIVATE ${def})
- endif()
-endmacro()
-
-### STD
-
-add_library(std)
-target_sources(std
- PUBLIC FILE_SET cxx_modules TYPE CXX_MODULES FILES
- std.cppm
-)
-
-target_include_directories(std SYSTEM PRIVATE @LIBCXX_CONFIGURED_INCLUDE_DIRS@)
-
-if (NOT @LIBCXX_ENABLE_EXCEPTIONS@)
- target_compile_options(std PUBLIC -fno-exceptions)
-endif()
-
-target_compile_options(std
- PUBLIC
- -nostdinc++
- -Wno-reserved-module-identifier
- -Wno-reserved-user-defined-literal
- @LIBCXX_COMPILE_FLAGS@
-)
-set_target_properties(std
- PROPERTIES
- OUTPUT_NAME "c++std"
-)
-
-### STD.COMPAT
-
-add_library(std.compat)
-target_sources(std.compat
- PUBLIC FILE_SET cxx_modules TYPE CXX_MODULES FILES
- std.compat.cppm
-)
-
-target_include_directories(std.compat SYSTEM PRIVATE @LIBCXX_CONFIGURED_INCLUDE_DIRS@)
-
-if (NOT @LIBCXX_ENABLE_EXCEPTIONS@)
- target_compile_options(std.compat PUBLIC -fno-exceptions)
-endif()
-
-target_compile_options(std.compat
- PUBLIC
- -nostdinc++
- -Wno-reserved-module-identifier
- -Wno-reserved-user-defined-literal
- @LIBCXX_COMPILE_FLAGS@
-)
-set_target_properties(std.compat
- PROPERTIES
- OUTPUT_NAME "c++std.compat"
-)
diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
index 48dd233462ab3b..52620fc55feeb7 100644
--- 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
index 0e3c3040c96446..72b2ddf378bb64 100644
--- 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/libcxx/module_std.gen.py b/libcxx/test/libcxx/module_std.gen.py
index 8e03d6e5b5b523..3ad2aff9d085f4 100644
--- a/libcxx/test/libcxx/module_std.gen.py
+++ b/libcxx/test/libcxx/module_std.gen.py
@@ -29,7 +29,8 @@
"%{clang-tidy}",
"%{test-tools}/clang_tidy_checks/libcxx-tidy.plugin",
"%{cxx}",
- "%{flags} %{compile_flags}",
+ "%{flags} %{compile_flags} %{module_flags}",
+ "std",
)
diff --git a/libcxx/test/libcxx/module_std_compat.gen.py b/libcxx/test/libcxx/module_std_compat.gen.py
index c4792db3d71e62..63fdd8188937e1 100644
--- a/libcxx/test/libcxx/module_std_compat.gen.py
+++ b/libcxx/test/libcxx/module_std_compat.gen.py
@@ -29,7 +29,8 @@
"%{clang-tidy}",
"%{test-tools}/clang_tidy_checks/libcxx-tidy.plugin",
"%{cxx}",
- "%{flags} %{compile_flags}",
+ "%{flags} %{compile_flags} %{module_flags}",
+ "std.compat",
)
diff --git a/libcxx/test/libcxx/selftest/modules/no-modules.sh.cpp b/libcxx/test/libcxx/selftest/modules/no-modules.sh.cpp
new file mode 100644
index 00000000000000..86d0afc13e3c40
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/modules/no-modules.sh.cpp
@@ -0,0 +1,12 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// Make sure that the module flags are empty when no module is supplied.
+
+// MODULES:
+// RUN: echo "%{module_flags}" | grep "^$"
diff --git a/libcxx/test/libcxx/selftest/modules/std-and-std.compat-module.sh.cpp b/libcxx/test/libcxx/selftest/modules/std-and-std.compat-module.sh.cpp
new file mode 100644
index 00000000000000..b9be1d40614aa0
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/modules/std-and-std.compat-module.sh.cpp
@@ -0,0 +1,17 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// Make sure that the module flags contain the expected elements.
+// The tests only look for the expected components and not the exact flags.
+// Otherwise changing the location of the module breaks this test.
+
+// MODULES: std std.compat
+//
+// RUN: echo "%{module_flags}" | grep -- "-fprebuilt-module-path="
+// RUN: echo "%{module_flags}" | grep "std.pcm"
+// RUN: echo "%{module_flags}" | grep "std.compat.pcm"
diff --git a/libcxx/test/libcxx/selftest/modules/std-module.sh.cpp b/libcxx/test/libcxx/selftest/modules/std-module.sh.cpp
new file mode 100644
index 00000000000000..6007511bd854a3
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/modules/std-module.sh.cpp
@@ -0,0 +1,19 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//=...
[truncated]
|
|
@llvm/pr-subscribers-libcxx Author: Mark de Wever (mordante) ChangesPatch is 38.03 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/76246.diff 35 Files Affected:
diff --git a/.github/workflows/libcxx-build-and-test.yaml b/.github/workflows/libcxx-build-and-test.yaml
index 370cf830a60cf8..a7aeeb1b0d59d2 100644
--- 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-32-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
index 75cb63222da35c..0ab900e7df3c51 100644
--- 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,9 +849,7 @@ 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")
@@ -866,9 +857,7 @@ 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
index f48d72d493c2f5..6ba9482af57851 100644
--- 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
index 0487377d4e9ba2..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_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
index f405f7fe993752..f0dffef60dba08 100644
--- 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
index fe14e7afed7b96..f33ed01418990b 100644
--- 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
index db62f86854d941..4000f3a3e8ef23 100644
--- 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
index 54a7ec3f1f5b36..79d6b44c7139aa 100644
--- 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
index adfa2458a8edf6..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_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
index 2aeab22915e00c..616baef1be7bef 100644
--- 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
index c5dc882e584428..27c826edfecffb 100644
--- 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
index 880e2d502ad91b..01160bf218981a 100644
--- 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
index 5036f6abd52e83..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_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
index 5099e6095582cf..1998cd9d1d267e 100644
--- 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
index 79608c631f1e62..45e12cec44e510 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -83,6 +83,10 @@ Improvements and New Features
- The ``_LIBCPP_ENABLE_CXX26_REMOVED_STRING_RESERVE`` macro has been added to make
the function ``std::basic_string<...>::reserve()`` 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
index fae6448a7eec84..31fbadf449f773 100644
--- 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
deleted file mode 100644
index 98168673ebfe9c..00000000000000
--- a/libcxx/modules/CMakeLists.txt.in
+++ /dev/null
@@ -1,86 +0,0 @@
-cmake_minimum_required(VERSION 3.26)
-
-project(libc++-modules LANGUAGES CXX)
-
-# Enable CMake's module support
-if(CMAKE_VERSION VERSION_LESS "3.28.0")
- if(CMAKE_VERSION VERSION_LESS "3.27.0")
- set(CMAKE_EXPERIMENTAL_CXX_MODULE_CMAKE_API "2182bf5c-ef0d-489a-91da-49dbc3090d2a")
- else()
- set(CMAKE_EXPERIMENTAL_CXX_MODULE_CMAKE_API "aa1f7df0-828a-4fcd-9afc-2dc80491aca7")
- endif()
- set(CMAKE_EXPERIMENTAL_CXX_MODULE_DYNDEP 1)
-else()
- cmake_policy(VERSION 3.28)
-endif()
-
-# Default to C++ extensions being off. Libc++'s modules support have trouble
-# with extensions right now.
-set(CMAKE_CXX_EXTENSIONS OFF)
-
-# Propagates the CMake options to the modules.
-#
-# This uses the std module hard-coded since the std.compat module does not
-# depend on these flags.
-macro(compile_define_if_not condition def)
- if (NOT ${condition})
- target_compile_definitions(std PRIVATE ${def})
- endif()
-endmacro()
-macro(compile_define_if condition def)
- if (${condition})
- target_compile_definitions(std PRIVATE ${def})
- endif()
-endmacro()
-
-### STD
-
-add_library(std)
-target_sources(std
- PUBLIC FILE_SET cxx_modules TYPE CXX_MODULES FILES
- std.cppm
-)
-
-target_include_directories(std SYSTEM PRIVATE @LIBCXX_CONFIGURED_INCLUDE_DIRS@)
-
-if (NOT @LIBCXX_ENABLE_EXCEPTIONS@)
- target_compile_options(std PUBLIC -fno-exceptions)
-endif()
-
-target_compile_options(std
- PUBLIC
- -nostdinc++
- -Wno-reserved-module-identifier
- -Wno-reserved-user-defined-literal
- @LIBCXX_COMPILE_FLAGS@
-)
-set_target_properties(std
- PROPERTIES
- OUTPUT_NAME "c++std"
-)
-
-### STD.COMPAT
-
-add_library(std.compat)
-target_sources(std.compat
- PUBLIC FILE_SET cxx_modules TYPE CXX_MODULES FILES
- std.compat.cppm
-)
-
-target_include_directories(std.compat SYSTEM PRIVATE @LIBCXX_CONFIGURED_INCLUDE_DIRS@)
-
-if (NOT @LIBCXX_ENABLE_EXCEPTIONS@)
- target_compile_options(std.compat PUBLIC -fno-exceptions)
-endif()
-
-target_compile_options(std.compat
- PUBLIC
- -nostdinc++
- -Wno-reserved-module-identifier
- -Wno-reserved-user-defined-literal
- @LIBCXX_COMPILE_FLAGS@
-)
-set_target_properties(std.compat
- PROPERTIES
- OUTPUT_NAME "c++std.compat"
-)
diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
index 48dd233462ab3b..52620fc55feeb7 100644
--- 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
index 0e3c3040c96446..72b2ddf378bb64 100644
--- 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/libcxx/module_std.gen.py b/libcxx/test/libcxx/module_std.gen.py
index 8e03d6e5b5b523..3ad2aff9d085f4 100644
--- a/libcxx/test/libcxx/module_std.gen.py
+++ b/libcxx/test/libcxx/module_std.gen.py
@@ -29,7 +29,8 @@
"%{clang-tidy}",
"%{test-tools}/clang_tidy_checks/libcxx-tidy.plugin",
"%{cxx}",
- "%{flags} %{compile_flags}",
+ "%{flags} %{compile_flags} %{module_flags}",
+ "std",
)
diff --git a/libcxx/test/libcxx/module_std_compat.gen.py b/libcxx/test/libcxx/module_std_compat.gen.py
index c4792db3d71e62..63fdd8188937e1 100644
--- a/libcxx/test/libcxx/module_std_compat.gen.py
+++ b/libcxx/test/libcxx/module_std_compat.gen.py
@@ -29,7 +29,8 @@
"%{clang-tidy}",
"%{test-tools}/clang_tidy_checks/libcxx-tidy.plugin",
"%{cxx}",
- "%{flags} %{compile_flags}",
+ "%{flags} %{compile_flags} %{module_flags}",
+ "std.compat",
)
diff --git a/libcxx/test/libcxx/selftest/modules/no-modules.sh.cpp b/libcxx/test/libcxx/selftest/modules/no-modules.sh.cpp
new file mode 100644
index 00000000000000..86d0afc13e3c40
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/modules/no-modules.sh.cpp
@@ -0,0 +1,12 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// Make sure that the module flags are empty when no module is supplied.
+
+// MODULES:
+// RUN: echo "%{module_flags}" | grep "^$"
diff --git a/libcxx/test/libcxx/selftest/modules/std-and-std.compat-module.sh.cpp b/libcxx/test/libcxx/selftest/modules/std-and-std.compat-module.sh.cpp
new file mode 100644
index 00000000000000..b9be1d40614aa0
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/modules/std-and-std.compat-module.sh.cpp
@@ -0,0 +1,17 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// Make sure that the module flags contain the expected elements.
+// The tests only look for the expected components and not the exact flags.
+// Otherwise changing the location of the module breaks this test.
+
+// MODULES: std std.compat
+//
+// RUN: echo "%{module_flags}" | grep -- "-fprebuilt-module-path="
+// RUN: echo "%{module_flags}" | grep "std.pcm"
+// RUN: echo "%{module_flags}" | grep "std.compat.pcm"
diff --git a/libcxx/test/libcxx/selftest/modules/std-module.sh.cpp b/libcxx/test/libcxx/selftest/modules/std-module.sh.cpp
new file mode 100644
index 00000000000000..6007511bd854a3
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/modules/std-module.sh.cpp
@@ -0,0 +1,19 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//=...
[truncated]
|
mordante
changed the title
|
✅ With the latest revision this PR passed the Python code formatter. |
mordante
commented
Dec 22, 2023
mordante
force-pushed
the
users/mordante/adds_module_testing
branch
from
0396727 to
10c2d9a
Compare
mordante
changed the base branch from
main
to
users/mordante/removes_module_testing
hawkinsw
reviewed
Dec 23, 2023
mordante
force-pushed
the
users/mordante/adds_module_testing
branch
from
10c2d9a to
fe2406d
Compare
Thanks for the suggestions! Please do no incorporate these in your PR. Other reviewers may have other suggestions which you then need to incorporate too. It's a lot easier, for both of us, when you finish the documentation PR and after you merged your branch I move the comments of this patch to the new location. I left this comment for other reviewers so they don't need to comment on it. This is how we typically resolve conflicts between patches.
Thanks! |
mordante
added a commit
that referenced
this pull request
Dec 23, 2023
Recent CI changes have disabled testing modules in different configurations. This broke building the std and std.compat module in C++20. This was found by the CI in #76246.
That makes total sense. I had already added the following to my PR (in case you are interested in incorporating here): but I will gladly revert that if you think it's a good idea. Sorry for the confusion -- just trying to be helpful!
|
Yes otherwise we need to block your patch on this patch.
No problem, I appreciate your help! |
FYI: I have reverted the patch! |
ldionne
requested changes
Jan 9, 2024
libcxx/utils/libcxx/test/format.py
Outdated
Comment on lines
174
to
176
| # It is not known whether the compiler expects the modules in the order | ||
| # of their dependencies. However it's trivial to provide them in that | ||
| # order. |
There was a problem hiding this comment.
@ChuanqiXu9 Is this required? We should know for sure here.
mordante
force-pushed
the
users/mordante/removes_module_testing
branch
from
5c5c968 to
9ff0a5f
Compare
mordante
force-pushed
the
users/mordante/adds_module_testing
branch
from
35a9aa3 to
a95db48
Compare
mordante
force-pushed
the
users/mordante/removes_module_testing
branch
from
9ff0a5f to
13d7990
Compare
mordante
force-pushed
the
users/mordante/adds_module_testing
branch
from
f8bf61f to
eebe9b2
Compare
mordante
commented
Jan 12, 2024
ldionne
approved these changes
Jan 16, 2024
libcxx/test/libcxx/selftest/modules/std-and-std.compat-module.sh.cpp
Outdated
Show resolved
Hide resolved
libcxx/utils/libcxx/test/format.py
Outdated
| ) | ||
|
|
||
|
|
||
| def _getSubstitution(substitution, config): |
There was a problem hiding this comment.
Suggestion: Use dsl._getSubstitution instead. It's kind of an implementation detail of that other file, but whatever.
There was a problem hiding this comment.
This doesn't work, dsl depends on format. What would work is move this to config. I've copied the _appendToSubstitution and will clean up in a follow-up commit.
Base automatically changed from
users/mordante/removes_module_testing
to
main
January 17, 2024 07:11
This adds a new module test infrastructure. This requires tagging tests using modules. The test runner uses this information to determine the compiler flags needed to build and use the module. Currently modules are build per test, which allows testing them for tests with ADDITIONAL_COMPILE_FLAGS. At the moment only 4 tests use modules. Therefore the performance penalty is not measurable. If in the future more tests use modules it would be good to measure the overhead and determine whether it's acceptable.
mordante
force-pushed
the
users/mordante/adds_module_testing
branch
from
dd2d7ce to
a6f3772
Compare
|
Hey, it looks like we're seeing test failures on the Fuchsia Windows x64 clang builder due to this: E.g. for |
|
Interesting the first two errors are due to headers being available that are not available in libc++. Are the headers The Unfortunately I don't have access to Windows so I can't reproduce this issue. I think the following change would solve the issue https://github.com/llvm/llvm-project/blob/main/libcxx/utils/libcxx/test/features.py#L326 Can you test this fix? |
|
I did a build with this patched in, and it doesn't resolve the issue: Accordingly, I've issued a revert until this can be fixed: 5c150e7 If you can't get access to a Windows machine, I can run a reland of this through our Windows CI and make sure that it works. |
mysterymath
added a commit
that referenced
this pull request
Jan 18, 2024
|
Can you test with Is still like to know who provides the In the supplied logs I don't see the configuration parameters used to configure libc++, can you provide them? |
mysterymath
added a commit
to mysterymath/llvm-project
that referenced
this pull request
Jan 19, 2024
This reverts commit 5c150e7.
|
The clang cmake params are here: It makes use of the Fuchsia.cmake present in the LLVM; this configures the build for both Windows host and Fuchsia target. It's the host part that's breaking. I've verified that both
Giving this a try; I'll let you know how it turns out. |
|
Thanks for reapplying the patch. |
|
I see the patch is only reverted in your local branch. Can you give an ETA when the testing it is done? This revert blocks work on modules which I really would like to land in LLVM 18. It also blocks other people from testing their fixes for modules. |
|
Unfortunately I still haven't heard back. I'm reapplying the patch with the Windows fix, which I'm quite sure fixes the issue. Since your bot is not in LLVM buildbot network I can't monitor it. If it breaks please don't revert again, please help me to find the proper lit flags needed to disable the tests on Windows. (AFAIK modules don't work with clang-cl yet.) At the moment it blocks testing and improving modules on platforms that have support. I still want to look afterwards how we can narrow this issue down further. |
|
btw it would be great if this configuration could be tested in the libc++ pre-commit CI. That would help us to prevent breaking your setup. Please reach out to libcxx on Discord if you want to add a buildbot. |
mordante
added a commit
that referenced
this pull request
Jan 21, 2024
Revert "Revert #76246 and #76083" This reverts commit 5c150e7. Adds a small fix that should properly disable the tests on Windows. Unfortunately the original poster has not provided feedback and the original patch did not fail in the LLVM CI infrastructure. Modules are known to fail on Windows due to non compliance of the C library. Currently not having this patch prevents testing on other platforms.
No, I don't think that's a configuration we intend to support. I'm surprised that this is how we run the tests on Windows. @mstorsjo would have more context though, this might be necessary for us to find some of the Windows SDK headers. |
Actually, this is exactly how we already run our tests right now. In MSVC-like configurations (contrary to mingw), the MSVC STL headers are available directly in one of the main include directories that we strictly need to have available. In Unix, it's the same as if The main difference, I believe, is that our CI runners have a slightly older install of MSVC, that lacks From the point of view of a libc++ user on top of MSVC, that makes it quite problematic to use |
|
Thanks for the info. That indeed means we don't know which header might get picked up. For now I disabled module testing for Windows. I can fix the |
|
It looks like the version of the disable with |
justinfargnoli
pushed a commit
to justinfargnoli/llvm-project
that referenced
this pull request
Jan 28, 2024
This was reviewed in llvm#76246
blueboxd
pushed a commit
to blueboxd/libcxx
that referenced
this pull request
Feb 1, 2024
This was reviewed in llvm/llvm-project#76246 NOKEYCHECK=True GitOrigin-RevId: ab398416a7932d9b85e216353b7c847189c1a597
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a new module test infrastructure. This requires tagging tests
using modules. The test runner uses this information to determine the
compiler flags needed to build and use the module.
Currently modules are build per test, which allows testing them for
tests with ADDITIONAL_COMPILE_FLAGS. At the moment only 4 tests use
modules. Therefore the performance penalty is not measurable. If in the
future more tests use modules it would be good to measure the overhead
and determine whether it's acceptable.