[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
Sat Dec 23 02:48:04 PST 2023


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

>From fe2406d5fb6c01db9efadc9107c94585a6051c8a 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/3] [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          | 13 ++++
 libcxx/utils/libcxx/test/format.py            | 73 +++++++++++++++++--
 libcxx/utils/libcxx/test/modules.py           |  5 +-
 12 files changed, 194 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..58c33e5bb37475 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -317,6 +317,19 @@ 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..058d4b2155c69d 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,48 @@ 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 +187,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 build 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 +247,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 +280,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 provide the additional compiler flags in
+           %{module_flags}. (Libc++ offers these modules in C++20 as an
+           extenstion.)
 
     Additional provided substitutions and features
     ==============================================
@@ -288,22 +351,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 +384,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 57e435da2160032bc811fda14d791374287e3e53 Mon Sep 17 00:00:00 2001
From: Mark de Wever <koraq at xs4all.nl>
Date: Sat, 23 Dec 2023 11:46:53 +0100
Subject: [PATCH 2/3] Update libcxx/utils/libcxx/test/format.py

Co-authored-by: Will Hawkins <hawkinsw at obs.cr>
---
 libcxx/utils/libcxx/test/format.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py
index 058d4b2155c69d..427b48e2e03e3f 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -189,7 +189,7 @@ def parseScript(test, preamble):
     ]
     # 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 build with the same compilation flags as the
+    # - 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.

>From 907f44289410eb7095e9e3ffdf2f5f5d974b0c8a Mon Sep 17 00:00:00 2001
From: Mark de Wever <koraq at xs4all.nl>
Date: Sat, 23 Dec 2023 11:47:57 +0100
Subject: [PATCH 3/3] Apply suggestions from code review

Co-authored-by: Will Hawkins <hawkinsw at obs.cr>
---
 libcxx/utils/libcxx/test/format.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py
index 427b48e2e03e3f..17610972da038e 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -187,7 +187,7 @@ def parseScript(test, preamble):
         else (s, x)
         for (s, x) in substitutions
     ]
-    # In order to use modules additional compilation flags are required.
+    # 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,
@@ -283,9 +283,9 @@ class CxxStandardLibraryTest(lit.formats.FileBasedTest):
         // MODULE: std std.compat
 
            This directive will build the required C++23 standard library
-           modules and add the provide the additional compiler flags in
+           modules and add the additional compiler flags in
            %{module_flags}. (Libc++ offers these modules in C++20 as an
-           extenstion.)
+           extension.)
 
     Additional provided substitutions and features
     ==============================================



More information about the llvm-branch-commits mailing list