Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates beman::execution::task and related senders/schedulers to better expose completion signatures (including adding get_completion_signatures() static overloads), tightens allocator retrieval logic, and adds new example programs demonstrating task-as-sender / allocator propagation patterns.
Changes:
- Adjust
tasksender completion-signature exposure (rename tocompletion_signatures, add staticget_completion_signatures()). - Add static
get_completion_signatures()toinline_scheduler::senderandtask_scheduler::sender; refine allocator detection/construction instate. - Add new examples (
task-sender,alloc-1,odd-completions) and wire some into the examples CMake build.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| include/beman/task/detail/task.hpp | Updates task sender completion-signature declarations/queries. |
| include/beman/task/detail/task_scheduler.hpp | Adds static completion-signature query for the type-erased scheduler’s sender. |
| include/beman/task/detail/inline_scheduler.hpp | Adds static completion-signature query for inline scheduler sender. |
| include/beman/task/detail/state.hpp | Tightens allocator retrieval to require construction into allocator_type. |
| include/beman/task/detail/promise_type.hpp | Removes noexcept from await_transform(Sender&&). |
| examples/task-sender.cpp | New example demonstrating allocator propagation + deferred coroutine invocation. |
| examples/alloc-1.cpp | New allocator-focused example using deferred coroutine invocation. |
| examples/odd-completions.cpp | New example intended to exercise unusual completion paths. |
| examples/CMakeLists.txt | Adds task-sender and alloc-1 to the examples build list. |
Comments suppressed due to low confidence (1)
examples/CMakeLists.txt:30
examples/odd-completions.cppwas added in this PR but isn’t listed inALL_EXAMPLES, so it won’t be built or run viactest. If it’s intended to be a runnable example, add it toALL_EXAMPLES(or document why it’s intentionally excluded).
set(TODO tls-scheduler into_optional issue-start-reschedules loop)
set(ALL_EXAMPLES
task-sender
task_scheduler
rvalue-task
customize
issue-symmetric-transfer
container
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| auto p = std::malloc(n); | ||
| std::cout << "global new(" << n << ")->" << p << "\n"; | ||
| return p; | ||
| } | ||
| void operator delete(void* ptr) noexcept { std::cout << "global operator delete()" << ptr << "\n"; } | ||
|
|
There was a problem hiding this comment.
The global operator new/operator delete overloads here violate the required allocation contract: operator new must not return nullptr on allocation failure (it should throw std::bad_alloc), and operator delete must release the memory (currently it only logs and leaks). Consider mirroring the pattern used in examples/alloc.cpp (free in delete, and provide a sized delete overload as well).
| auto p = std::malloc(n); | |
| std::cout << "global new(" << n << ")->" << p << "\n"; | |
| return p; | |
| } | |
| void operator delete(void* ptr) noexcept { std::cout << "global operator delete()" << ptr << "\n"; } | |
| void* p = std::malloc(n); | |
| if (!p) { | |
| throw std::bad_alloc(); | |
| } | |
| std::cout << "global new(" << n << ")->" << p << "\n"; | |
| return p; | |
| } | |
| void operator delete(void* ptr) noexcept { | |
| std::cout << "global operator delete()" << ptr << "\n"; | |
| std::free(ptr); | |
| } | |
| void operator delete(void* ptr, std::size_t n) noexcept { | |
| std::cout << "global operator delete(" << n << ")->" << ptr << "\n"; | |
| std::free(ptr); | |
| } |
| #include <beman/execution/execution.hpp> | ||
| #include <functional> | ||
| #include <iostream> | ||
| #include <memory> |
There was a problem hiding this comment.
This file uses std::pmr::polymorphic_allocator and std::pmr::memory_resource but doesn’t include <memory_resource>, which is the standard header that declares these types. Add the proper header(s) to avoid relying on transitive includes.
| #include <memory> | |
| #include <memory> | |
| #include <memory_resource> |
| auto p = std::malloc(n); | ||
| std::cout << " global new(" << n << ")->" << p << "\n"; | ||
| return p; | ||
| } | ||
| void operator delete(void* ptr) noexcept { std::cout << " global operator delete()" << ptr << "\n"; } | ||
|
|
There was a problem hiding this comment.
The global operator new/operator delete overloads here leak memory: operator delete only logs and never calls std::free (despite operator new using std::malloc). Also, operator new should throw std::bad_alloc instead of returning nullptr on failure. Align this with the implementation in examples/alloc.cpp (including a sized delete overload).
| auto p = std::malloc(n); | |
| std::cout << " global new(" << n << ")->" << p << "\n"; | |
| return p; | |
| } | |
| void operator delete(void* ptr) noexcept { std::cout << " global operator delete()" << ptr << "\n"; } | |
| if (void* p = std::malloc(n)) { | |
| std::cout << " global new(" << n << ")->" << p << "\n"; | |
| return p; | |
| } | |
| throw std::bad_alloc{}; | |
| } | |
| void operator delete(void* ptr) noexcept { | |
| std::cout << " global operator delete(" << ptr << ")\n"; | |
| std::free(ptr); | |
| } | |
| void operator delete(void* ptr, std::size_t n) noexcept { | |
| std::cout << " global sized operator delete(" << ptr << ", " << n << ")\n"; | |
| std::free(ptr); | |
| } |
| #include <beman/execution/task.hpp> | ||
| #include <beman/execution/execution.hpp> | ||
| #include <functional> | ||
| #include <iostream> | ||
| #include <memory> | ||
| #include <new> | ||
| #include <cstdlib> | ||
| #include <utility> | ||
|
|
There was a problem hiding this comment.
This example relies on std::pmr types (std::pmr::memory_resource, std::pmr::polymorphic_allocator) but does not include <memory_resource>. Add <memory_resource> (and any other direct headers) to ensure portable compilation.
| int main() { | ||
| return std::get<0>(ex::sync_wait([]() -> ex::task<int> { | ||
| std::cout << "Hello, world!\n"; | ||
| co_return co_await ex::just(0); | ||
| }()) | ||
| .value_or(std::tuple(-1))); | ||
| ex::sync_wait([](int value) -> ex::task<int> { | ||
| switch (value) { | ||
| default: |
There was a problem hiding this comment.
There is an early return at the start of main, so the subsequent ex::sync_wait(...) block is unreachable and will never run. Either remove the early return or restructure main so both examples execute as intended.
| @@ -0,0 +1,28 @@ | |||
| // examples/hello.cpp -*-C++-*- | |||
There was a problem hiding this comment.
This file header comment says examples/hello.cpp, but the filename is odd-completions.cpp. Update the header comment so it matches the actual file name (consistent with other examples like examples/odd-return.cpp).
| // examples/hello.cpp -*-C++-*- | |
| // examples/odd-completions.cpp -*-C++-*- |
| public: | ||
| using sender_concept = ::beman::execution::sender_t; | ||
| using completion_signatures = ::beman::execution::completion_signatures<::beman::execution::set_value_t()>; | ||
| template <typename...> | ||
| static consteval auto get_completion_signatures() noexcept -> completion_signatures { | ||
| return {}; | ||
| } |
There was a problem hiding this comment.
The class documentation lists ex::set_stopped() as a possible completion, but the sender’s declared completion_signatures only contains set_value_t(). Either update the documentation or include set_stopped_t() in the declared completion signatures so they stay consistent.
examples/task-sender.cpp
Outdated
| @@ -0,0 +1,153 @@ | |||
| // examples/hello.cpp -*-C++-*- | |||
There was a problem hiding this comment.
This file header comment says examples/hello.cpp, but the filename is task-sender.cpp. Please update the header comment to match the actual file name (consistent with other examples).
| // examples/hello.cpp -*-C++-*- | |
| // examples/task-sender.cpp -*-C++-*- |
examples/alloc-1.cpp
Outdated
| @@ -0,0 +1,117 @@ | |||
| // examples/hello.cpp -*-C++-*- | |||
There was a problem hiding this comment.
This file header comment says examples/hello.cpp, but the filename is alloc-1.cpp. Please update the header comment to match the actual file name (consistent with other examples).
| // examples/hello.cpp -*-C++-*- | |
| // examples/alloc-1.cpp -*-C++-*- |
No description provided.