Add range-based connect composed operation#233
Add range-based connect composed operation#233mvandeberg merged 1 commit intocppalliance:developfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds coroutine-based Changes
Sequence Diagram(s)(Skipped — changes are focused on a new internal coroutine API; no multi-component sequential flow diagram provided.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
include/boost/corosio/connect.hpp (3)
180-181: Drop the no-opstatic_casts on the condition arguments.
last_ecandepare already lvalues of the expected types; they bind tostd::error_code const&/endpoint_type const&implicitly. The casts add noise without changing overload resolution (the predicate concept at Lines 70-73 / 81-84 only requires invocability with those const refs).♻️ Proposed simplification
- if (!cond(static_cast<std::error_code const&>(last_ec), - static_cast<endpoint_type const&>(ep))) - continue; + if (!cond(last_ec, ep)) + continue;Apply the same at Lines 273–274.
Also applies to: 273-274
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/connect.hpp` around lines 180 - 181, Remove the redundant static_casts when invoking the predicate `cond` with `last_ec` and `ep` in `connect.hpp`: change the call sites where `cond(static_cast<std::error_code const&>(last_ec), static_cast<endpoint_type const&>(ep))` (and the analogous call later) to pass `last_ec` and `ep` directly, since they already bind to `std::error_code const&` and `endpoint_type const&`; update both occurrences (the one near the first `if` using `cond` and the analogous occurrence around lines 273–274) to eliminate the no-op casts.
66-86: Consider a forwarding-referenceRange&&/Iterpair instead of by-value.Taking
Range endpointsby value works and — as the comment at Lines 25-42 notes — extends the lifetime of temporaries likeresolver_results. But it also silently copies lvalue containers (e.g. a user'sstd::vector<endpoint>), which can be surprising and pointlessly expensive. A forwarding reference +std::views::all/ explicit move gets you both behaviors without the copy:template<class Socket, std::ranges::input_range Range, class ConnectCondition> requires /* ... */ capy::task<capy::io_result<typename Socket::endpoint_type>> connect(Socket& s, Range&& endpoints, ConnectCondition cond);Callers passing rvalues still get the frame-extended lifetime (via the forwarded value captured into the coroutine frame); callers passing lvalues avoid the deep copy. If by-value is a deliberate parity choice with Asio's
async_connect, the current comment could call that out explicitly.Non-blocking — flag for consideration.
Also applies to: 129-138, 226-238
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/connect.hpp` around lines 66 - 86, The two connect overloads take Range endpoints and Iter by-value which causes unnecessary copies of lvalue containers; change them to forwarding references (e.g., template parameters Range&& endpoints and Iter&& begin/Iter&& end or an Iter pair wrapper) and use std::views::all / std::ranges::borrowed_range or explicit std::move when capturing rvalues so temporaries still get coroutine-frame lifetime extension while lvalues avoid deep copies; update the declarations for the connect(Socket& s, Range&& endpoints, ConnectCondition cond) and the connect(Socket& s, Iter begin, Iter end, ConnectCondition cond) overloads (and the analogous declarations at lines noted) to accept forwarding references and adapt the implementation to capture/move the forwarded ranges/iterators into the coroutine frame.
88-128: Bring the composed-connect doxygen into line with the repo's doc guidelines.The docstrings are thorough prose, but a few required elements from the project's doc template are missing on a public header in a non-detail namespace:
- No
@libulleted Completion conditions list (success, error, cancellation, empty/exhausted-range).- No
@tparam Socketstating the required operations.Range/Iter/ConnectConditionalready carry concept/requiresconstraints and per guidelines do not need@tparam, butSockethas no concept and implicitly requiresendpoint_type,is_open(),close(), and an awaitableconnect(endpoint_type)— worth spelling out.- No concurrency / overlap note (e.g. "this composed operation is implemented in terms of
Socket::connect; only one connect may be in flight, and no other connect-initiating calls may be issued concurrently ons").- No
@seeat the end (e.g.Socket::connect, Asio'sasync_connectfor the semantics you're mirroring).- The iterator overload at Lines 204–225 omits the cancellation paragraph present on the range overload, even though the implementation at Line 285–286 does return
endoncapy::cond::canceled.As per coding guidelines: "Completion conditions — Bulleted
@lilist of conditions under which the operation completes and the coroutine resumes", "@tparam documentation — For non-variadic template parameters, state the concept requirement", "Concurrency and overlap — State which operations may be simultaneously in flight ... all calls to the stream must be made from the same implicit or explicit serialization context", and "@see cross-references — Always place last".Also applies to: 140-160, 204-225, 240-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/connect.hpp` around lines 88 - 128, Update the corosio::connect doxygen: add a bulleted "@li" Completion conditions list covering success (connected endpoint), failure after all attempts (last error + default endpoint), cancellation (immediately returns capy::cond::canceled and does not try further endpoints), and empty-range (std::errc::no_such_device_or_address); add an "@tparam Socket" describing required members/types (Socket::endpoint_type, is_open(), close(), and a connect(endpoint_type) returning an awaitable); add a concurrency/overlap note stating the composed operation is implemented in terms of Socket::connect so only one connect may be in flight on the same socket and no other connect-initiating calls may be issued concurrently on s; add a "@see" pointing to Socket::connect and asio::async_connect; and mirror the range-overload cancellation paragraph in the iterator overload (document that capy::cond::canceled causes the operation to complete immediately and the iterator-returning overload returns end on cancellation).
🤖 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/corosio/connect.hpp`:
- Around line 184-185: Document that calling s.close() before each connect
attempt discards any socket options the caller previously set: add a one-line
`@note` to the overload docs in connect.hpp (the overloads that perform the
candidate-loop using s.close()) stating that user-applied socket options (e.g.
SO_REUSEADDR, IPV6_V6ONLY, TCP_NODELAY) are discarded between attempts and that
callers should set options via the ConnectCondition hook or apply options after
the composed operation completes; reference the s.close() behavior and the
ConnectCondition hook in the note so users know where to apply options.
---
Nitpick comments:
In `@include/boost/corosio/connect.hpp`:
- Around line 180-181: Remove the redundant static_casts when invoking the
predicate `cond` with `last_ec` and `ep` in `connect.hpp`: change the call sites
where `cond(static_cast<std::error_code const&>(last_ec),
static_cast<endpoint_type const&>(ep))` (and the analogous call later) to pass
`last_ec` and `ep` directly, since they already bind to `std::error_code const&`
and `endpoint_type const&`; update both occurrences (the one near the first `if`
using `cond` and the analogous occurrence around lines 273–274) to eliminate the
no-op casts.
- Around line 66-86: The two connect overloads take Range endpoints and Iter
by-value which causes unnecessary copies of lvalue containers; change them to
forwarding references (e.g., template parameters Range&& endpoints and Iter&&
begin/Iter&& end or an Iter pair wrapper) and use std::views::all /
std::ranges::borrowed_range or explicit std::move when capturing rvalues so
temporaries still get coroutine-frame lifetime extension while lvalues avoid
deep copies; update the declarations for the connect(Socket& s, Range&&
endpoints, ConnectCondition cond) and the connect(Socket& s, Iter begin, Iter
end, ConnectCondition cond) overloads (and the analogous declarations at lines
noted) to accept forwarding references and adapt the implementation to
capture/move the forwarded ranges/iterators into the coroutine frame.
- Around line 88-128: Update the corosio::connect doxygen: add a bulleted "@li"
Completion conditions list covering success (connected endpoint), failure after
all attempts (last error + default endpoint), cancellation (immediately returns
capy::cond::canceled and does not try further endpoints), and empty-range
(std::errc::no_such_device_or_address); add an "@tparam Socket" describing
required members/types (Socket::endpoint_type, is_open(), close(), and a
connect(endpoint_type) returning an awaitable); add a concurrency/overlap note
stating the composed operation is implemented in terms of Socket::connect so
only one connect may be in flight on the same socket and no other
connect-initiating calls may be issued concurrently on s; add a "@see" pointing
to Socket::connect and asio::async_connect; and mirror the range-overload
cancellation paragraph in the iterator overload (document that
capy::cond::canceled causes the operation to complete immediately and the
iterator-returning overload returns end on cancellation).
🪄 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: dd550d39-7311-452e-9e0c-1961237aecda
⛔ Files ignored due to path filters (2)
doc/modules/ROOT/pages/4.guide/4d.sockets.adocis excluded by!**/doc/**test/unit/connect.cppis excluded by!**/test/**
📒 Files selected for processing (3)
include/boost/corosio/connect.hppinclude/boost/corosio/local_stream_socket.hppinclude/boost/corosio/tcp_socket.hpp
|
An automated preview of the documentation is available at https://233.corosio.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 16:42:32 UTC |
|
GCOVR code coverage report https://233.corosio.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-04-17 16:51:19 UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #233 +/- ##
========================================
Coverage 77.71% 77.71%
========================================
Files 96 96
Lines 7298 7298
Branches 1787 1787
========================================
Hits 5672 5672
Misses 1108 1108
Partials 518 518
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Free functions in boost/corosio/connect.hpp that try each endpoint in a range (or iterator pair) until one connects, with optional connect-condition predicate. Mirrors Boost.Asio's async_connect semantics adapted to corosio's coroutine model.
mvandeberg
force-pushed
the
pr/124-range-connect
branch
from
41fc681 to
4cb9a08
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
2 participants
Closes #124
Free functions in boost/corosio/connect.hpp that try each endpoint in a range (or iterator pair) until one connects, with optional connect-condition predicate. Mirrors Boost.Asio's async_connect semantics adapted to corosio's coroutine model.
Summary by CodeRabbit