Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds header-only Asio integration for capy: a co_await-able completion token, executor adapters/wrappers for Boost and standalone Asio, spawn/initiation glue and async_result specializations, plus multiple detail helpers for completion, continuation allocation, and context services. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Adapter as asio_executor_adapter
participant SpawnOp as asio_spawn_op / async_initiate
participant InitCoro as init coroutine / promise
participant Capy as Capy coroutine
participant I/O as I/O subsystem
Caller->>Adapter: submit async operation(token)
Adapter->>SpawnOp: asio_spawn_op::operator()(token)
SpawnOp->>InitCoro: async_initiate -> allocate promise (handler allocator)
InitCoro->>Capy: start capy coroutine with io_env (executor, stop token, allocator)
Capy->>I/O: co_await -> initiate async I/O with completion handler
I/O-->>InitCoro: invoke completion handler (inline or posted)
InitCoro->>Capy: store result / resume capy coroutine
Capy->>InitCoro: return/propagate result
InitCoro->>Caller: complete original completion token
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
An automated preview of the documentation is available at https://246.capy.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-04-17 00:27:08 UTC |
| { | ||
| return awaitable_t< | ||
| std::decay_t<Initiation>, | ||
| std::decay_t<Args>...>( |
There was a problem hiding this comment.
I don't think std::decay_t here is correct, because std::make_tuple makes more transformations than just decaying (e.g. std::reference_wrapper)
| standalone_asio_immediate_executor_helper::completed_immediately_t * completed_immediately = nullptr; | ||
|
|
||
| using allocator_type = std::pmr::polymorphic_allocator<void>; | ||
| allocator_type get_allocator() const {return env->frame_allocator;} |
There was a problem hiding this comment.
I wonder if this is a good choice? My understanding is that the frame allocator is tuned to allocate coroutine frames, which follow a predictable pattern with predictable sizes. Isn't propagating it down the Asio chain prone to disrupting the invariant to which it was tuned to? Maybe use asio::recycling_allocator, instead?
| standalone_asio_coroutine_completion_handler && | ||
| ) noexcept = default; | ||
|
|
||
| void operator()(Args ... args) |
| signal.slot(), | ||
| &ci); | ||
|
|
||
| using ci_t = boost::capy::detail::standalone_asio_immediate_executor_helper::completed_immediately_t; |
There was a problem hiding this comment.
This seems to be missing from the Asio version
| &ci); | ||
|
|
||
| using ci_t = boost::capy::detail::standalone_asio_immediate_executor_helper::completed_immediately_t; | ||
| ci = ci_t::initiating; |
There was a problem hiding this comment.
I don't see any code path setting completed_immediately_t::yes - how does this work?
There was a problem hiding this comment.
It doesn't because that's missing. Thank you :)
| template<Executor ExecutorType, IoRunnable Runnable, | ||
| boost::asio::completion_token_for<detail::completion_signature_for_io_runnable<Runnable>> Token | ||
| = boost::asio::default_completion_token_t<ExecutorType>> | ||
| auto asio_spawn(ExecutorType exec, Runnable && runnable, Token token) |
There was a problem hiding this comment.
What would happen if:
- I create and run an
asio::io_context. - From that context, I call
asio_spawnto create a capy task. - That task creates corosio I/O objects, like timers and sockets.
Would this work? Most user-facing libraries need corosio objects to be useful.
klemens-morgenstern
force-pushed
the
asio
branch
from
92e52b8 to
7e8b401
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (5)
include/boost/capy/asio/detail/completion_handler.hpp (3)
182-182:await_resumeassumesresult_has value.Dereferencing
result_without checkinghas_value()relies on the invariant that the completion handler is always invoked before resumption. While this is guaranteed by correct Asio usage, an assertion would help catch misuse during development.♻️ Optional assertion
- std::tuple<Ts...> await_resume() {return std::move(*result_); } + std::tuple<Ts...> await_resume() { + assert(result_.has_value() && "completion handler was not invoked"); + return std::move(*result_); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/completion_handler.hpp` at line 182, The await_resume method currently dereferences result_ without verification; add a debug-time check (e.g., an assertion) that result_.has_value() before returning std::move(*result_) in await_resume to catch misuse during development. Locate await_resume (method name) in the completion handler type that holds result_ and insert an appropriate assert or BOOST_ASSERT(result_.has_value()) (or equivalent project assertion macro) so the code still returns std::move(*result_) when the assertion passes.
36-59: Complex state machine for immediate completion detection.The
executemethod implements a subtle state machine to detect if a composed operation completed inline. The flow:
- If
initiatingormaybe, set tomaybeand execute inline- After
fn(), if state didn't becomeyes, reset toinitiatingThis handles nested composed operations correctly, but consider adding a brief comment explaining the state transitions for future maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/completion_handler.hpp` around lines 36 - 59, Add a short clarifying comment above the execute method (or at the top of its body) that explains the completed_immediately state transitions used by execute: what the flags initiating, maybe and yes represent, why we set *completed_immediately = maybe before calling fn(), and why we restore initiating if fn() did not flip the flag to yes; also note that falling through to exec.post via make_continuation occurs when the operation cannot complete inline. Reference the execute function and the variables completed_immediately, initiating, maybe, yes, exec.post, make_continuation and exec.context().get_frame_allocator() to make it easy for future maintainers to find the logic.
127-131: Completion handler takes arguments by value.
operator()(Args... args)copies each argument before forwarding toemplace. For large types, this adds unnecessary overhead.♻️ Proposed fix using perfect forwarding
- void operator()(Args ... args) + template<typename... Ts> + void operator()(Ts&&... args) { - result.emplace(std::forward<Args>(args)...); + result.emplace(std::forward<Ts>(args)...); std::move(handle)(); }Alternatively, if the signature must match exactly for Asio's handler requirements, consider using
Args&&...directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/completion_handler.hpp` around lines 127 - 131, The completion handler's operator() currently takes Args... by value and copies each argument; change operator() to take forwarding references (operator()(Args&&... args)) and forward them into result.emplace using std::forward<Args>(args)... so emplace receives originals without unnecessary copies; update any call-sites or type-deductions if needed, or if Asio requires exact signature, alternatively change to operator()(Args const&... args) to avoid copies for large types while preserving the required signature; reference: operator(), result.emplace and handle.include/boost/capy/asio/detail/asio_coroutine_unique_handle.hpp (1)
39-44: Consider adding&&qualifier tooperator()for clarity.Since
operator()releases ownership and the handle can only be resumed once, adding the rvalue-ref qualifier documents this move-only semantics more clearly. This also prevents accidental calls on lvalues.♻️ Proposed change
- void operator()() + void operator()() && { std::coroutine_handle<void>::from_address( handle.release() ).resume(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/asio_coroutine_unique_handle.hpp` around lines 39 - 44, The call operator currently releases and resumes the coroutine handle but is callable on lvalues; change the member function signature of operator() to be an rvalue-qualified overload (operator()&&) so it can only be invoked on temporaries, reflecting the move‑only semantics around handle.release(), and update any call sites or tests that call operator() on lvalues to either move the object or call the new rvalue-qualified operator; keep the body intact (using handle.release() and std::coroutine_handle<void>::from_address(...).resume()) so ownership is transferred before resume.include/boost/capy/asio/detail/continuation.hpp (1)
120-129: Returning reference to coroutine-localcontis fragile.
make_continuationreturns a reference tocontwhich is a member of the promise stored in the coroutine frame. The coroutine is initially suspended atinitial_suspend, so the frame is alive when the reference is returned. However, callers must be careful not to use this reference after the coroutine is destroyed. Consider adding a comment documenting this lifetime dependency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/continuation.hpp` around lines 120 - 129, make_continuation currently returns a reference to continuation::cont that lives inside the coroutine promise created by detail::make_continuation_helper; document this lifetime dependency by adding a clear comment above make_continuation (and optionally above detail::make_continuation_helper) stating that the returned continuation& aliases the coroutine frame's promise member cont, that the frame is only guaranteed alive while the coroutine hasn't been destroyed (e.g., until the coroutine is resumed/destroyed), and callers must not use the reference after the coroutine is destroyed—include references to make_continuation, detail::make_continuation_helper and cont in the comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/boost/capy/asio/boost.hpp`:
- Around line 669-689: The success path inside the noexcept-sensitive
await_resume handling currently returns std::current_exception() which is empty;
change the try branches in the non-void and void cases to return a
default-constructed std::exception_ptr{} on success instead of
std::current_exception(), i.e. in the block guarded by if constexpr
(!noexcept(r.await_resume())) where type = decltype(r.await_resume()), replace
the successful-return expressions that call std::current_exception() with
std::exception_ptr{} so that the function mirrors the standalone version and
signals success explicitly.
- Around line 451-494: Update the documentation blocks for both asio_spawn
overloads (the ExecutorType overload and the Context overload) to follow
async-doc guidelines: add explicit "Completion" detailing when/where the
coroutine completes and what conditions lead to success/failure, a
"Cancellation" section explaining supported cancellation points/behavior,
"Return" and "Errors/Exceptions" describing returned/propagated types and thrown
exceptions, "Concurrency" or "Thread-safety" notes if relevant, brief usage
examples under "Examples", and ensure the `@see` tag is placed last; keep
parameter descriptions for exec/ctx, runnable, and token, and ensure completion
signature references detail::completion_signature_for_io_runnable<Runnable>.
- Around line 288-299: The construction/destruction of the tracked_executor in
work_started and work_finished is racy because the atomic counter doesn't
serialize the actual object lifetime; add a simple synchronization primitive to
serialize the 0↔1 transition: introduce a mutex (e.g., lifecycle_mutex) and
acquire it around the branch that checks the counter and constructs/destroys the
object in both work_started(const Executor& exec) and work_finished(),
performing the same atomic increment/decrement but holding the mutex when you
test the previous value and call placement-new or the destructor on buffer for
tracked_executor so construction and destruction cannot overlap.
- Around line 589-608: The await_suspend implementation reads `ex` from the
coroutine promise after calling h.destroy(), causing a use-after-destroy; fix by
capturing the executor and any needed locals before destroying the promise
frame: create local copies (e.g., asio_executor_adapter ex_ = std::move(ex);
auto args_ = std::move(args); and build the appended handler) prior to calling
h.destroy(), then call h.destroy() and use those locals when calling
boost::asio::get_associated_immediate_executor and boost::asio::dispatch
(symbols: await_suspend, h.destroy(), ex, args, asio_executor_adapter,
get_associated_immediate_executor, dispatch).
In `@include/boost/capy/asio/detail/asio_context_service.hpp`:
- Line 23: The declaration uses a dependent type Context::id without the
required typename; update the static member declaration (static Context::id id)
to include the typename keyword (static typename Context::id id) so the compiler
recognizes Context::id as a type (apply this change where the Context::id static
member is declared in asio_context_service.hpp).
- Around line 32-33: The out-of-line definition for the dependent type is
missing the typename keyword: update the definition of
asio_context_service<Context>::id so the dependent type is declared as typename
Context::id (i.e., prepend "typename" before Context::id in the definition of
asio_context_service<Context>::id) to correctly refer to the nested type.
In `@include/boost/capy/asio/detail/continuation.hpp`:
- Around line 44-56: The delete implementation in operator delete computes
padded size m from n but then calls alloc.deallocate with the original n,
causing a size mismatch; update operator delete to pass the full allocated size
(the padded m plus sizeof(alloc_t) used in operator new) to alloc.deallocate so
the deallocation matches the allocation (i.e., use the same computed m and
include sizeof(alloc_t) when calling alloc.deallocate on the pointer), keeping
the existing logic around alloc_t, m, n, and alloc.deallocate.
In `@include/boost/capy/asio/executor_adapter.hpp`:
- Around line 255-258: The operator!= implementation in asio_executor_adapter
incorrectly uses && so it only returns true when both executor_ and allocator_
differ; change its logic to return true when either field differs (use logical
OR between executor_ != rhs.executor_ and allocator_ != rhs.allocator_) so that
operator!= is the proper complement of operator== for asio_executor_adapter
while preserving the noexcept spec.
- Around line 120-147: The templated cross-type copy/move ctors in
asio_executor_adapter miss the same-type copy/move overloads so copying/moving
an adapter with identical Bits skips the on_work_started()/on_work_finished()
pairing; add explicit same-type copy constructor and same-type move constructor
(or delete them if intended) for asio_executor_adapter<Executor, Allocator,
Bits> that perform the same work-tracking logic: copy/move executor_ and
allocator_ and, if ((Bits & work_mask) == work_tracked), call
executor_.on_work_started() in the constructor (and ensure destructor still
calls on_work_finished()), so same-type copies/moves follow the same
resource-tracking path as the templated versions.
In `@include/boost/capy/asio/executor_from_asio.hpp`:
- Around line 63-83: The concepts AsioBoostStandardExecutor and
AsioStandaloneStandardExecutor rely on the internal type
execution::detail::context_t<0>; replace that with checks against the public
execution::context_t property by using a requires-expression or public query
trait (e.g., requires { boost::asio::query(Executor{},
boost::asio::execution::context_t); } / requires { ::asio::query(Executor{},
::asio::execution::context_t); } ) and then verify the query result is an
execution_context& (instead of referring to execution::detail::context_t<0>),
updating the use sites that currently reference boost::asio::query_result /
::asio::query_result and std::same_as to use the public query-based check for
AsioBoostStandardExecutor and AsioStandaloneStandardExecutor.
In `@include/boost/capy/asio/spawn.hpp`:
- Around line 117-142: Both operator() overloads currently move from executor_
and runnable_, making repeated calls use moved-from state; change the signatures
of both template operator()(Token && token) overloads to be rvalue-qualified
(add && after the parameter list) so they can only be called on an rvalue
asio_spawn_op, e.g. template<...> auto operator()(Token&& token) && { return
detail::initialize_asio_spawn_helper<...>::init(std::move(executor_),
std::move(runnable_), std::forward<Token>(token)); } and similarly
rvalue-qualify the standalone overload that calls
detail::initialize_asio_standalone_spawn_helper to prevent accidental multiple
invocations on the same object.
In `@include/boost/capy/asio/standalone.hpp`:
- Around line 281-293: The counter-only synchronization causes a lifetime race
on buffer; protect the tracked_executor storage by introducing a dedicated
synchronization around the placement-new and destructor operations: add a
storage mutex (or similar lock) and acquire it in work_started() when
constructing into buffer and in work_finished() when destroying from buffer,
while keeping the atomic work counter updates; ensure you still use
work.fetch_add(1u) and work.fetch_sub(1u) to decide whether to
construct/destruct but perform the placement-new and the destructor call under
the storage lock (referencing work_started, work_finished, work, buffer, and
tracked_executor).
- Around line 552-570: The coroutine destroys its frame with h.destroy() but
then reads ex to build asio_executor_adapter aex, causing use-after-destroy; fix
by not accessing ex after h.destroy(): either remove aex if unused, or construct
aex from the already-moved-in ex_ (or build aex before calling h.destroy()) and
then use that local (ex_ or aex) in the subsequent
::asio::get_associated_immediate_executor/::asio::dispatch calls; update
await_suspend to reference the safe local (ex_ or the prebuilt aex) instead of
the destroyed ex.
---
Nitpick comments:
In `@include/boost/capy/asio/detail/asio_coroutine_unique_handle.hpp`:
- Around line 39-44: The call operator currently releases and resumes the
coroutine handle but is callable on lvalues; change the member function
signature of operator() to be an rvalue-qualified overload (operator()&&) so it
can only be invoked on temporaries, reflecting the move‑only semantics around
handle.release(), and update any call sites or tests that call operator() on
lvalues to either move the object or call the new rvalue-qualified operator;
keep the body intact (using handle.release() and
std::coroutine_handle<void>::from_address(...).resume()) so ownership is
transferred before resume.
In `@include/boost/capy/asio/detail/completion_handler.hpp`:
- Line 182: The await_resume method currently dereferences result_ without
verification; add a debug-time check (e.g., an assertion) that
result_.has_value() before returning std::move(*result_) in await_resume to
catch misuse during development. Locate await_resume (method name) in the
completion handler type that holds result_ and insert an appropriate assert or
BOOST_ASSERT(result_.has_value()) (or equivalent project assertion macro) so the
code still returns std::move(*result_) when the assertion passes.
- Around line 36-59: Add a short clarifying comment above the execute method (or
at the top of its body) that explains the completed_immediately state
transitions used by execute: what the flags initiating, maybe and yes represent,
why we set *completed_immediately = maybe before calling fn(), and why we
restore initiating if fn() did not flip the flag to yes; also note that falling
through to exec.post via make_continuation occurs when the operation cannot
complete inline. Reference the execute function and the variables
completed_immediately, initiating, maybe, yes, exec.post, make_continuation and
exec.context().get_frame_allocator() to make it easy for future maintainers to
find the logic.
- Around line 127-131: The completion handler's operator() currently takes
Args... by value and copies each argument; change operator() to take forwarding
references (operator()(Args&&... args)) and forward them into result.emplace
using std::forward<Args>(args)... so emplace receives originals without
unnecessary copies; update any call-sites or type-deductions if needed, or if
Asio requires exact signature, alternatively change to operator()(Args const&...
args) to avoid copies for large types while preserving the required signature;
reference: operator(), result.emplace and handle.
In `@include/boost/capy/asio/detail/continuation.hpp`:
- Around line 120-129: make_continuation currently returns a reference to
continuation::cont that lives inside the coroutine promise created by
detail::make_continuation_helper; document this lifetime dependency by adding a
clear comment above make_continuation (and optionally above
detail::make_continuation_helper) stating that the returned continuation&
aliases the coroutine frame's promise member cont, that the frame is only
guaranteed alive while the coroutine hasn't been destroyed (e.g., until the
coroutine is resumed/destroyed), and callers must not use the reference after
the coroutine is destroyed—include references to make_continuation,
detail::make_continuation_helper and cont in the comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b7196eed-2923-4aba-a511-42d3b4999546
⛔ Files ignored due to path filters (7)
doc/modules/ROOT/nav.adocis excluded by!**/doc/**doc/modules/ROOT/pages/4.coroutines/4i.asio-integration.adocis excluded by!**/doc/**test/unit/CMakeLists.txtis excluded by!**/test/**test/unit/asio.cppis excluded by!**/test/**test/unit/asio_both.cppis excluded by!**/test/**test/unit/asio_standalone.cppis excluded by!**/test/**test/unit/test_helpers.hppis excluded by!**/test/**
📒 Files selected for processing (12)
include/boost/capy/asio/as_io_awaitable.hppinclude/boost/capy/asio/boost.hppinclude/boost/capy/asio/detail/asio_context_service.hppinclude/boost/capy/asio/detail/asio_coroutine_unique_handle.hppinclude/boost/capy/asio/detail/completion_handler.hppinclude/boost/capy/asio/detail/completion_traits.hppinclude/boost/capy/asio/detail/continuation.hppinclude/boost/capy/asio/detail/fwd.hppinclude/boost/capy/asio/executor_adapter.hppinclude/boost/capy/asio/executor_from_asio.hppinclude/boost/capy/asio/spawn.hppinclude/boost/capy/asio/standalone.hpp
| /** @brief Spawns a capy coroutine with a Boost.Asio completion token (executor overload). | ||
| * | ||
| * Convenience overload that combines the two-step spawn process into one call. | ||
| * Equivalent to `asio_spawn(exec, runnable)(token)`. | ||
| * | ||
| * @tparam ExecutorType The executor type | ||
| * @tparam Runnable The coroutine type | ||
| * @tparam Token A Boost.Asio completion token | ||
| * @param exec The executor to run on | ||
| * @param runnable The coroutine to spawn | ||
| * @param token The completion token | ||
| * @return Depends on the token type | ||
| */ | ||
| template<Executor ExecutorType, | ||
| IoRunnable Runnable, | ||
| boost::asio::completion_token_for< | ||
| detail::completion_signature_for_io_runnable<Runnable> | ||
| > Token> | ||
| auto asio_spawn(ExecutorType exec, Runnable && runnable, Token token) | ||
| { | ||
| return asio_spawn(exec, std::forward<Runnable>(runnable))(std::move(token)); | ||
| } | ||
|
|
||
| /** @brief Spawns a capy coroutine with a Boost.Asio completion token (context overload). | ||
| * | ||
| * Convenience overload that extracts the executor from a context. | ||
| * | ||
| * @tparam Context The execution context type | ||
| * @tparam Runnable The coroutine type | ||
| * @tparam Token A Boost.Asio completion token | ||
| * @param ctx The execution context | ||
| * @param runnable The coroutine to spawn | ||
| * @param token The completion token | ||
| * @return Depends on the token type | ||
| */ | ||
| template<ExecutionContext Context, | ||
| IoRunnable Runnable, | ||
| boost::asio::completion_token_for< | ||
| detail::completion_signature_for_io_runnable<Runnable> | ||
| > Token> | ||
| auto asio_spawn(Context & ctx, Runnable && runnable, Token token) | ||
| { | ||
| return asio_spawn(ctx.get_executor(), std::forward<Runnable>(runnable))(std::move(token)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fill in the required async docs for asio_spawn.
These overloads introduce public async API, but the current javadocs still miss the required cancellation section, completion conditions, return/error semantics, throws, remarks/examples, and @see placement.
As per coding guidelines, async/awaitable function javadoc must document completion conditions, concurrency, parameters, return values, throws, cancellation support, examples, and place @see last.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@include/boost/capy/asio/boost.hpp` around lines 451 - 494, Update the
documentation blocks for both asio_spawn overloads (the ExecutorType overload
and the Context overload) to follow async-doc guidelines: add explicit
"Completion" detailing when/where the coroutine completes and what conditions
lead to success/failure, a "Cancellation" section explaining supported
cancellation points/behavior, "Return" and "Errors/Exceptions" describing
returned/propagated types and thrown exceptions, "Concurrency" or
"Thread-safety" notes if relevant, brief usage examples under "Examples", and
ensure the `@see` tag is placed last; keep parameter descriptions for exec/ctx,
runnable, and token, and ensure completion signature references
detail::completion_signature_for_io_runnable<Runnable>.
| using type = decltype(r.await_resume()); | ||
| if constexpr (!noexcept(r.await_resume())) | ||
| { | ||
| if constexpr (std::is_void_v<type>) | ||
| try | ||
| { | ||
| r.await_resume(); | ||
| return {std::exception_ptr()}; | ||
| } | ||
| catch (...) | ||
| { | ||
| return std::current_exception(); | ||
| } | ||
| else | ||
| try | ||
| { | ||
| return {std::current_exception(), r.await_resume()}; | ||
| } | ||
| catch (...) | ||
| { | ||
| return {std::current_exception(), type()}; |
There was a problem hiding this comment.
Return an empty exception_ptr on the success path.
Inside the try branch there is no active exception, so std::current_exception() is empty by definition. This should mirror the standalone version and report success with a default-constructed std::exception_ptr{} instead.
Suggested fix
else
try
{
- return {std::current_exception(), r.await_resume()};
+ return {std::exception_ptr(), r.await_resume()};
}
catch (...)
{
return {std::current_exception(), type()};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| using type = decltype(r.await_resume()); | |
| if constexpr (!noexcept(r.await_resume())) | |
| { | |
| if constexpr (std::is_void_v<type>) | |
| try | |
| { | |
| r.await_resume(); | |
| return {std::exception_ptr()}; | |
| } | |
| catch (...) | |
| { | |
| return std::current_exception(); | |
| } | |
| else | |
| try | |
| { | |
| return {std::current_exception(), r.await_resume()}; | |
| } | |
| catch (...) | |
| { | |
| return {std::current_exception(), type()}; | |
| using type = decltype(r.await_resume()); | |
| if constexpr (!noexcept(r.await_resume())) | |
| { | |
| if constexpr (std::is_void_v<type>) | |
| try | |
| { | |
| r.await_resume(); | |
| return {std::exception_ptr()}; | |
| } | |
| catch (...) | |
| { | |
| return std::current_exception(); | |
| } | |
| else | |
| try | |
| { | |
| return {std::exception_ptr(), r.await_resume()}; | |
| } | |
| catch (...) | |
| { | |
| return {std::current_exception(), type()}; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@include/boost/capy/asio/boost.hpp` around lines 669 - 689, The success path
inside the noexcept-sensitive await_resume handling currently returns
std::current_exception() which is empty; change the try branches in the non-void
and void cases to return a default-constructed std::exception_ptr{} on success
instead of std::current_exception(), i.e. in the block guarded by if constexpr
(!noexcept(r.await_resume())) where type = decltype(r.await_resume()), replace
the successful-return expressions that call std::current_exception() with
std::exception_ptr{} so that the function mirrors the standalone version and
signals success explicitly.
| : Context::service | ||
| , capy::execution_context | ||
| { | ||
| static Context::id id; |
There was a problem hiding this comment.
Missing typename for dependent type.
Context::id is a dependent type and requires the typename keyword.
🐛 Proposed fix
- static Context::id id;
+ static typename Context::id id;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static Context::id id; | |
| static typename Context::id id; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@include/boost/capy/asio/detail/asio_context_service.hpp` at line 23, The
declaration uses a dependent type Context::id without the required typename;
update the static member declaration (static Context::id id) to include the
typename keyword (static typename Context::id id) so the compiler recognizes
Context::id as a type (apply this change where the Context::id static member is
declared in asio_context_service.hpp).
| concept AsioBoostStandardExecutor = std::same_as< | ||
| typename boost::asio::query_result< | ||
| Executor, | ||
| boost::asio::execution::detail::context_t<0>>::type, | ||
| boost::asio::execution_context&>; | ||
|
|
||
| /** @brief Concept for standalone Asio standard executors. | ||
| * @internal | ||
| * | ||
| * Matches executors compatible with the P0443/P2300 executor model as | ||
| * implemented in standalone Asio. These executors return | ||
| * `::asio::execution_context&` from context queries. | ||
| * | ||
| * @tparam Executor The type to check | ||
| */ | ||
| template<typename Executor> | ||
| concept AsioStandaloneStandardExecutor = std::same_as< | ||
| typename ::asio::query_result< | ||
| Executor, | ||
| ::asio::execution::detail::context_t<0>>::type, | ||
| ::asio::execution_context&>; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Boost.Asio and standalone Asio, is execution::detail::context_t<0> part of the documented public API, and what public type/property should be used instead when constraining executors by their execution-context query result?
💡 Result:
No, execution::detail::context_t<0> is not part of the documented public API in Boost.Asio or standalone Asio. It is an internal implementation detail used within the execution/context.hpp header for traits and compatibility, particularly for older C++ standards lacking variable templates or deduced traits. The public type is execution::context_t, which is a typedef to detail::context_t<> and documented as "A property that is used to obtain the execution context that is associated with an executor." To constrain executors by their execution-context query result, use execution::context_t publicly. The query asio::query(ex, execution::context_t) returns execution_context& (or reference to derived), as required for I/O executors. Constraints typically check if the executor is associated with a specific execution_context instance (e.g., via dynamic_cast or address comparison) or type (e.g., std::is_base_of_v<execution_context, std::decay_t<decltype(query(ex, execution::context_t))>>), rather than the property type itself.
Citations:
- 1: https://www.boost.org/doc/libs/latest/doc/html/boost_asio/reference/execution__context_t.html
- 2: https://codebrowser.dev/boost/boost/libs/asio/include/boost/asio/execution/context.hpp.html
- 3: https://www.boost.org/doc/libs/latest/doc/html/boost_asio/reference/execution_context.html
- 4: https://think-async.com/Asio/asio-1.26.0/doc/asio/reference/ExecutionContext.html
- 5: https://www.boost.org/doc/libs/latest/doc/html/boost_asio/std_executors.html
- 6: https://boost.org/doc/libs/master/doc/html/boost_asio/std_executors.html
Replace execution::detail::context_t<0> with the public execution::context_t property.
Both concepts depend on Asio's internal execution::detail::context_t<0> type, which is not documented and will break on library updates. Use the public execution::context_t property instead via a requires expression like requires { asio::query(Executor{}, asio::execution::context_t); } or a type trait that queries the property on the public API surface.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@include/boost/capy/asio/executor_from_asio.hpp` around lines 63 - 83, The
concepts AsioBoostStandardExecutor and AsioStandaloneStandardExecutor rely on
the internal type execution::detail::context_t<0>; replace that with checks
against the public execution::context_t property by using a requires-expression
or public query trait (e.g., requires { boost::asio::query(Executor{},
boost::asio::execution::context_t); } / requires { ::asio::query(Executor{},
::asio::execution::context_t); } ) and then verify the query result is an
execution_context& (instead of referring to execution::detail::context_t<0>),
updating the use sites that currently reference boost::asio::query_result /
::asio::query_result and std::same_as to use the public query-based check for
AsioBoostStandardExecutor and AsioStandaloneStandardExecutor.
| template<detail::asio_spawn_token<Executor, Runnable> Token> | ||
| auto operator()(Token && token) | ||
| { | ||
| return detail::initialize_asio_spawn_helper<Runnable, Token>::init( | ||
| std::move(executor_), | ||
| std::move(runnable_), | ||
| std::forward<Token>(token) | ||
| ); | ||
| } | ||
|
|
||
| /** @brief Initiates the spawn with a standalone Asio completion token. | ||
| * | ||
| * @tparam Token A valid standalone Asio completion token | ||
| * @param token The completion token determining how to handle completion | ||
| * @return Depends on the token | ||
| */ | ||
| template<detail::asio_standalone_spawn_token<Executor, Runnable> Token> | ||
| auto operator()(Token && token) | ||
| { | ||
| return detail::initialize_asio_standalone_spawn_helper<Runnable, Token>::init | ||
| ( | ||
| std::move(executor_), | ||
| std::move(runnable_), | ||
| std::forward<Token>(token) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Multiple calls to operator() will use moved-from state.
Both overloads move from executor_ and runnable_, so calling operator() more than once results in undefined behavior. Consider either:
- Marking
operator()with&&qualifier to require moving theasio_spawn_opitself, or - Documenting that the operation can only be initiated once.
♻️ Option 1: Add && qualifier
template<detail::asio_spawn_token<Executor, Runnable> Token>
- auto operator()(Token && token)
+ auto operator()(Token && token) &&
{
return detail::initialize_asio_spawn_helper<Runnable, Token>::init(
std::move(executor_),
std::move(runnable_),
std::forward<Token>(token)
);
}
template<detail::asio_standalone_spawn_token<Executor, Runnable> Token>
- auto operator()(Token && token)
+ auto operator()(Token && token) &&
{
return detail::initialize_asio_standalone_spawn_helper<Runnable, Token>::init
(
std::move(executor_),
std::move(runnable_),
std::forward<Token>(token)
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@include/boost/capy/asio/spawn.hpp` around lines 117 - 142, Both operator()
overloads currently move from executor_ and runnable_, making repeated calls use
moved-from state; change the signatures of both template operator()(Token &&
token) overloads to be rvalue-qualified (add && after the parameter list) so
they can only be called on an rvalue asio_spawn_op, e.g. template<...> auto
operator()(Token&& token) && { return
detail::initialize_asio_spawn_helper<...>::init(std::move(executor_),
std::move(runnable_), std::forward<Token>(token)); } and similarly
rvalue-qualify the standalone overload that calls
detail::initialize_asio_standalone_spawn_helper to prevent accidental multiple
invocations on the same object.
| /** @brief Spawns a capy coroutine with a standalone Asio completion token (executor overload). | ||
| * | ||
| * Convenience overload that combines the two-step spawn process into one call. | ||
| * | ||
| * @tparam ExecutorType The executor type | ||
| * @tparam Runnable The coroutine type | ||
| * @tparam Token A standalone Asio completion token | ||
| * @param exec The executor to run on | ||
| * @param runnable The coroutine to spawn | ||
| * @param token The completion token | ||
| * @return Depends on the token type | ||
| */ | ||
| template<Executor ExecutorType, | ||
| IoRunnable Runnable, | ||
| ::asio::completion_token_for< | ||
| detail::completion_signature_for_io_runnable<Runnable> | ||
| > Token> | ||
| auto asio_spawn(ExecutorType exec, Runnable && runnable, Token token) | ||
| { | ||
| return asio_spawn(exec, std::forward<Runnable>(runnable))(std::move(token)); | ||
| } | ||
|
|
||
| /** @brief Spawns a capy coroutine with a standalone Asio completion token (context overload). | ||
| * | ||
| * Convenience overload that extracts the executor from a context. | ||
| * | ||
| * @tparam Context The execution context type | ||
| * @tparam Runnable The coroutine type | ||
| * @tparam Token A standalone Asio completion token | ||
| * @param ctx The execution context | ||
| * @param runnable The coroutine to spawn | ||
| * @param token The completion token | ||
| * @return Depends on the token type | ||
| */ | ||
| template<ExecutionContext Context, | ||
| IoRunnable Runnable, | ||
| ::asio::completion_token_for< | ||
| detail::completion_signature_for_io_runnable<Runnable> | ||
| > Token | ||
| > | ||
| auto asio_spawn(Context & ctx, Runnable && runnable, Token token) | ||
| { | ||
| return asio_spawn(ctx.get_executor(), std::forward<Runnable>(runnable)) | ||
| (std::move(token)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fill in the required async docs for asio_spawn.
These are new public async initiating functions, but the doc blocks still omit the repository-required pieces: suspension/async behavior, completion conditions, cancellation semantics, return/error aggregate details, throws, remarks/examples, and @see last.
As per coding guidelines, async/awaitable function javadoc must document completion conditions, concurrency, parameters, return values, throws, cancellation support, examples, and place @see last.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
include/boost/capy/asio/standalone.hpp (1)
164-178: Remove redundantstatickeyword for consistency.The
statickeyword on this free function template is inconsistent with other similarqueryfunctions in this file (e.g., Line 102). For templates,staticgives internal linkage which is redundant since templates have their own linkage rules. Consider removing it for consistency.Suggested fix
template<typename Executor, typename Allocator, int Bits> -static constexpr ::asio::execution::outstanding_work_t query( +constexpr ::asio::execution::outstanding_work_t query( const asio_executor_adapter<Executor, Allocator, Bits> &, ::asio::execution::outstanding_work_t) noexcept🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/standalone.hpp` around lines 164 - 178, The free function template "query" declared as "template<typename Executor, typename Allocator, int Bits> static constexpr ::asio::execution::outstanding_work_t query(...)" should drop the redundant "static" to match other query overloads; edit the declaration for the function that takes "const asio_executor_adapter<Executor, Allocator, Bits>&" and "::asio::execution::outstanding_work_t" so it becomes a non-static template function (keep the same return type, parameters, body and noexcept) to restore consistent linkage and style.include/boost/capy/asio/boost.hpp (2)
172-186: Removestaticfrom namespace-scope function template.The
statickeyword on a namespace-scope function template is unusual. For templates,staticgives internal linkage which may cause ODR violations if different translation units instantiate with the same types. The otherqueryoverloads in this file don't usestatic.🔧 Suggested fix
template<typename Executor, typename Allocator, int Bits> -static constexpr boost::asio::execution::outstanding_work_t query( +constexpr boost::asio::execution::outstanding_work_t query( const asio_executor_adapter<Executor, Allocator, Bits> &, boost::asio::execution::outstanding_work_t) noexcept🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/boost.hpp` around lines 172 - 186, The namespace-scope function template named query for asio_executor_adapter<Executor, Allocator, Bits> incorrectly uses the static specifier; remove the leading static to give the template external linkage like the other query overloads. Edit the template declaration "template<typename Executor, typename Allocator, int Bits> static constexpr boost::asio::execution::outstanding_work_t query(...)" to drop static so the function is "constexpr ... query(...)" while keeping the body and logic (use of ex::work_mask, ex::work_tracked, ex::work_untracked) unchanged.
163-166: Inconsistent indentation.The
constexpr int new_bitsstatement is not indented to match the surrounding code.🔧 Suggested fix
template<typename Executor, typename Allocator, int Bits> constexpr auto require(const asio_executor_adapter<Executor, Allocator, Bits> & exec, boost::asio::execution::blocking_t::always_t) { - using ex = asio_executor_adapter<Executor, Allocator, Bits>; -constexpr int new_bits = (Bits & ~ex::blocking_mask) | ex::blocking_always; + using ex = asio_executor_adapter<Executor, Allocator, Bits>; + constexpr int new_bits = (Bits & ~ex::blocking_mask) | ex::blocking_always; return asio_executor_adapter<Executor, Allocator, new_bits>(exec); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/boost.hpp` around lines 163 - 166, Indent the misaligned declaration so it matches surrounding lines: align the "constexpr int new_bits = (Bits & ~ex::blocking_mask) | ex::blocking_always;" line with the "using ex = asio_executor_adapter<Executor, Allocator, Bits>;" and the return statement inside the same block (i.e., same indentation level), ensuring it sits inside the function scope alongside asio_executor_adapter, blocking_mask, and blocking_always usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/boost/capy/asio/boost.hpp`:
- Around line 582-586: The promise type incorrectly defines return_value() for a
coroutine that returns no value; replace the return_value() member with a
return_void() member (matching the promise API for coroutines that fall off the
end or use co_return;) keeping the same empty body and noexcept/exception
semantics as the other promise methods (see existing unhandled_exception(),
initial_suspend(), final_suspend()); also verify any callers or tests
referencing completer::await_suspend() or promise::return_value() are updated to
the new return_void() name.
In `@include/boost/capy/asio/standalone.hpp`:
- Around line 542-547: The promise type currently defines void return_value()
but the coroutine boost_asio_standalone_init::operator() returns by falling off
the end (no co_return), so replace the promise's return_value() with
return_void() (matching the coroutine's void-return behavior); locate the
promise in include/boost/capy/asio/standalone.hpp and change the method
name/signature to return_void() (and maintain the same body and noexcept if
applicable) so the coroutine machinery calls the correct hook.
---
Nitpick comments:
In `@include/boost/capy/asio/boost.hpp`:
- Around line 172-186: The namespace-scope function template named query for
asio_executor_adapter<Executor, Allocator, Bits> incorrectly uses the static
specifier; remove the leading static to give the template external linkage like
the other query overloads. Edit the template declaration "template<typename
Executor, typename Allocator, int Bits> static constexpr
boost::asio::execution::outstanding_work_t query(...)" to drop static so the
function is "constexpr ... query(...)" while keeping the body and logic (use of
ex::work_mask, ex::work_tracked, ex::work_untracked) unchanged.
- Around line 163-166: Indent the misaligned declaration so it matches
surrounding lines: align the "constexpr int new_bits = (Bits &
~ex::blocking_mask) | ex::blocking_always;" line with the "using ex =
asio_executor_adapter<Executor, Allocator, Bits>;" and the return statement
inside the same block (i.e., same indentation level), ensuring it sits inside
the function scope alongside asio_executor_adapter, blocking_mask, and
blocking_always usage.
In `@include/boost/capy/asio/standalone.hpp`:
- Around line 164-178: The free function template "query" declared as
"template<typename Executor, typename Allocator, int Bits> static constexpr
::asio::execution::outstanding_work_t query(...)" should drop the redundant
"static" to match other query overloads; edit the declaration for the function
that takes "const asio_executor_adapter<Executor, Allocator, Bits>&" and
"::asio::execution::outstanding_work_t" so it becomes a non-static template
function (keep the same return type, parameters, body and noexcept) to restore
consistent linkage and style.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8aaa2806-c62f-41a8-8213-0b9c614c959b
📒 Files selected for processing (2)
include/boost/capy/asio/boost.hppinclude/boost/capy/asio/standalone.hpp
| | `void(error_code, std::size_t)` | ||
| | `std::tuple<error_code, std::size_t>` | ||
|
|
||
| | `void(error_code)` |
There was a problem hiding this comment.
I think this is incompatible with when_any and friends? Which expect io_result. I understand std::tuple provides consistency, but I'd consider using io_result.
| | `io_task<T>` (may throw) | ||
| | `void(std::exception_ptr, T)` | ||
|
|
||
| | `io_task<T>` (noexcept) |
There was a problem hiding this comment.
I'm not convinced of this being a good idea - at the end of the day coroutine frame allocation ends up being covered by this noexcept, and this could potentially throw. Also what happens with the error_code? does it get translated into an exception?
There was a problem hiding this comment.
That's the result of await_resume on the task.
If that's noexcept there's not exception_ptr. Not task<int> f() noexcept
| { | ||
| completed_immediately = completed_immediately_t::initiating; | ||
| stopper.emplace(env->stop_token, signal); | ||
| using slot_t = decltype(CancellationSignal().slot()); |
There was a problem hiding this comment.
Shouldn't this be decayed? just in case
|
|
||
| if (completed_immediately == completed_immediately_t::initiating) | ||
| completed_immediately = completed_immediately_t::no; | ||
| return completed_immediately != completed_immediately_t::yes; |
There was a problem hiding this comment.
completed_immediately_t::yes doesn't seem to be set anywhere, probably needs some unit tests?
There was a problem hiding this comment.
It works by adding the correct assignment to completion_handler::operator(). Sorry 'bout forgetting that a second time.
| { | ||
| return awaitable_t< | ||
| std::decay_t<Initiation>, | ||
| std::decay_t<Args>...>( |
There was a problem hiding this comment.
There's a mismatch between make_tuple and decay_t<Args>
There was a problem hiding this comment.
There is?
There was a problem hiding this comment.
make_tuple doesn't just decay, it transforms reference wrappers into references.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
include/boost/capy/asio/detail/completion_handler.hpp (2)
167-167:⚠️ Potential issue | 🟠 MajorDecay the slot type and avoid default-constructing the signal.
decltype(CancellationSignal().slot())still isn't decayed, so ifslot()returns a reference (as it does for Asio'scancellation_signal, which returnscancellation_slotby value but some custom signals return references),slot_tbecomes a reference type and then gets used as a non-reference template parameter forasio_coroutine_completion_handler<slot_t, Ts...>where it's stored by value — yielding a reference member. Additionally,CancellationSignal()requiresCancellationSignalto be default-constructible just to query the slot type; usestd::declvalinstead.🛠️ Proposed fix
- using slot_t = decltype(CancellationSignal().slot()); + using slot_t = std::decay_t< + decltype(std::declval<CancellationSignal&>().slot())>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/completion_handler.hpp` at line 167, The current typedef using `using slot_t = decltype(CancellationSignal().slot());` wrongly depends on default-constructing CancellationSignal and may yield a reference type; change it to use std::decay_t and std::declval so the type is decayed and no default construction is required — e.g. compute slot_t as the decayed type of `decltype(std::declval<CancellationSignal>().slot())`; update any use-sites such as the template instantiation of `asio_coroutine_completion_handler<slot_t, Ts...>` to rely on this corrected `slot_t`.
205-214:⚠️ Potential issue | 🟠 Major
std::make_tuplevsstd::decay_t<Args>...still mismatch forreference_wrapper.
std::make_tuple(std::forward<Args>(args)...)unwrapsstd::reference_wrapper<T>intoT&, whereasstd::decay_t<Args>...keepsreference_wrapper<T>. Any call site that forwards astd::reference_wrapperviaargswill hit a tuple-type mismatch atawaitable_tconstruction. Construct the tuple explicitly to keep the two in sync.🛠️ Proposed fix
- return awaitable_t< - std::decay_t<Initiation>, - std::decay_t<Args>...>( - std::forward<Initiation>(initiation), - std::make_tuple(std::forward<Args>(args)...)); + return awaitable_t< + std::decay_t<Initiation>, + std::decay_t<Args>...>( + std::forward<Initiation>(initiation), + std::tuple<std::decay_t<Args>...>(std::forward<Args>(args)...));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/completion_handler.hpp` around lines 205 - 214, The code uses std::make_tuple(std::forward<Args>(args)...) which will unwrap std::reference_wrapper<T> into T&, causing a mismatch with the target awaitable_t parameter pack of std::decay_t<Args>.... Change the tuple construction to an explicit tuple of the decayed types so the types match exactly; e.g. construct std::tuple<std::decay_t<Args>...>(std::forward<Args>(args)...) when calling awaitable_t in the initiate function to preserve reference_wrapper semantics for any std::reference_wrapper arguments.
🧹 Nitpick comments (4)
include/boost/capy/asio/detail/completion_handler.hpp (4)
85-91: Defensively decayCancellationSlotin the handler signature.If the fix on line 167 stores a reference in
slot_t, that reference will flow in here andCancellationSlot slot;becomes ill-formed or holds a dangling reference once the caller's slot temporary expires. Even after decaying at the deduction site, it's worth making the member itself robust to the template argument.🧹 Proposed refactor
-template<typename CancellationSlot, typename ... Args> -struct asio_coroutine_completion_handler -{ - asio_coroutine_unique_handle handle; - std::optional<std::tuple<Args...>> & result; - const capy::io_env * env; - CancellationSlot slot; +template<typename CancellationSlot, typename ... Args> +struct asio_coroutine_completion_handler +{ + asio_coroutine_unique_handle handle; + std::optional<std::tuple<Args...>> & result; + const capy::io_env * env; + std::decay_t<CancellationSlot> slot;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/completion_handler.hpp` around lines 85 - 91, The member type for CancellationSlot in asio_coroutine_completion_handler should be a decayed type to avoid storing references or dangling temporaries; change the member declaration `CancellationSlot slot;` to `std::decay_t<CancellationSlot> slot;` (and include <type_traits> if not already present) so the handler always holds a value type regardless of how CancellationSlot was deduced.
150-151: Value-initializecompleted_immediately.The member is left indeterminate until
await_suspendwrites it. Today it's only read after that write, but a future change (e.g. an early-ready path or inspection from a debug log) could read it prematurely and hit UB. A one-character fix makes the state machine safe-by-default.🧹 Proposed refactor
- CancellationSignal signal; - completed_immediately_t completed_immediately; + CancellationSignal signal; + completed_immediately_t completed_immediately{};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/completion_handler.hpp` around lines 150 - 151, The member completed_immediately_t completed_immediately is left indeterminate until await_suspend writes it; update its declaration to value-initialize it (e.g. completed_immediately_t completed_immediately{} or = completed_immediately_t{}), so the state machine is safe-by-default and avoids UB if read early; keep CancellationSignal as-is and only change the completed_immediately declaration.
23-24: Missing file-level implementation overview.This header implements a non-trivial state machine (the
initiating/maybe/yes/noprotocol shared betweenasio_immediate_executor_helper::execute,asio_coroutine_completion_handler::operator(), andawaitable_t::await_suspend). The interaction between the three is subtle — especially the "reset toinitiatingafterfn()returns without calling the handler" behavior on line 49-50, which is how composed operations distinguish a deferred inner op from a truly immediate completion. Future maintainers would benefit from a short/* */block after the includes describing the state transitions.As per coding guidelines: "Files containing non-trivial implementation logic should include a
/* */block comment after the includes that provides a high-level overview of how the implementation works."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/completion_handler.hpp` around lines 23 - 24, Add a file-level /* ... */ block comment immediately after the includes in completion_handler.hpp describing the initiating/maybe/yes/no state machine and its interactions across asio_immediate_executor_helper::execute, asio_coroutine_completion_handler::operator(), and awaitable_t::await_suspend; explicitly document the state transitions (initiating -> maybe -> yes/no), the semantics of "reset to initiating after fn() returns without calling the handler" used to distinguish deferred vs immediate completions, and a short example/sequence of events showing how composed operations should behave so future maintainers can quickly understand the protocol.
173-180:std::applylambda should forward, not copy.
[&](auto ... args)takes each tuple element by value, which forces copies of every forwarded argument and fails to compile for move-only args stored inargs_. Sinceargs_is moved in, the tuple elements are rvalues; forward them toinit_instead.🧹 Proposed refactor
- std::apply( - [&](auto ... args) - { - std::move(init_)( - std::move(ch), - std::move(args)...); - }, - std::move(args_)); + std::apply( + [&](auto&&... args) + { + std::move(init_)( + std::move(ch), + std::forward<decltype(args)>(args)...); + }, + std::move(args_));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/completion_handler.hpp` around lines 173 - 180, The lambda passed to std::apply in completion_handler.hpp currently captures and takes tuple elements by value, which forces copies and breaks for move-only args; change the lambda to accept forwarding references (auto&&... args) and forward each argument into init_ (using std::forward for each args) while still moving ch and args_ as intended so move-only tuple elements are preserved; update the std::apply call that invokes init_ with std::move(ch) and forwarded args (referring to init_, ch, and args_ in the existing code).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@include/boost/capy/asio/detail/completion_handler.hpp`:
- Line 167: The current typedef using `using slot_t =
decltype(CancellationSignal().slot());` wrongly depends on default-constructing
CancellationSignal and may yield a reference type; change it to use std::decay_t
and std::declval so the type is decayed and no default construction is required
— e.g. compute slot_t as the decayed type of
`decltype(std::declval<CancellationSignal>().slot())`; update any use-sites such
as the template instantiation of `asio_coroutine_completion_handler<slot_t,
Ts...>` to rely on this corrected `slot_t`.
- Around line 205-214: The code uses
std::make_tuple(std::forward<Args>(args)...) which will unwrap
std::reference_wrapper<T> into T&, causing a mismatch with the target
awaitable_t parameter pack of std::decay_t<Args>.... Change the tuple
construction to an explicit tuple of the decayed types so the types match
exactly; e.g. construct
std::tuple<std::decay_t<Args>...>(std::forward<Args>(args)...) when calling
awaitable_t in the initiate function to preserve reference_wrapper semantics for
any std::reference_wrapper arguments.
---
Nitpick comments:
In `@include/boost/capy/asio/detail/completion_handler.hpp`:
- Around line 85-91: The member type for CancellationSlot in
asio_coroutine_completion_handler should be a decayed type to avoid storing
references or dangling temporaries; change the member declaration
`CancellationSlot slot;` to `std::decay_t<CancellationSlot> slot;` (and include
<type_traits> if not already present) so the handler always holds a value type
regardless of how CancellationSlot was deduced.
- Around line 150-151: The member completed_immediately_t completed_immediately
is left indeterminate until await_suspend writes it; update its declaration to
value-initialize it (e.g. completed_immediately_t completed_immediately{} or =
completed_immediately_t{}), so the state machine is safe-by-default and avoids
UB if read early; keep CancellationSignal as-is and only change the
completed_immediately declaration.
- Around line 23-24: Add a file-level /* ... */ block comment immediately after
the includes in completion_handler.hpp describing the initiating/maybe/yes/no
state machine and its interactions across
asio_immediate_executor_helper::execute,
asio_coroutine_completion_handler::operator(), and awaitable_t::await_suspend;
explicitly document the state transitions (initiating -> maybe -> yes/no), the
semantics of "reset to initiating after fn() returns without calling the
handler" used to distinguish deferred vs immediate completions, and a short
example/sequence of events showing how composed operations should behave so
future maintainers can quickly understand the protocol.
- Around line 173-180: The lambda passed to std::apply in completion_handler.hpp
currently captures and takes tuple elements by value, which forces copies and
breaks for move-only args; change the lambda to accept forwarding references
(auto&&... args) and forward each argument into init_ (using std::forward for
each args) while still moving ch and args_ as intended so move-only tuple
elements are preserved; update the std::apply call that invokes init_ with
std::move(ch) and forwarded args (referring to init_, ch, and args_ in the
existing code).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d4a0eba1-ed16-434b-b706-df550cc9ea81
⛔ Files ignored due to path filters (2)
test/unit/asio.cppis excluded by!**/test/**test/unit/asio_standalone.cppis excluded by!**/test/**
📒 Files selected for processing (1)
include/boost/capy/asio/detail/completion_handler.hpp
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
include/boost/capy/asio/detail/continuation.hpp (1)
44-56:⚠️ Potential issue | 🔴 CriticalDeallocation size still mismatches allocation — heap corruption risk.
operator newat line 38 allocatesm + sizeof(alloc)bytes, butoperator deleteat line 55 still only passesntoalloc.deallocate(...). The padding (m - n) plus the trailingsizeof(alloc_t)bytes are never accounted for on deallocation, which is UB for allocators that validate or rely on the size (e.g. PMR pools). This was previously flagged and still appears unfixed.🐛 Proposed fix
- alloc.deallocate(static_cast<char*>(p), n); + alloc.deallocate(static_cast<char*>(p), m + sizeof(alloc_t));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/continuation.hpp` around lines 44 - 56, operator delete currently computes padded size m but calls alloc.deallocate(..., n), causing size mismatch; update operator delete (the function named operator delete in continuation.hpp) to pass the full allocated size (the padded m plus sizeof(alloc_t) trailing metadata) into alloc.deallocate instead of n — reproduce the same padding logic used in operator new to compute m from n, then call alloc.deallocate(static_cast<char*>(p), m + sizeof(alloc_t)) after destroying the alloc_t metadata; ensure you still reconstruct and destroy alloc_t as done via alloc_t alloc(std::move(*a)); a->~alloc_t().include/boost/capy/asio/standalone.hpp (1)
409-453: 🛠️ Refactor suggestion | 🟠 Major
asio_spawnjavadoc still doesn't satisfy the async-function guidelines.The doc blocks describe params/return but still omit: the one-sentence verb-started brief + extended description saying it's asynchronous, the
@parcompletion-conditions@lilist,@throws, cancellation semantics (supports cancellation via the Asio completion token's cancellation slot →capy::cond::canceled), a@par Example,@par Remarkson equivalence to the two-step form, and@seeplaced last. As per coding guidelines, async/awaitable function javadoc must include a verb-led brief, completion conditions, cancellation behavior,@throws, examples, and end with@see.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/standalone.hpp` around lines 409 - 453, Update the documentation for the two asio_spawn overloads (the ExecutorType overload and the Context overload) to follow the async-function guidelines: replace the current brief with a one-sentence verb-led description, add an extended sentence stating the function is asynchronous, add a `@par` "Completion conditions" section with an `@li` list describing success and error paths, document cancellation semantics (supports cancellation via the Asio completion token's cancellation slot and maps to capy::cond::canceled), add an `@throws` section for exceptions that may be propagated, include a `@par` Example showing usage, add a `@par` Remarks noting equivalence to the two-step spawn form, and finish with a `@see` entry; keep these changes for both template overloads (asio_spawn(ExecutorType, Runnable, Token) and asio_spawn(Context&, Runnable, Token)).
🧹 Nitpick comments (4)
include/boost/capy/asio/standalone.hpp (2)
164-178: Dropstaticfrom thisqueryoverload.Other
query/requirefree-function overloads in this header areconstexpr(or plain non-static) while only this one is also markedstatic. On a non-inline function template at namespace scope,staticforces internal linkage — each translation unit gets its own set of instantiations — which is both inconsistent with the surrounding overloads and a subtle source of ADL/ODR surprises when Asio'squery()customization point picks up overloads fromboost::capy.♻️ Proposed fix
-template<typename Executor, typename Allocator, int Bits> -static constexpr ::asio::execution::outstanding_work_t query( +template<typename Executor, typename Allocator, int Bits> +constexpr ::asio::execution::outstanding_work_t query( const asio_executor_adapter<Executor, Allocator, Bits> &, ::asio::execution::outstanding_work_t) noexcept🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/standalone.hpp` around lines 164 - 178, The query function template overload for asio_executor_adapter (template<typename Executor, typename Allocator, int Bits> static constexpr ::asio::execution::outstanding_work_t query(...)) is declared static, giving it internal linkage and causing ADL/ODR surprises; remove the static specifier so the free-function query for asio_executor_adapter has external linkage and matches the other non-static constexpr require/query overloads (locate the query overload taking asio_executor_adapter<Executor, Allocator, Bits> and drop the leading static keyword).
483-521: Rounding usessizeof(std::max_align_t)but should usealignof.Alignment padding conceptually uses
alignof(T), notsizeof(T). They coincide forstd::max_align_ton mainstream ABIs (both typically 16) so this works today, but it's semantically off and easy to miscopy. Since only the trailingallocator_typeneeds alignment, rounding toalignof(allocator_type)(ascontinuation_handle_promise_base_does) is both tighter and clearer.♻️ Suggested tidy-up
- // round n up to max_align - if (const auto d = n % sizeof(std::max_align_t); d > 0u) - n += (sizeof(std::max_align_t) - d); + // round n up so the trailing allocator_type slot is aligned + if (const auto d = n % alignof(allocator_type); d > 0u) + n += (alignof(allocator_type) - d); @@ - if (const auto d = n % sizeof(std::max_align_t); d > 0u) - n += (sizeof(std::max_align_t) - d); + if (const auto d = n % alignof(allocator_type); d > 0u) + n += (alignof(allocator_type) - d);Note: the allocation/deallocation sizes here do match (both use the same padded
n + sizeof(allocator_type)), unlikecontinuation_handle_promise_base_::operator delete— good catch keeping those symmetric.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/standalone.hpp` around lines 483 - 521, The padding in boost_asio_standalone_promise_type_allocator_base::operator new and ::operator delete uses sizeof(std::max_align_t) but should use the allocator alignment; change the rounding divisor from sizeof(std::max_align_t) to alignof(allocator_type) (where allocator_type is the rebind_alloc<char> alias used in those functions) so both allocation and deallocation round n to alignof(allocator_type) before computing the stored-allocator placement and deallocation size, preserving the existing n + sizeof(allocator_type) semantics.include/boost/capy/asio/detail/completion_handler.hpp (1)
193-201: Move constructor doesn't carrysignal,completed_immediately, orstopper.
awaitable_t(awaitable_t&& rhs)only movesinit_,args_, andresult_.signalis default-initialized (freshcancellation_signal),completed_immediatelyis left with an indeterminate value, andstopperis a fresh empty optional. This is fine in practice becauseawait_suspendinitializes all three before use and the awaitable is normally consumed right afterinitiate()returns — but any code that introspects or mutates the signal/slot between construction and suspension would silently lose state across a move. Worth either value-initializingcompleted_immediatelyin-class (= completed_immediately_t::no;) and using= defaultfor the move ctor, or explicitly documenting that the awaitable must not be used before being awaited.♻️ Suggested tidy-up
- CancellationSignal signal; - completed_immediately_t completed_immediately; + CancellationSignal signal{}; + completed_immediately_t completed_immediately = completed_immediately_t::no; @@ - awaitable_t(awaitable_t && rhs) noexcept - : init_(std::move(rhs.init_)) - , args_(std::move(rhs.args_)) - , result_(std::move(rhs.result_)) {} + awaitable_t(awaitable_t && rhs) = default;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/completion_handler.hpp` around lines 193 - 201, The move constructor awaitable_t(awaitable_t&& rhs) currently only moves init_, args_, and result_, leaving signal, completed_immediately, and stopper in new/indeterminate states; fix by either (A) value-initialize completed_immediately in-class (e.g., completed_immediately = completed_immediately_t::no) and revert to the compiler-generated move ctor (= default) so all members are moved/copied correctly, or (B) explicitly move/copy the remaining members inside the custom move ctor (move rhs.signal into signal, move rhs.stopper into stopper, and copy rhs.completed_immediately into completed_immediately) so awaitable_t preserves these fields across moves (ensure consistency with await_suspend and initiate()).include/boost/capy/asio/detail/continuation.hpp (1)
89-107: Confirmyield_valuetakingFunction&is intentional.
yield_valuebinds the argument by lvalue reference and thenstd::moves it intoyielder. That works becausemake_continuation_helperco_yields a local variable (func) — but if any other caller everco_yields a prvalue/xvalue (e.g.co_yield std::move(func_expr);), overload resolution will fail since a non-const lvalue reference can't bind to a temporary. ConsiderFunction&&with a forwarding reference or at leastconst Function&/Function&&overloads to avoid the footgun if this promise type is later reused.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/asio/detail/continuation.hpp` around lines 89 - 107, yield_value currently takes Function& which prevents binding temporaries; change its signature to template<typename Function> auto yield_value(Function&& func) and have yielder hold a decayed copy (e.g. using MemberT = std::decay_t<Function>; MemberT func;), then construct yielder with std::forward<Function>(func) and use std::move only when invoking the stored func; this preserves forwarding for prvalues/xvalues and avoids the lvalue-reference footgun (refer to yield_value, yielder and callers such as make_continuation_helper).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/boost/capy/asio/standalone.hpp`:
- Around line 628-666: The catch handler in
completion_tuple_for_io_runnable<Runnable>::await_resume() constructs a default
value with type() which requires the await_resume() return type to be
default-constructible even though IoRunnable doesn't mandate that; change the
code to value-initialize using type{} (or otherwise value-initialize the
fallback) so non-default-constructible return types still compile, or
alternatively update the IoRunnable concept to require default-constructibility
of decltype(std::declval<Runnable&>().await_resume()) if you prefer enforcing
the requirement — locate the await_resume() body and replace the problematic
type() construction with value-initialization (type{}) or add the constraint to
IoRunnable accordingly.
---
Duplicate comments:
In `@include/boost/capy/asio/detail/continuation.hpp`:
- Around line 44-56: operator delete currently computes padded size m but calls
alloc.deallocate(..., n), causing size mismatch; update operator delete (the
function named operator delete in continuation.hpp) to pass the full allocated
size (the padded m plus sizeof(alloc_t) trailing metadata) into alloc.deallocate
instead of n — reproduce the same padding logic used in operator new to compute
m from n, then call alloc.deallocate(static_cast<char*>(p), m + sizeof(alloc_t))
after destroying the alloc_t metadata; ensure you still reconstruct and destroy
alloc_t as done via alloc_t alloc(std::move(*a)); a->~alloc_t().
In `@include/boost/capy/asio/standalone.hpp`:
- Around line 409-453: Update the documentation for the two asio_spawn overloads
(the ExecutorType overload and the Context overload) to follow the
async-function guidelines: replace the current brief with a one-sentence
verb-led description, add an extended sentence stating the function is
asynchronous, add a `@par` "Completion conditions" section with an `@li` list
describing success and error paths, document cancellation semantics (supports
cancellation via the Asio completion token's cancellation slot and maps to
capy::cond::canceled), add an `@throws` section for exceptions that may be
propagated, include a `@par` Example showing usage, add a `@par` Remarks noting
equivalence to the two-step spawn form, and finish with a `@see` entry; keep these
changes for both template overloads (asio_spawn(ExecutorType, Runnable, Token)
and asio_spawn(Context&, Runnable, Token)).
---
Nitpick comments:
In `@include/boost/capy/asio/detail/completion_handler.hpp`:
- Around line 193-201: The move constructor awaitable_t(awaitable_t&& rhs)
currently only moves init_, args_, and result_, leaving signal,
completed_immediately, and stopper in new/indeterminate states; fix by either
(A) value-initialize completed_immediately in-class (e.g., completed_immediately
= completed_immediately_t::no) and revert to the compiler-generated move ctor (=
default) so all members are moved/copied correctly, or (B) explicitly move/copy
the remaining members inside the custom move ctor (move rhs.signal into signal,
move rhs.stopper into stopper, and copy rhs.completed_immediately into
completed_immediately) so awaitable_t preserves these fields across moves
(ensure consistency with await_suspend and initiate()).
In `@include/boost/capy/asio/detail/continuation.hpp`:
- Around line 89-107: yield_value currently takes Function& which prevents
binding temporaries; change its signature to template<typename Function> auto
yield_value(Function&& func) and have yielder hold a decayed copy (e.g. using
MemberT = std::decay_t<Function>; MemberT func;), then construct yielder with
std::forward<Function>(func) and use std::move only when invoking the stored
func; this preserves forwarding for prvalues/xvalues and avoids the
lvalue-reference footgun (refer to yield_value, yielder and callers such as
make_continuation_helper).
In `@include/boost/capy/asio/standalone.hpp`:
- Around line 164-178: The query function template overload for
asio_executor_adapter (template<typename Executor, typename Allocator, int Bits>
static constexpr ::asio::execution::outstanding_work_t query(...)) is declared
static, giving it internal linkage and causing ADL/ODR surprises; remove the
static specifier so the free-function query for asio_executor_adapter has
external linkage and matches the other non-static constexpr require/query
overloads (locate the query overload taking asio_executor_adapter<Executor,
Allocator, Bits> and drop the leading static keyword).
- Around line 483-521: The padding in
boost_asio_standalone_promise_type_allocator_base::operator new and ::operator
delete uses sizeof(std::max_align_t) but should use the allocator alignment;
change the rounding divisor from sizeof(std::max_align_t) to
alignof(allocator_type) (where allocator_type is the rebind_alloc<char> alias
used in those functions) so both allocation and deallocation round n to
alignof(allocator_type) before computing the stored-allocator placement and
deallocation size, preserving the existing n + sizeof(allocator_type) semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eb4c4af6-8e34-4444-b50e-d63989a2244b
⛔ Files ignored due to path filters (3)
doc/modules/ROOT/pages/4.coroutines/4i.asio-integration.adocis excluded by!**/doc/**test/unit/asio.cppis excluded by!**/test/**test/unit/asio_standalone.cppis excluded by!**/test/**
📒 Files selected for processing (5)
include/boost/capy/asio/boost.hppinclude/boost/capy/asio/detail/completion_handler.hppinclude/boost/capy/asio/detail/continuation.hppinclude/boost/capy/asio/detail/fwd.hppinclude/boost/capy/asio/standalone.hpp
✅ Files skipped from review due to trivial changes (1)
- include/boost/capy/asio/detail/fwd.hpp
🚧 Files skipped from review as they are similar to previous changes (1)
- include/boost/capy/asio/boost.hpp
| completion_tuple_for_io_runnable<Runnable> await_resume() | ||
| { | ||
| if (cancel_slot.is_connected()) | ||
| cancel_slot.clear(); | ||
|
|
||
| using type = decltype(r.await_resume()); | ||
| if constexpr (!noexcept(r.await_resume())) | ||
| { | ||
| if constexpr (std::is_void_v<type>) | ||
| try | ||
| { | ||
| r.await_resume(); | ||
| return {std::exception_ptr()}; | ||
| } | ||
| catch (...) | ||
| { | ||
| return std::current_exception(); | ||
| } | ||
| else | ||
| try | ||
| { | ||
| return {std::exception_ptr(), r.await_resume()}; | ||
| } | ||
| catch (...) | ||
| { | ||
| return {std::current_exception(), type()}; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| if constexpr (std::is_void_v<type>) | ||
| { | ||
| r.await_resume(); | ||
| return {}; | ||
| } | ||
| else | ||
| return {r.await_resume()}; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the IoRunnable concept / io_result payload types
# already require default-constructibility of await_resume's return type.
fd -e hpp -e h | xargs rg -n -C3 'concept\s+IoRunnable|io_runnable|completion_tuple_for_io_runnable'Repository: cppalliance/capy
Length of output: 13748
🏁 Script executed:
cat -n include/boost/capy/concept/io_runnable.hppRepository: cppalliance/capy
Length of output: 5043
🏁 Script executed:
sed -n '628,666p' include/boost/capy/asio/standalone.hppRepository: cppalliance/capy
Length of output: 1050
🏁 Script executed:
cat -n include/boost/capy/asio/detail/completion_traits.hppRepository: cppalliance/capy
Length of output: 2297
🏁 Script executed:
cat -n include/boost/capy/concept/io_awaitable.hpp | head -100Repository: cppalliance/capy
Length of output: 3952
🏁 Script executed:
fd -e hpp -e h -e cpp | xargs rg -l 'io_result' | head -5Repository: cppalliance/capy
Length of output: 175
🏁 Script executed:
find . -name "io_result.hpp" -type fRepository: cppalliance/capy
Length of output: 94
🏁 Script executed:
cat -n include/boost/capy/io_result.hppRepository: cppalliance/capy
Length of output: 4251
type() requires default-constructibility of await_resume() return type, but IoRunnable does not constrain this.
At line 653, the catch handler constructs a completion tuple using type(). This requires the return type to be default-constructible. The IoRunnable concept does not explicitly require this, so code satisfying IoRunnable with a non-default-constructible await_resume() return type will fail to compile with an opaque diagnostic deep in the tuple template instantiation.
Either constrain IoRunnable to require default-constructibility of the return type, or change the code to use type{} for value-initialization (consistent with the io_result pattern in this codebase).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@include/boost/capy/asio/standalone.hpp` around lines 628 - 666, The catch
handler in completion_tuple_for_io_runnable<Runnable>::await_resume() constructs
a default value with type() which requires the await_resume() return type to be
default-constructible even though IoRunnable doesn't mandate that; change the
code to value-initialize using type{} (or otherwise value-initialize the
fallback) so non-default-constructible return types still compile, or
alternatively update the IoRunnable concept to require default-constructibility
of decltype(std::declval<Runnable&>().await_resume()) if you prefer enforcing
the requirement — locate the await_resume() body and replace the problematic
type() construction with value-initialization (type{}) or add the constraint to
IoRunnable accordingly.
3 participants
Summary by CodeRabbit
New Features
Bug Fixes / Stability