Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces an “infallible scheduler” concept and uses it to constrain task_scheduler, along with accompanying test assertions and paper (docs) updates related to allocation and scheduler affinity wording.
Changes:
- Add
beman::task::detail::infallible_schedulerconcept and apply it as a constraint ontask_schedulerconstruction. - Update scheduler-related tests to provide
get_completion_signaturesand assertinfallible_schedulercompliance. - Revise P3980/P3941 paper text to reflect updated wording and concepts (including
get_start_schedulerusage).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/beman/task/task_scheduler.test.cpp | Adds get_completion_signatures to a test sender and asserts infallible_scheduler for the test scheduler. |
| include/beman/task/detail/task_scheduler.hpp | Includes and constrains task_scheduler to only accept infallible schedulers in env<>. |
| include/beman/task/detail/inline_scheduler.hpp | Adds an include (currently unused) and an extra scheduler static_assert. |
| include/beman/task/detail/infallible_scheduler.hpp | New header defining completes_with / infallible_scheduler concepts. |
| docs/P3980-allocation.md | Updates paper metadata and wording around allocator/operator new requirements. |
| docs/P3941-affinity.md | Updates paper metadata and wording, adds infallible-scheduler exposition and shifts to get_start_scheduler. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #include <beman/execution/execution.hpp> | ||
| #include <concepts> | ||
| #include <type_traits> | ||
|
|
||
| // ---------------------------------------------------------------------------- | ||
|
|
||
| namespace beman::task::detail { | ||
| template <class Sch, class Env, class... Comp> | ||
| concept completes_with = | ||
| ::std::same_as<::beman::execution::completion_signatures<Comp...>, | ||
| ::beman::execution:: | ||
| completion_signatures_of_t<decltype(::beman::execution::schedule(::std::declval<Sch>())), Env>>; | ||
|
|
There was a problem hiding this comment.
std::declval is used in completes_with but this header doesn’t include <utility>, which is where std::declval is declared. Relying on transitive includes can break builds depending on include order; add #include <utility> (or otherwise ensure declval is available) in this header.
| template <typename S, typename Allocator = ::std::allocator<void>> | ||
| requires(not std::same_as<task_scheduler, std::remove_cvref_t<S>>) && | ||
| ::beman::execution::scheduler<::std::remove_cvref_t<S>> | ||
| ::beman::execution::scheduler<::std::remove_cvref_t<S>> && | ||
| ::beman::task::detail::infallible_scheduler<::std::remove_cvref_t<S>, ::beman::execution::env<>> | ||
| explicit task_scheduler(S&& s, Allocator = {}) | ||
| : scheduler(static_cast<concrete<std::decay_t<S>>*>(nullptr), std::forward<S>(s)) {} |
There was a problem hiding this comment.
The new infallible_scheduler constraint changes task_scheduler from “type-erase any scheduler” (per the class comment) to only accepting schedulers whose schedule completes infallibly in env<>. Please update the class documentation (and the listed completion signatures/error behavior) to match the new, stricter contract so users aren’t misled.
| #define INCLUDED_BEMAN_TASK_DETAIL_INLINE_SCHEDULER | ||
|
|
||
| #include <beman/execution/execution.hpp> | ||
| #include <beman/task/detail/infallible_scheduler.hpp> |
There was a problem hiding this comment.
<beman/task/detail/infallible_scheduler.hpp> is included here but nothing in this header references infallible_scheduler (or related helpers). Consider removing this include to avoid unnecessary header coupling and reduce compile time.
| #include <beman/task/detail/infallible_scheduler.hpp> |
| ill-formed.]{.add} Let `op` be an lvalue referring to the operation | ||
| type `Env` [such that <code>sender_in<_OutSndr_, Env></code> | ||
| is `true`]{.rm}. [If <code>get_start_scheduler(get_env(out_rcvr))</code> is ill-formed | ||
| or does not satisfy `@_infallible-scheduler_@<Env>` then evaluation of the |
There was a problem hiding this comment.
This uses @_infallible-scheduler_@<Env> but earlier the paper defines @_infallible-scheduler_@ as a two-parameter concept template<class Sch, class Env>. Update this reference to pass the scheduler type as well (e.g., based on decltype(get_start_scheduler(get_env(out_rcvr)))).
| or does not satisfy `@_infallible-scheduler_@<Env>` then evaluation of the | |
| or does not satisfy `@_infallible-scheduler_@<decltype(get_start_scheduler(get_env(out_rcvr))), Env>` then evaluation of the |
| } | ||
| ``` | ||
|
|
||
| ::: ednot |
There was a problem hiding this comment.
Typo in directive: ::: ednot should be ::: ednote (otherwise tooling that recognizes ednote blocks won’t pick this up).
| ::: ednot | |
| ::: ednote |
No description provided.