[libcxx-commits] [libcxx] d9e3c85 - [libc++][modules] Simplifies C++20 module testing.

Mark de Wever via libcxx-commits libcxx-commits at lists.llvm.org
Tue Aug 22 11:06:37 PDT 2023


Author: Mark de Wever
Date: 2023-08-22T20:06:28+02:00
New Revision: d9e3c85f5703f42b094e8f91e60e2a1af4fac531

URL: https://github.com/llvm/llvm-project/commit/d9e3c85f5703f42b094e8f91e60e2a1af4fac531
DIFF: https://github.com/llvm/llvm-project/commit/d9e3c85f5703f42b094e8f91e60e2a1af4fac531.diff

LOG: [libc++][modules] Simplifies C++20 module testing.

The building of the std module has been moved from `params.py` and
`dsl.py` to a `lit.local.cfg` for the entire test suite. In theory this
change allows testing modules in most configurations, except:
- combined with clang modules
- C++ versions that don't support the std module

Currently only C++23 with all parts enabled works.
C++26 is expected to work properly with CMake 3.27. That versions of CMake
knows how to invoke clang using C++26.
The parts disabled modi of libc++ have not been modularized yet.

It still is the goal that in the future CMake will be able to do the work
done in `lit.local.cfg`. Doing this in CMake would require a more mature
libc++ implementation.

Thanks a lot to @ldionne for giving hints how to enable modules in a
`lit.local.cfg`.

Reviewed By: #libc, ldionne

Differential Revision: https://reviews.llvm.org/D157625

Added: 
    libcxx/test/lit.local.cfg

Modified: 
    libcxx/CMakeLists.txt
    libcxx/cmake/caches/Generic-module-std-cxx23.cmake
    libcxx/include/__config_site.in
    libcxx/test/libcxx/module_std.gen.py
    libcxx/test/libcxx/modules_include.gen.py
    libcxx/test/std/modules/std.pass.cpp
    libcxx/utils/libcxx/test/dsl.py
    libcxx/utils/libcxx/test/features.py
    libcxx/utils/libcxx/test/params.py

Removed: 
    


################################################################################
diff  --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 6333a15d6ecaed..b9d0ed51be2603 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -755,6 +755,7 @@ 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_VENDOR_AVAILABILITY_ANNOTATIONS _LIBCPP_HAS_NO_VENDOR_AVAILABILITY_ANNOTATIONS)
 if (LIBCXX_HARDENING_MODE STREQUAL "hardened")
   config_define(1 _LIBCPP_ENABLE_HARDENED_MODE_DEFAULT)

diff  --git a/libcxx/cmake/caches/Generic-module-std-cxx23.cmake b/libcxx/cmake/caches/Generic-module-std-cxx23.cmake
index 6475bd79329632..ffff55381e955b 100644
--- a/libcxx/cmake/caches/Generic-module-std-cxx23.cmake
+++ b/libcxx/cmake/caches/Generic-module-std-cxx23.cmake
@@ -1,4 +1,4 @@
 set(LIBCXX_ENABLE_STD_MODULES ON CACHE BOOL "")
-set(LIBCXX_TEST_PARAMS "enable_modules=std;std=c++23" CACHE STRING "")
+set(LIBCXX_TEST_PARAMS "std=c++23" CACHE STRING "")
 set(LIBCXXABI_TEST_PARAMS "std=c++23" CACHE STRING "")
 

diff  --git a/libcxx/include/__config_site.in b/libcxx/include/__config_site.in
index 8ec3722eafc516..40694314d2e9d3 100644
--- a/libcxx/include/__config_site.in
+++ b/libcxx/include/__config_site.in
@@ -27,6 +27,7 @@
 #cmakedefine _LIBCPP_HAS_NO_RANDOM_DEVICE
 #cmakedefine _LIBCPP_HAS_NO_LOCALIZATION
 #cmakedefine _LIBCPP_HAS_NO_WIDE_CHARACTERS
+#cmakedefine _LIBCPP_HAS_NO_STD_MODULES
 
 // PSTL backends
 #cmakedefine _LIBCPP_PSTL_CPU_BACKEND_SERIAL

