Document move preconditions and awaitable lifetime requirements acros…#152
Conversation
…s public API Add @pre conditions to move constructors and move assignment operators for all public I/O types: tcp_socket, tcp_acceptor, timer, signal_set, resolver, and their native variants. Document awaitable lifetime requirements and buffer validity for read_some/write_some.
📝 WalkthroughWalkthroughThis pull request adds comprehensive documentation updates across Boost Corosio header files, clarifying lifetime requirements and preconditions for awaitables and move operations. All changes are documentation-only with no functional code modifications. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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://152.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-02-18 18:35:36 UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #152 +/- ##
===========================================
+ Coverage 82.04% 82.21% +0.17%
===========================================
Files 70 70
Lines 5876 5876
===========================================
+ Hits 4821 4831 +10
+ Misses 1055 1045 -10
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
include/boost/corosio/native/native_signal_set.hpp (1)
139-147:⚠️ Potential issue | 🟡 MinorLifetime note is placed after
@return, inconsistent with other files.In Doxygen, text after
@returnis parsed as part of the return description. Here, "This signal set must outlive the returned awaitable." (line 146) appears after@return(line 144), so it will render as part of the return value documentation rather than as a general requirement.In all the other files (
io_signal_set.hpp,io_timer.hpp,io_read_stream.hpp, etc.), the lifetime note is placed before@return. Move it before@returnfor consistency and correct Doxygen rendering.Suggested diff
/** Wait for a signal to be delivered. Calls the backend implementation directly, bypassing virtual dispatch. Otherwise identical to `@ref` signal_set::wait. + This signal set must outlive the returned awaitable. + `@return` An awaitable yielding `io_result<int>`. - - This signal set must outlive the returned awaitable. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/native_signal_set.hpp` around lines 139 - 147, The Doxygen lifetime sentence for native_signal_set::wait is placed after the `@return` tag and will be rendered as part of the return description; move the line "This signal set must outlive the returned awaitable." so it appears before the `@return` block in the documentation comment for the wait function (the comment in include/boost/corosio/native/native_signal_set.hpp describing the backend wait awaitable) to match other files like io_signal_set.hpp and ensure correct Doxygen rendering.
🧹 Nitpick comments (8)
include/boost/corosio/io/io_write_stream.hpp (1)
115-119: Same asio_read_stream: consider moving the buffer validity note into@param.The buffer lifetime requirement on lines 116-117 would be better placed under
@param buffers(line 119), consistent with the coding guideline for documenting ownership/lifetime semantics on the@paramline.Suggested diff
This stream must outlive the returned awaitable. The memory - referenced by `@p` buffers must remain valid until the operation - completes. - `@param` buffers The buffer sequence containing data to write. + `@param` buffers The buffer sequence containing data to write. + The caller retains ownership and must guarantee the + referenced memory remains valid until the operation completes.As per coding guidelines: "@param documentation for each parameter — Including ownership/lifetime semantics; for buffers state that the caller retains ownership and must guarantee validity until the operation completes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/io/io_write_stream.hpp` around lines 115 - 119, Move the standalone buffer-lifetime sentence currently above the parameter list into the `@param` for buffers in the io_write_stream documentation: remove the two-line note ("This stream must outlive..." / "The memory referenced by `@p` buffers must remain valid until the operation completes.") and append its ownership/lifetime clause to the existing `@param` buffers description so it reads that the caller retains ownership and must guarantee buffers remain valid until the operation completes (apply this change in the io_write_stream comment block referencing the buffers parameter).include/boost/corosio/io/io_read_stream.hpp (1)
115-119: Consider moving the buffer validity note into the@paramsection.Per the coding guidelines, buffer ownership/lifetime semantics should be documented on the
@paramline. Currently the buffer validity requirement is in the extended description (line 116-117), while@param buffers(line 119) only says "The buffer sequence to read data into." Moving the lifetime note there would align better with the guideline: "for buffers state that the caller retains ownership and must guarantee validity until the operation completes."Suggested diff
This stream must outlive the returned awaitable. The memory - referenced by `@p` buffers must remain valid until the operation - completes. - `@param` buffers The buffer sequence to read data into. + `@param` buffers The buffer sequence to read data into. The caller + retains ownership and must guarantee the referenced memory + remains valid until the operation completes.As per coding guidelines: "@param documentation for each parameter — Including ownership/lifetime semantics; for buffers state that the caller retains ownership and must guarantee validity until the operation completes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/io/io_read_stream.hpp` around lines 115 - 119, Move the lifetime/ownership sentence about buffers from the extended description into the `@param` for buffers so the `@param` line documents ownership semantics (caller retains ownership and must guarantee the memory referenced by buffers remains valid until the operation completes); update the `@param` "buffers" description in include/boost/corosio/io/io_read_stream.hpp to include that exact lifetime requirement and remove the duplicate sentence from the main paragraph.include/boost/corosio/native/native_tcp_acceptor.hpp (1)
160-162: Lifetime note foraccept()uses freestanding text, unliketcp_acceptor::accept()which places it under@par Preconditions.For consistency with the base class documentation in
tcp_acceptor.hpp(lines 237-242), consider placing this under@preor@par Preconditions:Suggested fix
`@throws` std::logic_error if the acceptor is not listening. - - Both this acceptor and `@p` peer must outlive the returned - awaitable. + + `@pre` Both this acceptor and `@p` peer must outlive the returned + awaitable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/native_tcp_acceptor.hpp` around lines 160 - 162, Move the freestanding lifetime note in native_tcp_acceptor::accept into the formal precondition section to match tcp_acceptor::accept; specifically, add the sentence "Both this acceptor and `@p` peer must outlive the returned awaitable." under an `@pre` or `@par` Preconditions block in the Doxygen comment for native_tcp_acceptor::accept (matching the style used in tcp_acceptor::accept) so the lifetime requirement appears consistently within the preconditions section.include/boost/corosio/native/native_tcp_socket.hpp (2)
245-248: Buffer lifetime notes should also state caller ownership, and use a Doxygen tag.Two items:
Ownership: The coding guidelines require buffer documentation to state that "the caller retains ownership and must guarantee validity until the operation completes." The current text covers validity but not ownership. Consider: "The caller retains ownership of the buffers and must ensure the referenced memory remains valid until the operation completes."
Formatting: Same freestanding-text issue noted in other files — place under
@preor@notefor proper Doxygen rendering.Suggested fix (read_some example; apply same pattern to write_some)
`@return` An awaitable yielding `(error_code, std::size_t)`. - This socket must outlive the returned awaitable. The memory - referenced by `@p` buffers must remain valid until the operation - completes. + `@pre` This socket must outlive the returned awaitable. + `@pre` The caller retains ownership of the buffers; the + referenced memory must remain valid until the operation + completes.As per coding guidelines: "for buffers state that the caller retains ownership and must guarantee validity until the operation completes."
Also applies to: 264-267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/native_tcp_socket.hpp` around lines 245 - 248, Update the buffer lifetime documentation for native_tcp_socket::read_some and native_tcp_socket::write_some to state caller ownership and move the text under a Doxygen tag (use `@pre` or `@note`); specifically, add a sentence like "The caller retains ownership of the buffers and must ensure the referenced memory remains valid until the operation completes." and place the combined validity+ownership sentence under the chosen Doxygen tag for both the read_some and write_some comments.
285-286: Same freestanding-text formatting concern for connect() lifetime note.Consider
@pre This socket must outlive the returned awaitable.for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/native_tcp_socket.hpp` around lines 285 - 286, Update the freestanding lifetime note for connect() to use Doxygen precondition syntax: replace the plain sentence "This socket must outlive the returned awaitable." with "@pre This socket must outlive the returned awaitable." in the documentation for the connect() method in native_tcp_socket.hpp so the lifetime requirement is consistent with other comments; make sure the annotation is placed in the connect() docblock adjacent to the function declaration/definition.include/boost/corosio/native/native_timer.hpp (1)
149-150: Same freestanding-text issue as noted inresolver.hpp.The lifetime note "This timer must outlive the returned awaitable." should be placed under
@preor@notefor proper Doxygen rendering, consistent with the@pretags used for the move operations in this same file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/native_timer.hpp` around lines 149 - 150, The stray sentence "This timer must outlive the returned awaitable." in native_timer.hpp should be moved into the function/class Doxygen block as a formal tag (e.g., `@pre` or `@note`) so it renders correctly; find the Doxygen comment for the function that returns the awaitable (look for references to the timer and the returned awaitable in native_timer.hpp) and add the sentence under `@pre` (or `@note`) consistent with the existing `@pre` usage for move operations in the same file.include/boost/corosio/resolver.hpp (2)
316-326: Move assignment doc says "Destroys the current implementation" but code does not callclose().The move assignment at line 332 only does
h_ = std::move(other.h_), relying on the handle's destructor semantics. In contrast,tcp_acceptor::operator=andtcp_socket::operator=explicitly callclose()before the move (seeinclude/boost/corosio/tcp_acceptor.hpplines 160-168 andinclude/boost/corosio/tcp_socket.hpplines 230-238). If the handle assignment internally releases the old resource, "Destroys" is technically accurate but misleading about the mechanism. Consider aligning the wording with the actual behavior, e.g., "Releases the current handle and transfers ownership from@pother."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/resolver.hpp` around lines 316 - 326, The doc for resolver's move assignment (resolver::operator=) is misleading because the implementation does not call close() but instead assigns the handle (h_ = std::move(other.h_)); update the documentation to reflect the actual behavior by replacing "Destroys the current implementation and transfers ownership" with wording like "Releases the current handle and transfers ownership from `@p` other" (or similar), and keep the existing preconditions; reference resolver::operator= and member h_ in the comment to clarify intent rather than implying an explicit close() call.
343-344: Lifetime notes should use a Doxygen tag for proper rendering.The freestanding text "This resolver must outlive the returned awaitable." after
@returnwill be parsed by Doxygen as a continuation of the return description or as detached body text, which may not render as intended. Consider placing these under@preor@notefor consistent structure with the move-constructor preconditions in the same file. This pattern also applies to the same-style notes in other files in this PR (native_tcp_socket, native_tcp_acceptor, native_timer).Suggested fix (applied consistently to all four overloads)
- This resolver must outlive the returned awaitable. + `@pre` This resolver must outlive the returned awaitable.Also applies to: 368-369, 389-390, 414-415
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/resolver.hpp` around lines 343 - 344, The freestanding lifetime sentence after the `@return` tag should be converted into a proper Doxygen tag (e.g., `@note` or `@pre`) so it renders correctly; locate the awaitable-returning overloads in resolver.hpp where the text "This resolver must outlive the returned awaitable." appears (and the similar occurrences at the other listed locations) and move that sentence under a new `@note` or `@pre` entry associated with the function’s documentation (keeping the same wording), and apply the same change to the other three overloads mentioned (and the equivalent notes in native_tcp_socket, native_tcp_acceptor, native_timer) for consistent documentation structure.
🤖 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/native/native_resolver.hpp`:
- Around line 169-175: Update the move assignment documentation in
include/boost/corosio/native/native_resolver.hpp to use the same parameter
naming as the rest of the PR: replace occurrences of the phrase "the source"
with the documented parameter name "@p other" in the Move assign comment (the
section describing preconditions for move assignment/operator=) so it matches
the move constructor and other files' terminology.
---
Outside diff comments:
In `@include/boost/corosio/native/native_signal_set.hpp`:
- Around line 139-147: The Doxygen lifetime sentence for native_signal_set::wait
is placed after the `@return` tag and will be rendered as part of the return
description; move the line "This signal set must outlive the returned
awaitable." so it appears before the `@return` block in the documentation comment
for the wait function (the comment in
include/boost/corosio/native/native_signal_set.hpp describing the backend wait
awaitable) to match other files like io_signal_set.hpp and ensure correct
Doxygen rendering.
---
Nitpick comments:
In `@include/boost/corosio/io/io_read_stream.hpp`:
- Around line 115-119: Move the lifetime/ownership sentence about buffers from
the extended description into the `@param` for buffers so the `@param` line
documents ownership semantics (caller retains ownership and must guarantee the
memory referenced by buffers remains valid until the operation completes);
update the `@param` "buffers" description in
include/boost/corosio/io/io_read_stream.hpp to include that exact lifetime
requirement and remove the duplicate sentence from the main paragraph.
In `@include/boost/corosio/io/io_write_stream.hpp`:
- Around line 115-119: Move the standalone buffer-lifetime sentence currently
above the parameter list into the `@param` for buffers in the io_write_stream
documentation: remove the two-line note ("This stream must outlive..." / "The
memory referenced by `@p` buffers must remain valid until the operation
completes.") and append its ownership/lifetime clause to the existing `@param`
buffers description so it reads that the caller retains ownership and must
guarantee buffers remain valid until the operation completes (apply this change
in the io_write_stream comment block referencing the buffers parameter).
In `@include/boost/corosio/native/native_tcp_acceptor.hpp`:
- Around line 160-162: Move the freestanding lifetime note in
native_tcp_acceptor::accept into the formal precondition section to match
tcp_acceptor::accept; specifically, add the sentence "Both this acceptor and `@p`
peer must outlive the returned awaitable." under an `@pre` or `@par` Preconditions
block in the Doxygen comment for native_tcp_acceptor::accept (matching the style
used in tcp_acceptor::accept) so the lifetime requirement appears consistently
within the preconditions section.
In `@include/boost/corosio/native/native_tcp_socket.hpp`:
- Around line 245-248: Update the buffer lifetime documentation for
native_tcp_socket::read_some and native_tcp_socket::write_some to state caller
ownership and move the text under a Doxygen tag (use `@pre` or `@note`);
specifically, add a sentence like "The caller retains ownership of the buffers
and must ensure the referenced memory remains valid until the operation
completes." and place the combined validity+ownership sentence under the chosen
Doxygen tag for both the read_some and write_some comments.
- Around line 285-286: Update the freestanding lifetime note for connect() to
use Doxygen precondition syntax: replace the plain sentence "This socket must
outlive the returned awaitable." with "@pre This socket must outlive the
returned awaitable." in the documentation for the connect() method in
native_tcp_socket.hpp so the lifetime requirement is consistent with other
comments; make sure the annotation is placed in the connect() docblock adjacent
to the function declaration/definition.
In `@include/boost/corosio/native/native_timer.hpp`:
- Around line 149-150: The stray sentence "This timer must outlive the returned
awaitable." in native_timer.hpp should be moved into the function/class Doxygen
block as a formal tag (e.g., `@pre` or `@note`) so it renders correctly; find the
Doxygen comment for the function that returns the awaitable (look for references
to the timer and the returned awaitable in native_timer.hpp) and add the
sentence under `@pre` (or `@note`) consistent with the existing `@pre` usage for move
operations in the same file.
In `@include/boost/corosio/resolver.hpp`:
- Around line 316-326: The doc for resolver's move assignment
(resolver::operator=) is misleading because the implementation does not call
close() but instead assigns the handle (h_ = std::move(other.h_)); update the
documentation to reflect the actual behavior by replacing "Destroys the current
implementation and transfers ownership" with wording like "Releases the current
handle and transfers ownership from `@p` other" (or similar), and keep the
existing preconditions; reference resolver::operator= and member h_ in the
comment to clarify intent rather than implying an explicit close() call.
- Around line 343-344: The freestanding lifetime sentence after the `@return` tag
should be converted into a proper Doxygen tag (e.g., `@note` or `@pre`) so it
renders correctly; locate the awaitable-returning overloads in resolver.hpp
where the text "This resolver must outlive the returned awaitable." appears (and
the similar occurrences at the other listed locations) and move that sentence
under a new `@note` or `@pre` entry associated with the function’s documentation
(keeping the same wording), and apply the same change to the other three
overloads mentioned (and the equivalent notes in native_tcp_socket,
native_tcp_acceptor, native_timer) for consistent documentation structure.
| /** Move assign. | ||
|
|
||
| @pre No awaitables returned by either `*this` or the source's | ||
| `resolve` methods exist. | ||
| @pre The execution context associated with the source must | ||
| outlive this resolver. | ||
| */ |
There was a problem hiding this comment.
Inconsistent terminology: "the source" vs @p other.
The move assignment doc uses "the source" (lines 171, 173) whereas the move constructor doc on line 162 and all other files in this PR consistently use @p other. This creates a minor terminology inconsistency.
Suggested diff
/** Move assign.
- `@pre` No awaitables returned by either `*this` or the source's
+ `@pre` No awaitables returned by either `*this` or `@p` other's
`resolve` methods exist.
- `@pre` The execution context associated with the source must
+ `@pre` The execution context associated with `@p` other must
outlive this resolver.
*/📝 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.
| /** Move assign. | |
| @pre No awaitables returned by either `*this` or the source's | |
| `resolve` methods exist. | |
| @pre The execution context associated with the source must | |
| outlive this resolver. | |
| */ | |
| /** Move assign. | |
| `@pre` No awaitables returned by either `*this` or `@p` other's | |
| `resolve` methods exist. | |
| `@pre` The execution context associated with `@p` other must | |
| outlive this resolver. | |
| */ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@include/boost/corosio/native/native_resolver.hpp` around lines 169 - 175,
Update the move assignment documentation in
include/boost/corosio/native/native_resolver.hpp to use the same parameter
naming as the rest of the PR: replace occurrences of the phrase "the source"
with the documented parameter name "@p other" in the Move assign comment (the
section describing preconditions for move assignment/operator=) so it matches
the move constructor and other files' terminology.
|
GCOVR code coverage report https://152.corosio.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-02-18 18:46:04 UTC |
2 participants
…s public API
Add @pre conditions to move constructors and move assignment operators for all public I/O types: tcp_socket, tcp_acceptor, timer, signal_set, resolver, and their native variants. Document awaitable lifetime requirements and buffer validity for read_some/write_some.
Summary by CodeRabbit