Feature/test with linux clang 23 - #302
Open
dietmarkuehl wants to merge 19 commits into
Open
Conversation
This seems to force linker errors with import std;
(and documenting the workaround) until LLVM fixes the underlying problem.
(for Clang only needed) Prevent Clang -Werror or fixed them too
There was a problem hiding this comment.
Pull request overview
This PR updates the project’s Linux Clang toolchain/CI configuration (targeting Clang/LLVM 23 and import std experiments) and adjusts tests/examples to compile cleanly under stricter warning/sanitizer settings.
Changes:
- Update CI workflows, presets, and toolchain logic to target newer LLVM/Clang and newer CMake
import stdexperimental IDs. - Tweak tests/examples to reduce unused-variable warnings and improve multi-threaded logging output.
- Add a GNUmakefile-based local workflow and refresh pre-commit/dev-tool versions.
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/beman/execution/non_assignable.test.cpp | Adds [[maybe_unused]] to avoid unused-variable warnings in tests. |
| tests/beman/execution/exec-dependent-sender.test.cpp | Avoids unused lambda parameter warnings via commented parameter name. |
| tests/beman/execution/exec-continues-on.test.cpp | Marks an intermediate sender [[maybe_unused]] to silence warnings. |
| tests/beman/execution/exec-affine.test.cpp | Formatting + [[maybe_unused]] usage; adds TODO related to Clang warning. |
| tests/beman/execution/CMakeLists.txt | Disables TSan for one Clang test target. |
| requirements.txt | Bumps codespell minimum version. |
| Makefile | Updates local LLVM/GCC version notes/paths for Linux/Clang 23. |
| infra/cmake/llvm-libc++-toolchain.cmake | Extends libc++ toolchain flags (incl. fortify changes). |
| infra/cmake/enable-experimental-import-std.cmake | Updates the CMake experimental import std GUID selection for newer CMake. |
| infra/.beman_submodule | Updates infra submodule commit hash pointer. |
| include/beman/execution/detail/counting_scope_join.hpp | Adds TODO comment for a Clang warning area. |
| GNUmakefile | Adds a GNUmakefile-based build/tidy workflow helpers. |
| examples/stopping.cpp | Adds mutex + locking around std::cout from multiple threads. |
| CMakePresets.json | Enables BEMAN_USE_STD_MODULE in LLVM presets. |
| CMakeLists.txt | Removes module-mode shared-library option and a Windows DLL export workaround. |
| cmake/README.md | Removes the CMake helper documentation file. |
| cmake/prelude.cmake | Adjusts CMake-version gating and Linux libc++/modules JSON discovery behavior. |
| cmake/cxx-modules-rules.cmake | Changes module scan/warning flags and injects env-based flags into compile/link options. |
| .pre-commit-config.yaml | Updates codespell and isort hook revisions. |
| .gitignore | Ignores .init (used by GNUmakefile workflow). |
| .github/workflows/pre-commit-update.yml | Updates reusable workflow reference. |
| .github/workflows/ci_tests.yml | Updates reusable workflow refs, adds concurrency, and updates the build matrix. |
Comments suppressed due to low confidence (2)
cmake/cxx-modules-rules.cmake:73
- add_link_options($ENV{CXXFLAGS}) has the same issue as add_compile_options: if CXXFLAGS contains multiple flags separated by spaces, they will be passed as one invalid argument. Reuse the list produced by separate_arguments and pass that list to add_link_options.
add_link_options($ENV{CXXFLAGS})
examples/stopping.cpp:100
- Same concern as above: the comma-operator + temporary lock_guard pattern is non-idiomatic and obscures the critical section. Prefer an explicit scoped lock_guard.
(void)std::lock_guard(mtx), std::cout << "requesting stop\n";
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+68
to
+72
| add_compile_options( | ||
| $ENV{CXXFLAGS} | ||
| -Wno-missing-braces | ||
| -Wno-unneeded-internal-declaration | ||
| ) |
| inject_cancel_sender{token, ex::read_env(ex::get_stop_token) | ex::then([&mtx](ex::inplace_stop_token tok) { | ||
| while (not tok.stop_requested()) { | ||
| std::cout << "sleeping\n"; | ||
| (void)std::lock_guard(mtx), std::cout << "sleeping\n"; |
Comment on lines
+107
to
109
| auto schedule() const noexcept { return sender{this->data_}; } | ||
| // TODO(CK): prevent -Wunneeded-internal-declaration error! | ||
| friend auto operator==(const test_scheduler&, const test_scheduler&) noexcept -> bool = default; |
Comment on lines
+56
to
+59
| # NOTE: double-colon targets which has no prerequisites must always remade by gmake? | ||
| # Prerequisite 'GNUmakefile' is newer than target '.init'. | ||
| # -> Must remake target '.init'. | ||
| # NOTE: impizit handled by gmake! GNUmakefile :: ; |
ClausKlein
reviewed
Jul 28, 2026
|
|
||
| if(NOT CMAKE_CXX_FLAGS MATCHES "-stdlib=libc\\+\\+") | ||
| string(APPEND CMAKE_CXX_FLAGS " -stdlib=libc++") | ||
| string(APPEND CMAKE_CXX_FLAGS " -stdlib=libc++ -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0") |
Collaborator
There was a problem hiding this comment.
this is a workaround not yet merged in infra:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.