[libcxx-commits] [PATCH] D157744: [libc++][CI] Moves modules build to C++23.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/cmake/caches/Generic-cxx23-no-modules.cmake:1
+set(LIBCXX_ENABLE_STD_MODULES OFF CACHE BOOL "")
 set(LIBCXX_TEST_PARAMS "std=c++23" CACHE STRING "")
----------------
Instead of adding a new job with std modules disabled, I would instead do this:

```
diff --git a/libcxx/utils/ci/buildkite-pipeline.yml b/libcxx/utils/ci/buildkite-pipeline.yml
index bda040285bb2..bd24a8429686 100644
--- a/libcxx/utils/ci/buildkite-pipeline.yml
+++ b/libcxx/utils/ci/buildkite-pipeline.yml
@@ -308,6 +308,7 @@ steps:
         CC: "clang-15"
         CXX: "clang++-15"
         ENABLE_CLANG_TIDY: "On"
+        ENABLE_STD_MODULES: "Off"
     agents:
       queue: "libcxx-builders"
       os: "linux"
@@ -326,6 +327,7 @@ steps:
         CC: "clang-16"
         CXX: "clang++-16"
         ENABLE_CLANG_TIDY: "On"
+        ENABLE_STD_MODULES: "Off"
     agents:
       queue: "libcxx-builders"
       os: "linux"
@@ -1108,6 +1110,7 @@ steps:
       env:
           CC: "clang15"
           CXX: "clang++15"
+          ENABLE_STD_MODULES: "Off"
       agents:
         queue: "libcxx-builders"
         os: "freebsd"
```

Then it becomes more clear that this is a temporary option until it can be enabled by default everywhere (also, please document that env var as such so that we eventually clean it up).



================
Comment at: libcxx/utils/ci/buildkite-pipeline.yml:196-197
     env:
-        CC: "clang-${LLVM_HEAD_VERSION}"
-        CXX: "clang++-${LLVM_HEAD_VERSION}"
+        # Note modules require and absolute path for clang-scan-deps
+        # https://github.com/llvm/llvm-project/issues/61006
+        CC: "/usr/lib/llvm-${LLVM_HEAD_VERSION}/bin/clang"
----------------
Everywhere


================
Comment at: libcxx/utils/ci/run-buildbot:462
 ;;
+# Using C++20 modules requires Clang 17 or newer.
+# This is a transition build for older Clang versions.
----------------
Not needed anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157744