diff  --git a/libcxx/test/libcxx/module_std.gen.py b/libcxx/test/libcxx/module_std.gen.py
index 3344e9f787f41d..e917e791cfc50a 100644
--- a/libcxx/test/libcxx/module_std.gen.py
+++ b/libcxx/test/libcxx/module_std.gen.py
@@ -127,9 +127,11 @@
     f"""\
 //--- module_std.sh.cpp
 // UNSUPPORTED{BLOCKLIT}: c++03, c++11, c++14, c++17, c++20
+// UNSUPPORTED{BLOCKLIT}: c++26
+// UNSUPPORTED{BLOCKLIT}: libcpp-has-no-std-modules
+// UNSUPPORTED{BLOCKLIT}: modules-build
 
 // REQUIRES{BLOCKLIT}: has-clang-tidy
-// REQUIRES{BLOCKLIT}: use_module_std
 
 // The GCC compiler flags are not always compatible with clang-tidy.
 // UNSUPPORTED{BLOCKLIT}: gcc

diff  --git a/libcxx/test/libcxx/modules_include.gen.py b/libcxx/test/libcxx/modules_include.gen.py
index 2e9fd73421ed20..1fd1e6ae9160e7 100644
--- a/libcxx/test/libcxx/modules_include.gen.py
+++ b/libcxx/test/libcxx/modules_include.gen.py
@@ -23,8 +23,6 @@
 //--- {header}.compile.pass.cpp
 // RUN{BLOCKLIT}: %{{cxx}} %s %{{flags}} %{{compile_flags}} -fmodules -fcxx-modules -fmodules-cache-path=%t -fsyntax-only
 
-// UNSUPPORTED{BLOCKLIT}: use_module_std
-
 // GCC doesn't support -fcxx-modules
 // UNSUPPORTED{BLOCKLIT}: gcc
 
@@ -51,7 +49,6 @@
 // RUN{BLOCKLIT}: %{{cxx}} %s %{{flags}} %{{compile_flags}} -fmodules -fcxx-modules -fmodules-cache-path=%t -fsyntax-only
 
 // REQUIRES{BLOCKLIT}: modules-build
-// UNSUPPORTED{BLOCKLIT}: use_module_std
 
 // GCC doesn't support -fcxx-modules
 // UNSUPPORTED{BLOCKLIT}: gcc

diff  --git a/libcxx/test/lit.local.cfg b/libcxx/test/lit.local.cfg
new file mode 100644
index 00000000000000..fe530409c399f2
--- /dev/null
+++ b/libcxx/test/lit.local.cfg
@@ -0,0 +1,69 @@
+# ===----------------------------------------------------------------------===##
+#
+# 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
+#
+# ===----------------------------------------------------------------------===##
+
+# This configuration builds the C++23 std module.
+# It is build when the current lit configuration supports modules.
+#
+# TODO MODULES Evaluate whether this file can be removed when CMake supports
+# modules in libc++.
+
+import os
+import site
+import subprocess
+import libcxx.test.params, libcxx.test.config, libcxx.test.dsl
+
+
+def getSubstitution(substitution, config):
+    for orig, replacement in config.substitutions:
+        if orig == substitution:
+            return replacement
+    raise ValueError("Substitution {} is not in the config.".format(substitution))
+
+
+def appendToSubstitution(substitutions, key, value):
+    return [(k, v + " " + value) if k == key else (k, v) for (k, v) in substitutions]
+
+
+std = getSubstitution("%{cxx_std}", config)
+if std == "cxx26":
+    # This fails to work properly. It might be due to
+    #   CMAKE_CXX_STANDARD 26
+    # does not work in CMake 3.26, it requires the upcomming CMake 3.27.
+    # TODO MODULES test whether this is fixed with CMake 3.27.
+    std = ""
+elif std == "cxx23":
+    std = "23"
+else:
+    std = ""
+
+if (
+    std
+    and not "libcpp-has-no-std-modules" in config.available_features
+    and not "modules-build" in config.available_features
+):
+    build = os.path.join(config.test_exec_root, "__config_module__")
+    config.substitutions = appendToSubstitution(
+        config.substitutions,
+        "%{compile_flags}",
+        "-fprebuilt-module-path="
+        + os.path.join(config.test_exec_root, "__config_module__/CMakeFiles/std.dir"),
+    )
+
+    flags = getSubstitution("%{flags}", config)
+    cmake = getSubstitution("%{cmake}", config)
+
+    subprocess.check_call(
+        [cmake, f"-DCMAKE_CXX_STANDARD={std}", f"-DCMAKE_CXX_FLAGS={flags}", build],
+        env={},
+    )
+    subprocess.check_call([cmake, "--build", build, "--", "-v"], env={})
+    config.substitutions = appendToSubstitution(
+        config.substitutions,
+        "%{link_flags}",
+        os.path.join(build, "libc++std.a"),
+    )

diff  --git a/libcxx/test/std/modules/std.pass.cpp b/libcxx/test/std/modules/std.pass.cpp
index ec50fdfd3bd3b2..b0b6ebe0255a0c 100644
--- a/libcxx/test/std/modules/std.pass.cpp
+++ b/libcxx/test/std/modules/std.pass.cpp
@@ -10,7 +10,8 @@
 // TODO MODULES fix c++26
 // XFAIL: c++26
 
-// REQUIRES: use_module_std
+// UNSUPPORTED: libcpp-has-no-std-modules
+// UNSUPPORTED: modules-build
 
 // A minimal test to validate import works.
 

diff  --git a/libcxx/utils/libcxx/test/dsl.py b/libcxx/utils/libcxx/test/dsl.py
index 29ec53a3305ead..847cebf5962f6a 100644
--- a/libcxx/utils/libcxx/test/dsl.py
+++ b/libcxx/utils/libcxx/test/dsl.py
@@ -437,35 +437,6 @@ def applyTo(self, config):
     def pretty(self, config, litParams):
         return "add {} to %{{flags}}".format(self._getFlag(config))
 
-class BuildStdModule(ConfigAction):
-  def applyTo(self, config):
-    build = os.path.join(config.test_exec_root, '__config_module__')
-
-    std = _getSubstitution('%{cxx_std}', config)
-    if std == 'cxx26':
-        # This fails to work properly. It might be due to
-        #   CMAKE_CXX_STANDARD 26
-        # does not work in CMake 3.26, it requires the upcomming CMake 3.27.
-        # TODO MODULES test whether this is fixed with CMake 3.27.
-        std = '17'
-    elif std == 'cxx23':
-        std = '23'
-    else:
-        std = '17' # Not allowed for modules
-
-    flags = _getSubstitution('%{flags}', config)
-    cmake = _getSubstitution('%{cmake}', config)
-
-    subprocess.check_call([cmake, f"-DCMAKE_CXX_STANDARD={std}", f"-DCMAKE_CXX_FLAGS={flags}", build], env={})
-    subprocess.check_call([cmake, "--build", build, "--", "-v"], env={})
-    config.substitutions = _appendToSubstitution(
-       # TODO MODULES Avoid manually modifying link_flags.
-       config.substitutions, "%{link_flags}",  os.path.join(build, "libc++std.a")
-    )
-
-  def pretty(self, config, litParams):
-    return "building std module with flags {}".format(_getSubstitution('%{flags}', config))
-
 class AddFlagIfSupported(ConfigAction):
     """
     This action adds the given flag to the %{flags} substitution, only if

diff  --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 61d8b81ac6e1d4..692f461e31f2f6 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -303,6 +303,7 @@ def _getSuitableClangTidy(cfg):
     "_LIBCPP_HAS_NO_LOCALIZATION": "no-localization",
     "_LIBCPP_HAS_NO_WIDE_CHARACTERS": "no-wide-characters",
     "_LIBCPP_HAS_NO_UNICODE": "libcpp-has-no-unicode",
+    "_LIBCPP_HAS_NO_STD_MODULES":  "libcpp-has-no-std-modules",
     "_LIBCPP_PSTL_CPU_BACKEND_LIBDISPATCH": "libcpp-pstl-cpu-backend-libdispatch",
 }
 for macro, feature in macros.items():

diff  --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index b957f92f57c59f..d91062139fe265 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -85,7 +85,7 @@ def getStdFlag(cfg, std):
     return None
 
 
-_allModules = ["none", "clang", "std"]
+_allModules = ["none", "clang"]
 
 
 def getModuleFlag(cfg, enable_modules):
@@ -129,7 +129,7 @@ def getModuleFlag(cfg, enable_modules):
         choices=_allModules,
         type=str,
         help="Whether to build the test suite with modules enabled. Select "
-        "`clang` for Clang modules and `std` for C++23 std module",
+        "`clang` for Clang modules",
         default=lambda cfg: next(s for s in _allModules if getModuleFlag(cfg, s)),
         actions=lambda enable_modules: [
             AddFeature("modules-build"),
@@ -141,19 +141,6 @@ def getModuleFlag(cfg, enable_modules):
             AddCompileFlag(lambda cfg: f"-fmodules-cache-path={cfg.test_exec_root}/ModuleCache"),
         ]
         if enable_modules == "clang"
-        else [
-            AddFeature("use_module_std"),
-            AddCompileFlag("-DTEST_USE_MODULE"),
-            AddCompileFlag("-DTEST_USE_MODULE_STD"),
-            AddCompileFlag(
-                lambda cfg: "-fprebuilt-module-path="
-                + os.path.join(
-                    cfg.test_exec_root, "__config_module__/CMakeFiles/std.dir"
-                )
-            ),
-            BuildStdModule(),
-        ]
-        if enable_modules == "std"
         else [],
     ),
     Parameter(


        


More information about the libcxx-commits mailing list