Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts when_all’s environment handling (notably get_stop_token) to better reflect the environment actually seen by its child operations, and extends tests to cover env-dependent senders composed with when_all.
Changes:
- Update
when_allto build its child environment usingenv{prop{get_stop_token, ...}}and introduce awhen_all_envtype transformation. - Use
when_all_env<env_of_t<Receiver>>forwhen_all’s internal state type computations andsender_inconstraints. - Extend
exec-read-envandexec-when-alltests with additional completion-signature and runtimesync_waitcoverage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| include/beman/execution/detail/when_all.hpp | Changes how when_all constructs/presents an env to its children and updates internal env-dependent type computations. |
| tests/beman/execution/exec-when-all.test.cpp | Adds runtime sync_wait checks involving read_env(get_stop_token) and when_all. |
| tests/beman/execution/exec-read-env.test.cpp | Adds more completion-signature checks for read_env under different env compositions and adds sync_wait cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| inline constexpr auto make_when_all_env = [](const ::beman::execution::inplace_stop_source& stop_src, | ||
| const auto& env) noexcept { | ||
| return ::beman::execution::detail::join_env( | ||
| ::beman::execution::detail::make_env(::beman::execution::get_stop_token, stop_src.get_token()), env); | ||
| ::beman::execution::env{::beman::execution::prop{::beman::execution::get_stop_token, stop_src.get_token()}}, | ||
| env); |
There was a problem hiding this comment.
when_all_t::get_completion_signatures() still computes child completion signatures using the unmodified Env type, but when_all actually connects child senders with an environment produced by make_when_all_env (i.e., when_all_env<Env> overriding get_stop_token). This can make get_completion_signatures<when_all(...), Env> disagree with the values delivered at runtime for env-dependent senders (e.g., read_env(get_stop_token) would be reported as never_stop_token but will receive an inplace_stop_token). Update the signature computation to use when_all_env<Env> consistently when querying value_types_of_t, error_types_of_t, and sends_stopped for the child senders.
| test_std::get_completion_signatures< | ||
| decltype(r), | ||
| decltype(test_detail::join_env( | ||
| test_std::env{test_std::prop{test_std::get_stop_token, std::declval<test_std::inplace_stop_token>()}}, | ||
| test_std::env{ | ||
| test_std::prop{test_std::get_stop_token, std::declval<test_std::never_stop_token>()}}))>()); |
There was a problem hiding this comment.
This test now uses test_detail::join_env(...), but in the BEMAN_HAS_MODULES configuration the file only imports beman.execution (which does not re-export detail::join_env). This will fail to compile with modules enabled. Add import beman.execution.detail; (or import beman.execution.detail.join_env;) under the #ifdef BEMAN_HAS_MODULES branch, or avoid referencing detail::join_env in this test.
| #include <beman/execution/detail/connect.hpp> | ||
| #include <beman/execution/detail/start.hpp> | ||
| #include <beman/execution/detail/get_stop_token.hpp> | ||
| #include <beman/execution/detail/sync_wait.hpp> |
There was a problem hiding this comment.
In the non-modules include path this test uses test_std::prop but does not include <beman/execution/detail/prop.hpp> directly, relying on it being pulled in transitively (currently via <beman/execution/detail/when_all.hpp>). Other tests that use test_std::prop include prop.hpp explicitly (e.g. tests/beman/execution/exec-affine-on.test.cpp:19). Consider adding the direct include to avoid brittle header dependencies.
| #include <beman/execution/detail/sync_wait.hpp> | |
| #include <beman/execution/detail/sync_wait.hpp> | |
| #include <beman/execution/detail/prop.hpp> |
|
Coverage is 95.542% — fix-when_all-environment into main. No base build found for |
No description provided.