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

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx
            
<details>
<summary>Changes</summary>
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".
--
Full diff: https://github.com/llvm/llvm-project/pull/66385.diff

2 Files Affected:

- (modified) libcxx/cmake/caches/Generic-modules-lsv.cmake (+1-1) 
- (modified) libcxx/utils/libcxx/test/params.py (+15-31) 


<pre>
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 &quot;enable_modules=clang;enable_modules_lsv=True&quot; CACHE STRING &quot;&quot;)
+set(LIBCXX_TEST_PARAMS &quot;enable_modules=clang-lsv&quot; CACHE STRING &quot;&quot;)
 set(LIBCXXABI_TEST_PARAMS &quot;${LIBCXX_TEST_PARAMS}&quot; CACHE STRING &quot;&quot;)
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 = [&quot;none&quot;, &quot;clang&quot;]
-
-
-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=&quot;enable_modules&quot;,
-        choices=_allModules,
+        choices=[&quot;none&quot;, &quot;clang&quot;, &quot;clang-lsv&quot;],
         type=str,
-        help=&quot;Whether to build the test suite with modules enabled. Select &quot;
-        &quot;`clang` for Clang modules&quot;,
-        default=lambda cfg: next(s for s in _allModules if getModuleFlag(cfg, s)),
-        actions=lambda enable_modules: [
-            AddFeature(&quot;clang-modules-build&quot;),
-            AddCompileFlag(&quot;-fmodules&quot;),
-            AddCompileFlag(&quot;-fcxx-modules&quot;), # AppleClang disregards -fmodules entirely when compiling C++. This enables modules for C++.
+        help=&quot;Whether to build the test suite with modules enabled. &quot;
+             &quot;Select `clang` for Clang modules, and &#x27;clang-lsv&#x27; for Clang modules with Local Submodule Visibility.&quot;,
+        default=&quot;none&quot;,
+        actions=lambda modules: filter(None, [
+            AddFeature(&quot;clang-modules-build&quot;)           if modules in (&quot;clang&quot;, &quot;clang-lsv&quot;) else None,
+
+            # Note: AppleClang disregards -fmodules entirely when compiling C++, so we also pass -fcxx-modules
+            #       to enable modules for C++.
+            AddCompileFlag(&quot;-fmodules -fcxx-modules&quot;)   if modules in (&quot;clang&quot;, &quot;clang-lsv&quot;) else None,
+
             # Note: We use a custom modules cache path to make sure that we don&#x27;t reuse
             #       the default one, which can be shared across CI builds with different
             #       configurations.
-            AddCompileFlag(lambda cfg: f&quot;-fmodules-cache-path={cfg.test_exec_root}/ModuleCache&quot;),
-        ]
-        if enable_modules == &quot;clang&quot;
-        else [],
-    ),
-    Parameter(
-        name=&quot;enable_modules_lsv&quot;,
-        choices=[True, False],
-        type=bool,
-        default=False,
-        help=&quot;Whether to enable Local Submodule Visibility in the Modules build.&quot;,
-        actions=lambda lsv: [] if not lsv else [
-            AddCompileFlag(&quot;-Xclang -fmodules-local-submodule-visibility&quot;),
-        ],
+            AddCompileFlag(lambda cfg: f&quot;-fmodules-cache-path={cfg.test_exec_root}/ModuleCache&quot;) if modules in (&quot;clang&quot;, &quot;clang-lsv&quot;) else None,
+
+            AddCompileFlag(&quot;-Xclang -fmodules-local-submodule-visibility&quot;) if modules == &quot;clang-lsv&quot; else None,
+        ])
     ),
     Parameter(
         name=&quot;enable_exceptions&quot;,
</pre>
</details>


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