Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements the affine_on sender adaptor, which ensures a sender completes on the same scheduler it was started on, even if the sender changes the scheduler. The scheduler is obtained from the receiver's environment using get_scheduler.
Changes:
- Adds new
affine_onsender adaptor with customization point support for nested senders - Updates
run_loopto use environment-dependent completion signatures based on stop token availability - Fixes tutorial example to remove unnecessary dereference operator
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| include/beman/execution/detail/affine_on.hpp | New sender adaptor implementation with scheduler affinity |
| include/beman/execution/detail/sender_has_affine_on.hpp | Concept to check if sender supports affine_on customization |
| include/beman/execution/detail/nested_sender_has_affine_on.hpp | Concept to check if nested sender supports affine_on |
| include/beman/execution/detail/just.hpp | Adds affine_on customization returning sender unchanged |
| include/beman/execution/detail/read_env.hpp | Adds affine_on customization returning sender unchanged |
| include/beman/execution/detail/then.hpp | Adds affine_on customization for nested sender forwarding |
| include/beman/execution/detail/write_env.hpp | Adds affine_on customization for nested sender forwarding |
| include/beman/execution/detail/run_loop.hpp | Updates completion signatures to be environment-dependent |
| include/beman/execution/detail/let.hpp | Adds noexcept specification and conditional error handling |
| include/beman/execution/execution.hpp | Includes affine_on header |
| tests/beman/execution/exec-affine-on.test.cpp | Comprehensive test suite for affine_on |
| tests/beman/execution/exec-run-loop-types.test.cpp | Updates tests for new completion signature behavior |
| src/beman/execution/CMakeLists.txt | Adds new headers to build |
| tests/beman/execution/CMakeLists.txt | Adds new test to build |
| docs/overview.md | Documents affine_on and write_env adaptors |
| docs/tutorial.mds | Fixes sync_wait usage example |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #define INCLUDED_INCLUDE_BEMAN_EXECUTION_DETAIL_AFFINE_ON | ||
|
|
||
| #include <beman/execution/detail/env.hpp> | ||
| #include <beman/execution/detail/env.hpp> |
There was a problem hiding this comment.
Duplicate include directive for env.hpp. Remove the duplicate include on line 8.
| #include <beman/execution/detail/env.hpp> |
| #if 0 | ||
| if constexpr (requires(const child_tag_t& t) { | ||
| { | ||
| t.affine_on(::beman::execution::detail::forward_like<Sender>(child), env) | ||
| } -> ::beman::execution::sender; | ||
| }) | ||
| #else | ||
| if constexpr (::beman::execution::detail::nested_sender_has_affine_on<Sender, Env>) | ||
| #endif |
There was a problem hiding this comment.
Commented-out code should be removed. If the alternative implementation using #if 0 is no longer needed, remove lines 102-108 and 110 entirely, keeping only the active implementation.
| #if 0 | |
| if constexpr (requires(const child_tag_t& t) { | |
| { | |
| t.affine_on(::beman::execution::detail::forward_like<Sender>(child), env) | |
| } -> ::beman::execution::sender; | |
| }) | |
| #else | |
| if constexpr (::beman::execution::detail::nested_sender_has_affine_on<Sender, Env>) | |
| #endif | |
| if constexpr (::beman::execution::detail::nested_sender_has_affine_on<Sender, Env>) |
|
|
||
| #include <beman/execution/detail/affine_on.hpp> | ||
| #include <beman/execution/detail/connect.hpp> | ||
| #include <beman/execution/detail/env.hpp> |
There was a problem hiding this comment.
Duplicate include directive for env.hpp. Remove the duplicate include on line 17.
| #include <beman/execution/detail/sender_in.hpp> | ||
| #include <beman/execution/detail/starts_on.hpp> | ||
| #include <beman/execution/detail/sync_wait.hpp> | ||
| #include <beman/execution/detail/env.hpp> |
There was a problem hiding this comment.
Duplicate include directive for env.hpp. Remove the duplicate include on line 17.
| #include <beman/execution/detail/env.hpp> |
| #include <beman/execution/detail/just.hpp> | ||
| #include <beman/execution/detail/read_env.hpp> | ||
| #include <beman/execution/detail/write_env.hpp> | ||
| #include <beman/execution/detail/prop.hpp> |
There was a problem hiding this comment.
Duplicate include directive for prop.hpp. Remove the duplicate include on line 18.
| #include <beman/execution/detail/starts_on.hpp> | ||
| #include <beman/execution/detail/sync_wait.hpp> | ||
| #include <beman/execution/detail/env.hpp> | ||
| #include <beman/execution/detail/prop.hpp> |
There was a problem hiding this comment.
Duplicate include directive for prop.hpp. Remove the duplicate include on line 18.
| #include <beman/execution/detail/prop.hpp> |
| auto set_value(auto&&...) && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | ||
| auto set_error(auto&&) && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | ||
| auto set_stopped() && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } |
There was a problem hiding this comment.
The expression this->awaiter_&& this->awaiter_->complete() uses the logical AND operator (&&) instead of the short-circuit evaluation operator. This should likely be this->awaiter_ && this->awaiter_->complete() with proper conditional evaluation, or use an if statement: if (this->awaiter_) this->awaiter_->complete();
| auto set_value(auto&&...) && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | |
| auto set_error(auto&&) && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | |
| auto set_stopped() && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | |
| auto set_value(auto&&...) && noexcept -> void { | |
| if (this->awaiter_) { | |
| this->awaiter_->complete(); | |
| } | |
| } | |
| auto set_error(auto&&) && noexcept -> void { | |
| if (this->awaiter_) { | |
| this->awaiter_->complete(); | |
| } | |
| } | |
| auto set_stopped() && noexcept -> void { | |
| if (this->awaiter_) { | |
| this->awaiter_->complete(); | |
| } | |
| } |
| auto set_value(auto&&...) && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | ||
| auto set_error(auto&&) && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | ||
| auto set_stopped() && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } |
There was a problem hiding this comment.
The expression this->awaiter_&& this->awaiter_->complete() uses the logical AND operator (&&) instead of the short-circuit evaluation operator. This should likely be this->awaiter_ && this->awaiter_->complete() with proper conditional evaluation, or use an if statement: if (this->awaiter_) this->awaiter_->complete();
| auto set_value(auto&&...) && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | |
| auto set_error(auto&&) && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | |
| auto set_stopped() && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | |
| auto set_value(auto&&...) && noexcept -> void { | |
| if (this->awaiter_) { | |
| this->awaiter_->complete(); | |
| } | |
| } | |
| auto set_error(auto&&) && noexcept -> void { | |
| if (this->awaiter_) { | |
| this->awaiter_->complete(); | |
| } | |
| } | |
| auto set_stopped() && noexcept -> void { | |
| if (this->awaiter_) { | |
| this->awaiter_->complete(); | |
| } | |
| } |
| auto set_value(auto&&...) && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | ||
| auto set_error(auto&&) && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | ||
| auto set_stopped() && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } |
There was a problem hiding this comment.
The expression this->awaiter_&& this->awaiter_->complete() uses the logical AND operator (&&) instead of the short-circuit evaluation operator. This should likely be this->awaiter_ && this->awaiter_->complete() with proper conditional evaluation, or use an if statement: if (this->awaiter_) this->awaiter_->complete();
| auto set_value(auto&&...) && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | |
| auto set_error(auto&&) && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | |
| auto set_stopped() && noexcept -> void { this->awaiter_&& this->awaiter_->complete(); } | |
| auto set_value(auto&&...) && noexcept -> void { | |
| if (this->awaiter_) { | |
| this->awaiter_->complete(); | |
| } | |
| } | |
| auto set_error(auto&&) && noexcept -> void { | |
| if (this->awaiter_) { | |
| this->awaiter_->complete(); | |
| } | |
| } | |
| auto set_stopped() && noexcept -> void { | |
| if (this->awaiter_) { | |
| this->awaiter_->complete(); | |
| } | |
| } |
docs/overview.md
Outdated
| <details> | ||
| <summary><code>affine_on(<i>sender</i>) -> <i>sender-of</i><<i>completions-of</i>(<i>sender</i>)></code></summary> | ||
| The expression <code>affine_on(<i>sender</i>)</code> creates | ||
| a sender which completes on the same scheduler it was started on, even if <code><i>sender</i></code> changes the scheduler. The scheduler to resume on is determined using <code>get_scheduler(get_env(<i>rcvr</i>))</code> where <code><i>rcvr</i></code> is the reciver the sender is <code>connect</code>ed to. |
There was a problem hiding this comment.
Corrected spelling of 'reciver' to 'receiver'.
| a sender which completes on the same scheduler it was started on, even if <code><i>sender</i></code> changes the scheduler. The scheduler to resume on is determined using <code>get_scheduler(get_env(<i>rcvr</i>))</code> where <code><i>rcvr</i></code> is the reciver the sender is <code>connect</code>ed to. | |
| a sender which completes on the same scheduler it was started on, even if <code><i>sender</i></code> changes the scheduler. The scheduler to resume on is determined using <code>get_scheduler(get_env(<i>rcvr</i>))</code> where <code><i>rcvr</i></code> is the receiver the sender is <code>connect</code>ed to. |
No description provided.