[libcxx-commits] [PATCH] D157625: [libc++][modules] Simplifies module testing.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/CMakeLists.txt:750
 config_define_if(LIBCXX_HAS_EXTERNAL_THREAD_API _LIBCPP_HAS_THREAD_API_EXTERNAL)
 config_define_if(LIBCXX_HAS_WIN32_THREAD_API _LIBCPP_HAS_THREAD_API_WIN32)
 config_define_if(LIBCXX_HAS_MUSL_LIBC _LIBCPP_HAS_MUSL_LIBC)
----------------
Commit message: let's say `Simplifies C++20 module testing.`

Just to make it real clear.


================
Comment at: libcxx/test/lit.local.cfg:42
+        # TODO MODULES test whether this is fixed with CMake 3.27.
+        quit()
+    elif std == "cxx23":
----------------
What is `quit()`?


================
Comment at: libcxx/utils/libcxx/test/features.py:283
+
+    Feature(name="libcpp-has-no-std-modules", when=_isGCC or _isMSVC or _isAppleClang),
 ]
----------------
I don't think this is required since we're keying off of the `__config_site` macro.


================
Comment at: libcxx/utils/libcxx/test/params.py:127
         actions=lambda enable_modules: [
             AddFeature("modules-build"),
             AddCompileFlag("-fmodules"),
----------------
In a separate patch, we should probably change this to `clang-modules-build` or `modules-build=clang` (and then we could have `modules-build=std`).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157625/new/

https://reviews.llvm.org/D157625