Skip to content

feat(net)!: remove owned halves#915

Merged
Berrysoft merged 2 commits into
compio-rs:masterfrom
Berrysoft:feat/remove-net-owned-half
May 17, 2026
Merged

feat(net)!: remove owned halves#915
Berrysoft merged 2 commits into
compio-rs:masterfrom
Berrysoft:feat/remove-net-owned-half

Conversation

@Berrysoft
Copy link
Copy Markdown
Member

We don't use a separate type for File, named pipe, or AsyncFd. It's meaningless to assign a new type now. Let's just make the half types itself.

@Berrysoft Berrysoft added the package: net Related to compio-net label May 10, 2026
@Berrysoft Berrysoft self-assigned this May 10, 2026
@Berrysoft Berrysoft requested review from George-Miao and fantix May 10, 2026 09:32
@github-actions github-actions Bot added breaking change enhancement New feature or request labels May 10, 2026
@Berrysoft Berrysoft requested a review from Copilot May 13, 2026 10:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the dedicated “owned split half” wrapper types from compio-net and instead represents owned halves as the stream type itself (e.g., TcpStream, UnixStream), aligning with the repo’s general approach for ref-counted I/O handles.

Changes:

  • Change TcpStream::into_split / UnixStream::into_split to return (Self, Self) via clone instead of OwnedReadHalf/OwnedWriteHalf.
  • Update Splittable implementations for TcpStream and UnixStream to use Self as both ReadHalf and WriteHalf.
  • Remove the reunite-focused test and add Deref support for borrowed ReadHalf/WriteHalf wrappers.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
compio-net/tests/split.rs Removes the reunite/unsplit test that depended on the deleted owned-half types.
compio-net/src/unix.rs Switches UnixStream::into_split and Splittable owned halves to (UnixStream, UnixStream) via cloning.
compio-net/src/tcp.rs Switches TcpStream::into_split and Splittable owned halves to (TcpStream, TcpStream) via cloning.
compio-net/src/split.rs Deletes owned-half wrappers and reunite error; adds Deref for borrowed ReadHalf/WriteHalf.

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

Comment thread compio-net/src/tcp.rs
Comment thread compio-net/src/unix.rs
@Berrysoft Berrysoft added this to the v0.19 milestone May 13, 2026
@Berrysoft Berrysoft requested a review from AsakuraMizu May 17, 2026 08:41
Copy link
Copy Markdown
Collaborator

@AsakuraMizu AsakuraMizu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also update the document of Splittable?

@Berrysoft
Copy link
Copy Markdown
Member Author

The docs of Splittable says,

TcpStream, UnixStream and references to them in compio::net implement this trait without any lock thanks to the underlying sockets’ duplex nature.

I think it's still valid.

@Berrysoft Berrysoft merged commit d6e3ab0 into compio-rs:master May 17, 2026
71 checks passed
@Berrysoft Berrysoft deleted the feat/remove-net-owned-half branch May 17, 2026 10:06
@github-actions github-actions Bot mentioned this pull request May 17, 2026
paddor added a commit to paddor/omq.rs that referenced this pull request May 22, 2026
compio upstream removed OwnedWriteHalf/OwnedReadHalf (compio-rs/compio#915).
Use cloned streams directly; AsyncWrite is implemented on &TcpStream/&UnixStream.
paddor added a commit to paddor/omq.rs that referenced this pull request May 23, 2026
compio upstream removed OwnedWriteHalf/OwnedReadHalf (compio-rs/compio#915).
Use cloned streams directly; AsyncWrite is implemented on &TcpStream/&UnixStream.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change enhancement New feature or request package: net Related to compio-net

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants