[libcxx-commits] [libcxx] [libc++] Simplify how we select modules flavors in the test suite (PR #66385)

[Louis Dionne via libcxx-commits]

https://github.com/ldionne created https://github.com/llvm/llvm-project/pull/66385:

This gets rid of the separate parameter enable_modules_lsv in favor of adding a named option to the enable_modules parameter. The patch also removes the getModuleFlag helper, which was just a really complicated way of hardcoding "none".

>From 6205efdb1ef7062f7b9d48a5cd6447129b4e27a5 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 14 Sep 2023 10:47:27 -0400
Subject: [PATCH] [libc++] Simplify how we select modules flavors in the test
 suite

This gets rid of the separate parameter enable_modules_lsv in favor of
adding a named option to the enable_modules parameter. The patch also
removes the getModuleFlag helper, which was just a really complicated
way of hardcoding "none".
---
 libcxx/cmake/caches/Generic-modules-lsv.cmake |  2 +-
 libcxx/utils/libcxx/test/params.py            | 46 ++++++-------------
 2 files changed, 16 insertions(+), 32 deletions(-)

diff --git a/libcxx/cmake/caches/Generic-modules-lsv.cmake b/libcxx/cmake/caches/Generic-modules-lsv.cmake
index 66b76134c282b8a..395fccc21765066 100644
--- a/libcxx/cmake/caches/Generic-modules-lsv.cmake
+++ b/libcxx/cmake/caches/Generic-modules-lsv.cmake
@@ -1,2 +1,2 @@
-set(LIBCXX_TEST_PARAMS "enable_modules=clang;enable_modules_lsv=True" CACHE STRING "")
+set(LIBCXX_TEST_PARAMS "enable_modules=clang-lsv" CACHE STRING "")
 set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")
diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index e05c6689964c734..6fe466ec0c6f595 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -87,15 +87,6 @@ def getStdFlag(cfg, std):
     return None
 
 
-_allModules = ["none", "clang"]
-
-
-def getModuleFlag(cfg, enable_modules):
-    if enable_modules in _allModules:
-        return enable_modules
-    return None
-
-
 # fmt: off
 DEFAULT_PARAMETERS = [
     Parameter(
@@ -128,32 +119,25 @@ def getModuleFlag(cfg, enable_modules):
     ),
     Parameter(
         name="enable_modules",
-        choices=_allModules,
+        choices=["none", "clang", "clang-lsv"],
         type=str,
-        help="Whether to build the test suite with modules enabled. Select "
-        "`clang` for Clang modules",
-        default=lambda cfg: next(s for s in _allModules if getModuleFlag(cfg, s)),
-        actions=lambda enable_modules: [
-            AddFeature("clang-modules-build"),
-            AddCompileFlag("-fmodules"),
-            AddCompileFlag("-fcxx-modules"), # AppleClang disregards -fmodules entirely when compiling C++. This enables modules for C++.
+        help="Whether to build the test suite with modules enabled. "
+             "Select `clang` for Clang modules, and 'clang-lsv' for Clang modules with Local Submodule Visibility.",
+        default="none",
+        actions=lambda modules: filter(None, [
+            AddFeature("clang-modules-build")           if modules in ("clang", "clang-lsv") else None,
+
+            # Note: AppleClang disregards -fmodules entirely when compiling C++, so we also pass -fcxx-modules
+            #       to enable modules for C++.
+            AddCompileFlag("-fmodules -fcxx-modules")   if modules in ("clang", "clang-lsv") else None,
+
             # Note: We use a custom modules cache path to make sure that we don't reuse
             #       the default one, which can be shared across CI builds with different
             #       configurations.
-            AddCompileFlag(lambda cfg: f"-fmodules-cache-path={cfg.test_exec_root}/ModuleCache"),
-        ]
-        if enable_modules == "clang"
-        else [],
-    ),
-    Parameter(
-        name="enable_modules_lsv",
-        choices=[True, False],
-        type=bool,
-        default=False,
-        help="Whether to enable Local Submodule Visibility in the Modules build.",
-        actions=lambda lsv: [] if not lsv else [
-            AddCompileFlag("-Xclang -fmodules-local-submodule-visibility"),
-        ],
+            AddCompileFlag(lambda cfg: f"-fmodules-cache-path={cfg.test_exec_root}/ModuleCache") if modules in ("clang", "clang-lsv") else None,
+
+            AddCompileFlag("-Xclang -fmodules-local-submodule-visibility") if modules == "clang-lsv" else None,
+        ])
     ),
     Parameter(
         name="enable_exceptions",