Task allocation by dietmarkuehl · Pull Request #74 · bemanproject/task

dietmarkuehl · 2026-03-23T02:50:01Z

No description provided.

coveralls · 2026-03-23T02:53:42Z

coverage: 94.212% (+0.09%) from 94.118%
when pulling e657f74 on task-allocation
into 70fa90d on main.

Copilot

Pull request overview

This PR introduces allocator propagation into the task state/promise plumbing by adding an allocator query to state_base and routing promise_type::get_allocator() through the active state, with corresponding updates to core task state types and unit tests.

Changes:

Add allocator_type, get_allocator(), and pure-virtual do_get_allocator() to state_base.
Implement allocator retrieval in detail::state (operation state) and detail::awaiter.
Update tests/mocks to satisfy the new state_base vtable requirement.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/beman/task/state_base.test.cpp	Updates mock state to implement `do_get_allocator()` (no assertions added yet).
tests/beman/task/promise_type.test.cpp	Updates test task state to implement `do_get_allocator()` (no allocator behavior asserted).
tests/beman/task/promise_base.test.cpp	Updates mock state to implement `do_get_allocator()` (no allocator behavior asserted).
include/beman/task/detail/state.hpp	Adds allocator plumbing to operation-state implementation.
include/beman/task/detail/state_base.hpp	Introduces allocator API and new pure virtual `do_get_allocator()`.
include/beman/task/detail/promise_type.hpp	Changes allocator lookup to defer to the active `state_base`.
include/beman/task/detail/awaiter.hpp	Adds allocator plumbing for awaited coroutines.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-23T02:54:33Z

include/beman/task/detail/state.hpp

+        if constexpr (requires { ::beman::execution::get_allocator(this->receiver); })
+            return ::beman::execution::get_allocator(this->receiver);


do_get_allocator() is querying get_allocator(this->receiver), but other environment queries in this type (e.g. stop token) correctly go through get_env(receiver). With the current code, receivers that provide an allocator via their environment (query(get_allocator_t) on env) will be ignored and the default allocator will be returned instead. Consider querying ::beman::execution::get_allocator(::beman::execution::get_env(this->receiver)) (and adjusting the requires accordingly) to match the established pattern in this file.

Suggested change

if constexpr (requires { ::beman::execution::get_allocator(this->receiver); })

return ::beman::execution::get_allocator(this->receiver);

if constexpr (requires {

::beman::execution::get_allocator(

::beman::execution::get_env(this->receiver));

})

return ::beman::execution::get_allocator(

::beman::execution::get_env(this->receiver));

Copilot · 2026-03-23T02:54:34Z

include/beman/task/detail/awaiter.hpp

+        if constexpr (requires { ::beman::execution::get_allocator(::beman::execution::get_env(this->parent)); })
+            return ::beman::execution::get_allocator(::beman::execution::get_env(this->parent));


do_get_allocator() uses ::beman::execution::get_env(this->parent) where this->parent is a std::coroutine_handle<ParentPromise>. Elsewhere in this file the environment is obtained from this->parent.promise(). As written, this is inconsistent and is likely ill-formed or will select the wrong overload, breaking allocator propagation for awaited coroutines. Consider using ::beman::execution::get_env(this->parent.promise()) (and matching it in the requires) to align with the scheduler logic above.

Suggested change

if constexpr (requires { ::beman::execution::get_allocator(::beman::execution::get_env(this->parent)); })

return ::beman::execution::get_allocator(::beman::execution::get_env(this->parent));

if constexpr (requires {

::beman::execution::get_allocator(::beman::execution::get_env(this->parent.promise()));

})

return ::beman::execution::get_allocator(::beman::execution::get_env(this->parent.promise()));

Copilot · 2026-03-23T02:54:34Z

include/beman/task/detail/promise_type.hpp


    auto get_scheduler() const noexcept -> scheduler_type { return this->get_state()->get_scheduler(); }
-    auto get_allocator() const noexcept -> allocator_type { return this->allocator; }
+    auto get_allocator() const noexcept -> allocator_type { return this->get_state()->get_allocator(); }


promise_type::get_allocator() now forwards to this->get_state()->get_allocator() but unlike get_environment() it doesn't assert that get_state() is non-null. If get_allocator is queried before start()/set_state() (e.g. via get_env()), this will dereference a null state pointer. Consider adding an assert(this->get_state()) (or a safe fallback allocator) similar to get_environment().

Suggested change

auto get_allocator() const noexcept -> allocator_type { return this->get_state()->get_allocator(); }

auto get_allocator() const noexcept -> allocator_type {

assert(this);

assert(this->get_state());

return this->get_state()->get_allocator();

}

Copilot · 2026-03-23T02:54:34Z

tests/beman/task/state_base.test.cpp

@@ -26,6 +28,7 @@ struct state : beman::task::detail::state_base<int, environment> {
        this->completed = true;
        return std::noop_coroutine();
    }
+    allocator_type  do_get_allocator() override { return allocator_type{}; }
    stop_token_type do_get_stop_token() override {


A new state_base::get_allocator() API was added, but this test only implements do_get_allocator() to satisfy the vtable and doesn't actually exercise the new get_allocator() behavior. Consider adding an assertion that s.get_allocator() returns the expected allocator instance/type to prevent regressions.

Copilot · 2026-03-23T02:54:35Z

tests/beman/task/promise_type.test.cpp

@@ -153,6 +155,7 @@ struct test_task : beman::task::detail::state_base<int, environment> {
        this->latch.count_down();
        return std::noop_coroutine();
    }
+    allocator_type  do_get_allocator() override { return allocator_type{}; }
    stop_token_type do_get_stop_token() override { return this->source.get_token(); }
    environment&    do_get_environment() override { return this->env; }


This test adds do_get_allocator() to compile with the new state_base interface, but it doesn't validate that promise_type::get_allocator() (or env get_allocator queries) are correctly routed through the state. Consider extending the test to query the allocator and assert the expected propagation/defaulting behavior.

Copilot · 2026-03-23T02:54:35Z

tests/beman/task/promise_base.test.cpp

@@ -62,6 +64,7 @@ struct state : bt::state_base<T, env<E...>> {
        this->completed = true;
        return std::noop_coroutine();
    }
+    allocator_type  do_get_allocator() override { return allocator_type{}; }
    stop_token_type do_get_stop_token() override {
        this->token = true;
        return this->source.get_token();


This test updates the mock state to implement do_get_allocator(), but it doesn't verify that promise_base/state interactions for the new allocator API work as intended (e.g. calling get_allocator() on the state). Consider adding an assertion that the allocator query path is exercised.

dietmarkuehl added 11 commits February 22, 2026 19:18

add a paper on task allocation

f927694

updated the overview

a87856f

added the missing member operator new

daacb37

fix date and typos

1930e4d

fixed a few issues, brought date up to date, and add LEWG outcome

dbdf2c0

fix some typos and update the execution reference

31c2b47

fixing a few more CI issues

ab16c4a

Merge branch 'main' into task-allocation

fa3f441

Merge branch 'main' into task-allocation

611a313

changed the code to take the allocator from the environment

520072e

clang-format

1bab4fb

dietmarkuehl requested review from bretbrownjr, steve-downey and wusatosi as code owners March 23, 2026 02:50

Copilot AI review requested due to automatic review settings March 23, 2026 02:50

Copilot started reviewing on behalf of dietmarkuehl March 23, 2026 02:50 View session

fix formatting issue

a67dd56

Copilot AI reviewed Mar 23, 2026

View reviewed changes

dietmarkuehl added 2 commits March 23, 2026 02:59

fixed environment access in get_allocator

89dd18e

clang-format again

e657f74

dietmarkuehl merged commit 1ac1fdf into main Mar 23, 2026
36 checks passed

dietmarkuehl deleted the task-allocation branch March 23, 2026 03:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Task allocation#74

Task allocation#74
dietmarkuehl merged 14 commits intomainfrom
task-allocation

dietmarkuehl commented Mar 23, 2026

Uh oh!

coveralls commented Mar 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		if constexpr (requires { ::beman::execution::get_allocator(this->receiver); })
		return ::beman::execution::get_allocator(this->receiver);

Conversation

dietmarkuehl commented Mar 23, 2026

Uh oh!

coveralls commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coveralls commented Mar 23, 2026 •

edited

Loading