Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enrich StreamReader and SinkWriter #5941

Merged

Conversation

nicflower
Copy link
Contributor

Implementing feature suggested in issue #5745

Basically what was done is:

  • implement Sink for StreamReader
  • implement Stream for SinkWriterStream>
  • implement AsyncRead for SinkWriter
  • implement Stream for CopyToBytes

The implementation is pretty straightforward since the actual work is all demanded to the underlying object, so I thought that introducing new tests would not be really helpful.

@nicflower nicflower force-pushed the extend_streamreader_and_sinkwriter branch from 6422cd8 to 90c89cf Compare August 22, 2023 16:07
Comment on lines 71 to 76
impl<S, T> Stream for CopyToBytes<S>
where
S: Stream<Item = T> + std::marker::Unpin,
{
type Item = T;
fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
let ref_mut = self.get_mut().get_mut();
Pin::new(ref_mut).poll_next(cx)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't want Unpin bounds if they're not necessary. Also, the T parameter can be dropped.

Suggested change
impl<S, T> Stream for CopyToBytes<S>
where
S: Stream<Item = T> + std::marker::Unpin,
{
type Item = T;
fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
let ref_mut = self.get_mut().get_mut();
Pin::new(ref_mut).poll_next(cx)
}
}
impl<S: Stream> Stream for CopyToBytes<S> {
type Item = S::Item;
fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
self.project().inner.poll_next(cx)
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

Please also fix it for SinkWriter and StreamReader.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@Darksonn Darksonn added A-tokio-util Area: The tokio-util crate M-io Module: tokio/io labels Aug 22, 2023
@nicflower nicflower force-pushed the extend_streamreader_and_sinkwriter branch 3 times, most recently from 28257ef to 109b46b Compare August 23, 2023 16:13
@nicflower nicflower force-pushed the extend_streamreader_and_sinkwriter branch from 109b46b to 83b564d Compare August 23, 2023 16:20
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me!

@Darksonn Darksonn enabled auto-merge (squash) August 23, 2023 17:25
@Darksonn Darksonn merged commit 59c9364 into tokio-rs:master Aug 23, 2023
71 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-util Area: The tokio-util crate M-io Module: tokio/io
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants