[llvm-branch-commits] [libcxx] [libc++][modules] Adds module testing. (PR #76246)

Mark de Wever via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue Jan 9 12:05:30 PST 2024


https://github.com/mordante updated https://github.com/llvm/llvm-project/pull/76246

>From 196cedd36534b02a7c55cf4a1746b34f87ead467 Mon Sep 17 00:00:00 2001
From: Mark de Wever <koraq at xs4all.nl>
Date: Wed, 20 Dec 2023 20:43:38 +0100
Subject: [PATCH 1/2] [libc++][modules] Adds module testing.

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.
---
 libcxx/test/libcxx/module_std.gen.py          |  3 +-
 libcxx/test/libcxx/module_std_compat.gen.py   |  3 +-
 .../libcxx/selftest/modules/no-modules.sh.cpp | 12 +++
 .../modules/std-and-std.compat-module.sh.cpp  | 23 ++++++
 .../libcxx/selftest/modules/std-module.sh.cpp | 25 ++++++
 .../selftest/modules/std.compat-module.sh.cpp | 25 ++++++
 .../modules/unknown-module.compile.pass.cpp   | 13 ++++
 libcxx/test/std/modules/std.compat.pass.cpp   |  5 +-
 libcxx/test/std/modules/std.pass.cpp          |  5 +-
 libcxx/utils/libcxx/test/features.py          | 12 +++
 libcxx/utils/libcxx/test/format.py            | 76 +++++++++++++++++--
 libcxx/utils/libcxx/test/modules.py           |  5 +-
 12 files changed, 196 insertions(+), 11 deletions(-)
 create mode 100644 libcxx/test/libcxx/selftest/modules/no-modules.sh.cpp
 create mode 100644 libcxx/test/libcxx/selftest/modules/std-and-std.compat-module.sh.cpp
 create mode 100644 libcxx/test/libcxx/selftest/modules/std-module.sh.cpp
 create mode 100644 libcxx/test/libcxx/selftest/modules/std.compat-module.sh.cpp
 create mode 100644 libcxx/test/libcxx/selftest/modules/unknown-module.compile.pass.cpp

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..da9497f2dbb768
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/modules/std-and-std.compat-module.sh.cpp
@@ -0,0 +1,23 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// UNSUPPORTED: clang-modules-build
+// UNSUPPORTED: gcc
+
+// XFAIL: has-no-module-support
+
+// 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..ed6fa96fe8252c
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/modules/std-module.sh.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// UNSUPPORTED: clang-modules-build
+// UNSUPPORTED: gcc
+
+// XFAIL: has-no-module-support
+
+// 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
+//
+// RUN: echo "%{module_flags}" | grep -- "-fprebuilt-module-path="
+// RUN: echo "%{module_flags}" | grep "std.pcm"
+
+// The std module should not provide the std.compat module
+// RUN: echo "%{module_flags}" | grep -v "std.compat.pcm"
diff --git a/libcxx/test/libcxx/selftest/modules/std.compat-module.sh.cpp b/libcxx/test/libcxx/selftest/modules/std.compat-module.sh.cpp
new file mode 100644
index 00000000000000..5e2fefe183ad40
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/modules/std.compat-module.sh.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// UNSUPPORTED: clang-modules-build
+// UNSUPPORTED: gcc
+
+// XFAIL: has-no-module-support
+
+// 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.compat
+//
+// RUN: echo "%{module_flags}" | grep -- "-fprebuilt-module-path="
+// RUN: echo "%{module_flags}" | grep "std.compat.pcm"
+
+// It's unspecified whether std.compat is built on the std module.
+// Therefore don't test its presence
diff --git a/libcxx/test/libcxx/selftest/modules/unknown-module.compile.pass.cpp b/libcxx/test/libcxx/selftest/modules/unknown-module.compile.pass.cpp
new file mode 100644
index 00000000000000..7c10f0db1e3401
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/modules/unknown-module.compile.pass.cpp
@@ -0,0 +1,13 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 modules that are unknown fail.
+
+// MODULES: this_module_is_not_a_standard_library_module
+
+// XFAIL: *
diff --git a/libcxx/test/std/modules/std.compat.pass.cpp b/libcxx/test/std/modules/std.compat.pass.cpp
index ba75f8e4010027..937e090757bf75 100644
--- a/libcxx/test/std/modules/std.compat.pass.cpp
+++ b/libcxx/test/std/modules/std.compat.pass.cpp
@@ -8,11 +8,14 @@
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 // UNSUPPORTED: clang-modules-build
+// UNSUPPORTED: gcc
 
-// XFAIL: *
+// XFAIL: has-no-module-support
 
 // A minimal test to validate import works.
 
+// MODULES: std.compat
+
 import std.compat;
 
 int main(int, char**) { return !(::strlen("Hello modular world") == 19); }
diff --git a/libcxx/test/std/modules/std.pass.cpp b/libcxx/test/std/modules/std.pass.cpp
index a018e42a265891..1c0e680c6d730a 100644
--- a/libcxx/test/std/modules/std.pass.cpp
+++ b/libcxx/test/std/modules/std.pass.cpp
@@ -8,11 +8,14 @@
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
 // UNSUPPORTED: clang-modules-build
+// UNSUPPORTED: gcc
 
-// XFAIL: *
+// XFAIL: has-no-module-support
 
 // A minimal test to validate import works.
 
+// MODULES: std
+
 import std;
 
 int main(int, char**) {
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 332def02980ca6..1939d553d72dc1 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -317,6 +317,18 @@ def _getAndroidDeviceApi(cfg):
             AddSubstitution("%{clang-tidy}", lambda cfg: _getSuitableClangTidy(cfg))
         ],
     ),
+    # Whether module support for the platform is available.
+    Feature(
+        name="has-no-module-support",
+        # The libc of these platforms have functions with internal linkages.
+        # This is not allowed per C11 7.1.2 Standard headers/6
+        #  Any declaration of a library function shall have external linkage.
+        when=lambda cfg: "__ANDROID__" in compilerMacros(cfg)
+        or "__PICOLIBC__" in compilerMacros(cfg)
+        or platform.system().lower().startswith("aix")
+        # Avoid building on platforms that don't support modules properly.
+        or not hasCompileFlag(cfg, "-Wno-reserved-module-identifier"),
+    ),
 ]
 
 # Deduce and add the test features that that are implied by the #defines in
diff --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py
index e58e404bfcd2a5..2840f7c29a3b10 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -91,6 +91,8 @@ def parseScript(test, preamble):
     # Parse the test file, including custom directives
     additionalCompileFlags = []
     fileDependencies = []
+    modules = []  # The enabled modules
+    moduleCompileFlags = []  # The compilation flags to use modules
     parsers = [
         lit.TestRunner.IntegratedTestKeywordParser(
             "FILE_DEPENDENCIES:",
@@ -102,6 +104,11 @@ def parseScript(test, preamble):
             lit.TestRunner.ParserKind.SPACE_LIST,
             initial_value=additionalCompileFlags,
         ),
+        lit.TestRunner.IntegratedTestKeywordParser(
+            "MODULES:",
+            lit.TestRunner.ParserKind.SPACE_LIST,
+            initial_value=modules,
+        ),
     ]
 
     # Add conditional parsers for ADDITIONAL_COMPILE_FLAGS. This should be replaced by first
@@ -131,6 +138,51 @@ def parseScript(test, preamble):
     script += preamble
     script += scriptInTest
 
+    has_std_module = False
+    has_std_compat_module = False
+    for module in modules:
+        if module == "std":
+            has_std_module = True
+        elif module == "std.compat":
+            has_std_compat_module = True
+        else:
+            script.insert(
+                0,
+                f"echo \"The module '{module}' is not valid, use 'std' or 'std.compat'\"",
+            )
+            script.insert(1, "false")
+            return script
+
+    if modules:
+        # This flag is needed for both modules.
+        moduleCompileFlags.append("-fprebuilt-module-path=%T")
+
+        # Building the modules needs to happen before the other script commands
+        # are executed. Therefore the commands are added to the front of the
+        # list.
+        if has_std_compat_module:
+            script.insert(
+                0,
+                "%dbg(MODULE std.compat) %{cxx} %{flags} %{compile_flags} "
+                "-Wno-reserved-module-identifier -Wno-reserved-user-defined-literal "
+                "--precompile -o %T/std.compat.pcm -c %{module}/std.compat.cppm",
+            )
+            moduleCompileFlags.append("%T/std.compat.pcm")
+
+        # Make sure the std module is added before std.compat.
+        # Libc++'s std.compat module will depend on its std module.
+        # 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.
+        if has_std_module:
+            script.insert(
+                0,
+                "%dbg(MODULE std) %{cxx} %{flags} %{compile_flags} "
+                "-Wno-reserved-module-identifier -Wno-reserved-user-defined-literal "
+                "--precompile -o %T/std.pcm -c %{module}/std.cppm",
+            )
+            moduleCompileFlags.append("%T/std.pcm")
+
     # Add compile flags specified with ADDITIONAL_COMPILE_FLAGS.
     substitutions = [
         (s, x + " " + " ".join(additionalCompileFlags))
@@ -138,6 +190,13 @@ def parseScript(test, preamble):
         else (s, x)
         for (s, x) in substitutions
     ]
+    # In order to use modules, additional compilation flags are required.
+    # Adding these to the %{compile_flags} gives a chicken and egg issue:
+    # - the modules need to be built with the same compilation flags as the
+    #   tests,
+    # - except for the module dependency, which does not exist.
+    # The issue is resolved by adding a private substitution.
+    substitutions.append(("%{module_flags}", " ".join(moduleCompileFlags)))
 
     # Perform substitutions in the script itself.
     script = lit.TestRunner.applySubstitutions(
@@ -191,6 +250,7 @@ class CxxStandardLibraryTest(lit.formats.FileBasedTest):
     constructs:
         %{cxx}           - A command that can be used to invoke the compiler
         %{compile_flags} - Flags to use when compiling a test case
+        %{module_flags}  - Flags to use when compiling a test case that imports modules
         %{link_flags}    - Flags to use when linking a test case
         %{flags}         - Flags to use either when compiling or linking a test case
         %{exec}          - A command to prefix the execution of executables
@@ -223,6 +283,12 @@ class CxxStandardLibraryTest(lit.formats.FileBasedTest):
             allows adding special compilation flags without having to use a
             .sh.cpp test, which would be more powerful but perhaps overkill.
 
+        // MODULE: std std.compat
+
+           This directive will build the required C++23 standard library
+           modules and add the additional compiler flags in
+           %{module_flags}. (Libc++ offers these modules in C++20 as an
+           extension.)
 
     Additional provided substitutions and features
     ==============================================
@@ -288,22 +354,22 @@ def execute(self, test, litConfig):
             ".compile.pass.mm"
         ):
             steps = [
-                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} -fsyntax-only"
+                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{module_flags} -fsyntax-only"
             ]
             return self._executeShTest(test, litConfig, steps)
         elif filename.endswith(".compile.fail.cpp"):
             steps = [
-                "%dbg(COMPILED WITH) ! %{cxx} %s %{flags} %{compile_flags} -fsyntax-only"
+                "%dbg(COMPILED WITH) ! %{cxx} %s %{flags} %{compile_flags} %{module_flags} -fsyntax-only"
             ]
             return self._executeShTest(test, litConfig, steps)
         elif filename.endswith(".link.pass.cpp") or filename.endswith(".link.pass.mm"):
             steps = [
-                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{link_flags} -o %t.exe"
+                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{module_flags} %{link_flags} -o %t.exe"
             ]
             return self._executeShTest(test, litConfig, steps)
         elif filename.endswith(".link.fail.cpp"):
             steps = [
-                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} -c -o %t.o",
+                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{module_flags} -c -o %t.o",
                 "%dbg(LINKED WITH) ! %{cxx} %t.o %{flags} %{link_flags} -o %t.exe",
             ]
             return self._executeShTest(test, litConfig, steps)
@@ -321,7 +387,7 @@ def execute(self, test, litConfig):
         # suffixes above too.
         elif filename.endswith(".pass.cpp") or filename.endswith(".pass.mm"):
             steps = [
-                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{link_flags} -o %t.exe",
+                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{module_flags} %{link_flags} -o %t.exe",
                 "%dbg(EXECUTED AS) %{exec} %t.exe",
             ]
             return self._executeShTest(test, litConfig, steps)
diff --git a/libcxx/utils/libcxx/test/modules.py b/libcxx/utils/libcxx/test/modules.py
index 9362d52cb72d2c..e2e94c239f21c7 100644
--- a/libcxx/utils/libcxx/test/modules.py
+++ b/libcxx/utils/libcxx/test/modules.py
@@ -113,6 +113,7 @@ class module_test_generator:
     clang_tidy_plugin: str
     compiler: str
     compiler_flags: str
+    module: str
 
     def write_lit_configuration(self):
         print(
@@ -120,13 +121,13 @@ def write_lit_configuration(self):
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 // UNSUPPORTED: clang-modules-build
 
-// XFAIL: *
-
 // REQUIRES: has-clang-tidy
 
 // The GCC compiler flags are not always compatible with clang-tidy.
 // UNSUPPORTED: gcc
 
+// MODULES: {self.module}
+
 // RUN: echo -n > {self.tmp_prefix}.all_partitions
 """
         )

>From a95db48a745c0b4bf9b764ff16290a23ecad240f Mon Sep 17 00:00:00 2001
From: Mark de Wever <koraq at xs4all.nl>
Date: Tue, 9 Jan 2024 20:05:47 +0100
Subject: [PATCH 2/2] Address review comments

---
 libcxx/test/libcxx/module_std.gen.py          |   2 +-
 libcxx/test/libcxx/module_std_compat.gen.py   |   2 +-
 .../libcxx/selftest/modules/no-modules.sh.cpp |   9 +-
 .../modules/std-and-std.compat-module.sh.cpp  |  14 +-
 .../libcxx/selftest/modules/std-module.sh.cpp |  14 +-
 .../selftest/modules/std.compat-module.sh.cpp |  12 +-
 .../modules/unknown-module.compile.pass.cpp   |  13 --
 libcxx/test/std/modules/std.compat.pass.cpp   |   4 +-
 libcxx/test/std/modules/std.pass.cpp          |   4 +-
 libcxx/utils/libcxx/test/features.py          |   4 +-
 libcxx/utils/libcxx/test/format.py            | 121 ++++++++++--------
 libcxx/utils/libcxx/test/modules.py           |   2 +-
 12 files changed, 101 insertions(+), 100 deletions(-)
 delete mode 100644 libcxx/test/libcxx/selftest/modules/unknown-module.compile.pass.cpp

diff --git a/libcxx/test/libcxx/module_std.gen.py b/libcxx/test/libcxx/module_std.gen.py
index 3ad2aff9d085f4..a9a05a0cd74e61 100644
--- a/libcxx/test/libcxx/module_std.gen.py
+++ b/libcxx/test/libcxx/module_std.gen.py
@@ -29,7 +29,7 @@
     "%{clang-tidy}",
     "%{test-tools}/clang_tidy_checks/libcxx-tidy.plugin",
     "%{cxx}",
-    "%{flags} %{compile_flags} %{module_flags}",
+    "%{flags} %{compile_flags}",
     "std",
 )
 
diff --git a/libcxx/test/libcxx/module_std_compat.gen.py b/libcxx/test/libcxx/module_std_compat.gen.py
index 63fdd8188937e1..2866066ccedc89 100644
--- a/libcxx/test/libcxx/module_std_compat.gen.py
+++ b/libcxx/test/libcxx/module_std_compat.gen.py
@@ -29,7 +29,7 @@
     "%{clang-tidy}",
     "%{test-tools}/clang_tidy_checks/libcxx-tidy.plugin",
     "%{cxx}",
-    "%{flags} %{compile_flags} %{module_flags}",
+    "%{flags} %{compile_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
index 86d0afc13e3c40..7d068c80180570 100644
--- a/libcxx/test/libcxx/selftest/modules/no-modules.sh.cpp
+++ b/libcxx/test/libcxx/selftest/modules/no-modules.sh.cpp
@@ -6,7 +6,10 @@
 //
 //===----------------------------------------------------------------------===//
 
-// Make sure that the module flags are empty when no module is supplied.
+// Make sure that the compile flags contain no module information.
 
-// MODULES:
-// RUN: echo "%{module_flags}"  | grep "^$"
+// MODULE_DEPENDENCIES:
+
+// RUN: echo "%{compile_flags}" | grep -v -- "-fprebuilt-module-path="
+// RUN: echo "%{compile_flags}" | grep -v "std.pcm"
+// RUN: echo "%{compile_flags}" | grep -v "std.compat.pcm"
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
index da9497f2dbb768..f78f3c910b3cd8 100644
--- 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
@@ -10,14 +10,14 @@
 // UNSUPPORTED: clang-modules-build
 // UNSUPPORTED: gcc
 
-// XFAIL: has-no-module-support
+// XFAIL: has-no-cxx-module-support
 
-// Make sure that the module flags contain the expected elements.
+// Make sure that the compile 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"
+// MODULE_DEPENDENCIES: std std.compat
+
+// RUN: echo "%{compile_flags}" | grep -- "-fprebuilt-module-path="
+// RUN: echo "%{compile_flags}" | grep "std.pcm"
+// RUN: echo "%{compile_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
index ed6fa96fe8252c..7d059009639091 100644
--- a/libcxx/test/libcxx/selftest/modules/std-module.sh.cpp
+++ b/libcxx/test/libcxx/selftest/modules/std-module.sh.cpp
@@ -10,16 +10,16 @@
 // UNSUPPORTED: clang-modules-build
 // UNSUPPORTED: gcc
 
-// XFAIL: has-no-module-support
+// XFAIL: has-no-cxx-module-support
 
-// Make sure that the module flags contain the expected elements.
+// Make sure that the compile 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
-//
-// RUN: echo "%{module_flags}" | grep -- "-fprebuilt-module-path="
-// RUN: echo "%{module_flags}" | grep "std.pcm"
+// MODULE_DEPENDENCIES: std
+
+// RUN: echo "%{compile_flags}" | grep -- "-fprebuilt-module-path="
+// RUN: echo "%{compile_flags}" | grep "std.pcm"
 
 // The std module should not provide the std.compat module
-// RUN: echo "%{module_flags}" | grep -v "std.compat.pcm"
+// RUN: echo "%{compile_flags}" | grep -v "std.compat.pcm"
diff --git a/libcxx/test/libcxx/selftest/modules/std.compat-module.sh.cpp b/libcxx/test/libcxx/selftest/modules/std.compat-module.sh.cpp
index 5e2fefe183ad40..14fa1489097ab0 100644
--- a/libcxx/test/libcxx/selftest/modules/std.compat-module.sh.cpp
+++ b/libcxx/test/libcxx/selftest/modules/std.compat-module.sh.cpp
@@ -10,16 +10,16 @@
 // UNSUPPORTED: clang-modules-build
 // UNSUPPORTED: gcc
 
-// XFAIL: has-no-module-support
+// XFAIL: has-no-cxx-module-support
 
-// Make sure that the module flags contain the expected elements.
+// Make sure that the compile 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.compat
-//
-// RUN: echo "%{module_flags}" | grep -- "-fprebuilt-module-path="
-// RUN: echo "%{module_flags}" | grep "std.compat.pcm"
+// MODULE_DEPENDENCIES: std.compat
+
+// RUN: echo "%{compile_flags}" | grep -- "-fprebuilt-module-path="
+// RUN: echo "%{compile_flags}" | grep "std.compat.pcm"
 
 // It's unspecified whether std.compat is built on the std module.
 // Therefore don't test its presence
diff --git a/libcxx/test/libcxx/selftest/modules/unknown-module.compile.pass.cpp b/libcxx/test/libcxx/selftest/modules/unknown-module.compile.pass.cpp
deleted file mode 100644
index 7c10f0db1e3401..00000000000000
--- a/libcxx/test/libcxx/selftest/modules/unknown-module.compile.pass.cpp
+++ /dev/null
@@ -1,13 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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 modules that are unknown fail.
-
-// MODULES: this_module_is_not_a_standard_library_module
-
-// XFAIL: *
diff --git a/libcxx/test/std/modules/std.compat.pass.cpp b/libcxx/test/std/modules/std.compat.pass.cpp
index 937e090757bf75..e840f3c6b629cf 100644
--- a/libcxx/test/std/modules/std.compat.pass.cpp
+++ b/libcxx/test/std/modules/std.compat.pass.cpp
@@ -10,11 +10,11 @@
 // UNSUPPORTED: clang-modules-build
 // UNSUPPORTED: gcc
 
-// XFAIL: has-no-module-support
+// XFAIL: has-no-cxx-module-support
 
 // A minimal test to validate import works.
 
-// MODULES: std.compat
+// MODULE_DEPENDENCIES: std.compat
 
 import std.compat;
 
diff --git a/libcxx/test/std/modules/std.pass.cpp b/libcxx/test/std/modules/std.pass.cpp
index 1c0e680c6d730a..ca05825b3a1863 100644
--- a/libcxx/test/std/modules/std.pass.cpp
+++ b/libcxx/test/std/modules/std.pass.cpp
@@ -10,11 +10,11 @@
 // UNSUPPORTED: clang-modules-build
 // UNSUPPORTED: gcc
 
-// XFAIL: has-no-module-support
+// XFAIL: has-no-cxx-module-support
 
 // A minimal test to validate import works.
 
-// MODULES: std
+// MODULE_DEPENDENCIES: std
 
 import std;
 
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 1939d553d72dc1..6387a5c0de007b 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -319,8 +319,8 @@ def _getAndroidDeviceApi(cfg):
     ),
     # Whether module support for the platform is available.
     Feature(
-        name="has-no-module-support",
-        # The libc of these platforms have functions with internal linkages.
+        name="has-no-cxx-module-support",
+        # The libc of these platforms have functions with internal linkage.
         # This is not allowed per C11 7.1.2 Standard headers/6
         #  Any declaration of a library function shall have external linkage.
         when=lambda cfg: "__ANDROID__" in compilerMacros(cfg)
diff --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py
index 2840f7c29a3b10..e7d09d9f13582f 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -52,6 +52,21 @@ def _executeScriptInternal(test, litConfig, commands):
     return (out, err, exitCode, timeoutInfo, parsedCommands)
 
 
+def _validateModuleDependencies(modules):
+    for m in modules:
+        if m not in ("std", "std.compat"):
+            raise RuntimeError(
+                f"Invalid module dependency '{m}', only 'std' and 'std.compat' are valid"
+            )
+
+
+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 parseScript(test, preamble):
     """
     Extract the script from a test, with substitutions applied.
@@ -105,7 +120,7 @@ def parseScript(test, preamble):
             initial_value=additionalCompileFlags,
         ),
         lit.TestRunner.IntegratedTestKeywordParser(
-            "MODULES:",
+            "MODULE_DEPENDENCIES:",
             lit.TestRunner.ParserKind.SPACE_LIST,
             initial_value=modules,
         ),
@@ -138,65 +153,62 @@ def parseScript(test, preamble):
     script += preamble
     script += scriptInTest
 
-    has_std_module = False
-    has_std_compat_module = False
-    for module in modules:
-        if module == "std":
-            has_std_module = True
-        elif module == "std.compat":
-            has_std_compat_module = True
-        else:
-            script.insert(
-                0,
-                f"echo \"The module '{module}' is not valid, use 'std' or 'std.compat'\"",
-            )
-            script.insert(1, "false")
-            return script
+    # Add compile flags specified with ADDITIONAL_COMPILE_FLAGS.
+    # Modules need to be build with the same compilation flags as the
+    # test. So add these flags before adding the modules.
+    substitutions = [
+        (s, x + " " + " ".join(additionalCompileFlags))
+        if s == "%{compile_flags}"
+        else (s, x)
+        for (s, x) in substitutions
+    ]
 
     if modules:
+        _validateModuleDependencies(modules)
+
         # This flag is needed for both modules.
         moduleCompileFlags.append("-fprebuilt-module-path=%T")
 
-        # Building the modules needs to happen before the other script commands
-        # are executed. Therefore the commands are added to the front of the
-        # list.
-        if has_std_compat_module:
+        # The moduleCompileFlags are added to the %{compile_flags}, but
+        # the modules need should be built without these flags. So
+        # expand the compile_flags and add the expanded value to the
+        # build script.
+        compileFlags = _getSubstitution("%{compile_flags}", test.config)
+
+        # Building the modules needs to happen before the other script
+        # commands are executed. Therefore the commands are added to the
+        # front of the list.
+        if "std.compat" in modules:
             script.insert(
                 0,
-                "%dbg(MODULE std.compat) %{cxx} %{flags} %{compile_flags} "
+                "%dbg(MODULE std.compat) %{cxx} %{flags} "
+                f"{compileFlags} "
                 "-Wno-reserved-module-identifier -Wno-reserved-user-defined-literal "
                 "--precompile -o %T/std.compat.pcm -c %{module}/std.compat.cppm",
             )
             moduleCompileFlags.append("%T/std.compat.pcm")
 
-        # Make sure the std module is added before std.compat.
-        # Libc++'s std.compat module will depend on its std module.
-        # 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.
-        if has_std_module:
-            script.insert(
-                0,
-                "%dbg(MODULE std) %{cxx} %{flags} %{compile_flags} "
-                "-Wno-reserved-module-identifier -Wno-reserved-user-defined-literal "
-                "--precompile -o %T/std.pcm -c %{module}/std.cppm",
-            )
-            moduleCompileFlags.append("%T/std.pcm")
-
-    # Add compile flags specified with ADDITIONAL_COMPILE_FLAGS.
-    substitutions = [
-        (s, x + " " + " ".join(additionalCompileFlags))
-        if s == "%{compile_flags}"
-        else (s, x)
-        for (s, x) in substitutions
-    ]
-    # In order to use modules, additional compilation flags are required.
-    # Adding these to the %{compile_flags} gives a chicken and egg issue:
-    # - the modules need to be built with the same compilation flags as the
-    #   tests,
-    # - except for the module dependency, which does not exist.
-    # The issue is resolved by adding a private substitution.
-    substitutions.append(("%{module_flags}", " ".join(moduleCompileFlags)))
+        # Make sure the std module is added before std.compat. Libc++'s
+        # std.compat module will depend on its std module.  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.
+        script.insert(
+            0,
+            "%dbg(MODULE std) %{cxx} %{flags} "
+            f"{compileFlags} "
+            "-Wno-reserved-module-identifier -Wno-reserved-user-defined-literal "
+            "--precompile -o %T/std.pcm -c %{module}/std.cppm",
+        )
+        moduleCompileFlags.append("%T/std.pcm")
+
+        # Add compile flags required for the modules.
+        substitutions = [
+            (s, x + " " + " ".join(moduleCompileFlags))
+            if s == "%{compile_flags}"
+            else (s, x)
+            for (s, x) in substitutions
+        ]
 
     # Perform substitutions in the script itself.
     script = lit.TestRunner.applySubstitutions(
@@ -250,7 +262,6 @@ class CxxStandardLibraryTest(lit.formats.FileBasedTest):
     constructs:
         %{cxx}           - A command that can be used to invoke the compiler
         %{compile_flags} - Flags to use when compiling a test case
-        %{module_flags}  - Flags to use when compiling a test case that imports modules
         %{link_flags}    - Flags to use when linking a test case
         %{flags}         - Flags to use either when compiling or linking a test case
         %{exec}          - A command to prefix the execution of executables
@@ -287,7 +298,7 @@ class CxxStandardLibraryTest(lit.formats.FileBasedTest):
 
            This directive will build the required C++23 standard library
            modules and add the additional compiler flags in
-           %{module_flags}. (Libc++ offers these modules in C++20 as an
+           %{compile_flags}. (Libc++ offers these modules in C++20 as an
            extension.)
 
     Additional provided substitutions and features
@@ -354,22 +365,22 @@ def execute(self, test, litConfig):
             ".compile.pass.mm"
         ):
             steps = [
-                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{module_flags} -fsyntax-only"
+                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} -fsyntax-only"
             ]
             return self._executeShTest(test, litConfig, steps)
         elif filename.endswith(".compile.fail.cpp"):
             steps = [
-                "%dbg(COMPILED WITH) ! %{cxx} %s %{flags} %{compile_flags} %{module_flags} -fsyntax-only"
+                "%dbg(COMPILED WITH) ! %{cxx} %s %{flags} %{compile_flags} -fsyntax-only"
             ]
             return self._executeShTest(test, litConfig, steps)
         elif filename.endswith(".link.pass.cpp") or filename.endswith(".link.pass.mm"):
             steps = [
-                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{module_flags} %{link_flags} -o %t.exe"
+                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{link_flags} -o %t.exe"
             ]
             return self._executeShTest(test, litConfig, steps)
         elif filename.endswith(".link.fail.cpp"):
             steps = [
-                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{module_flags} -c -o %t.o",
+                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} -c -o %t.o",
                 "%dbg(LINKED WITH) ! %{cxx} %t.o %{flags} %{link_flags} -o %t.exe",
             ]
             return self._executeShTest(test, litConfig, steps)
@@ -387,7 +398,7 @@ def execute(self, test, litConfig):
         # suffixes above too.
         elif filename.endswith(".pass.cpp") or filename.endswith(".pass.mm"):
             steps = [
-                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{module_flags} %{link_flags} -o %t.exe",
+                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{link_flags} -o %t.exe",
                 "%dbg(EXECUTED AS) %{exec} %t.exe",
             ]
             return self._executeShTest(test, litConfig, steps)
diff --git a/libcxx/utils/libcxx/test/modules.py b/libcxx/utils/libcxx/test/modules.py
index e2e94c239f21c7..3e9fcae4c5389a 100644
--- a/libcxx/utils/libcxx/test/modules.py
+++ b/libcxx/utils/libcxx/test/modules.py
@@ -126,7 +126,7 @@ def write_lit_configuration(self):
 // The GCC compiler flags are not always compatible with clang-tidy.
 // UNSUPPORTED: gcc
 
-// MODULES: {self.module}
+// MODULE_DEPENDENCIES: {self.module}
 
 // RUN: echo -n > {self.tmp_prefix}.all_partitions
 """



More information about the llvm-branch-commits mailing list