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

Add AsyncWriteExt::into_sink #1675

Merged
merged 3 commits into from
Jun 17, 2019
Merged

Add AsyncWriteExt::into_sink #1675

merged 3 commits into from
Jun 17, 2019

Conversation

Nemo157
Copy link
Member

@Nemo157 Nemo157 commented Jun 14, 2019

closes #1660


struct Block {
offset: usize,
bytes: Box<dyn AsRef<[u8]>>,
Copy link
Member

Choose a reason for hiding this comment

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

I'd make Block and IntoSink generic on the kind of Bytes: AsRef<[u8]> they accept. This would allow folks to send in e.g. references to buffers, as sending vectors without double-allocating them. If folks want to send multiple different kinds of Bytes, they can set Box<dyn AsRef[u8]> as the generic parameter.

Note that this would allow dropping the 'static bounds below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, it doesn't actually allow using Box<dyn AsRef<[u8]>> as there is impl<T: ?Sized> AsRef<T> for Box<T> instead of impl<U, T: AsRef<U> + ?Sized> AsRef<U> for Box<T>, but it should be possible to make a custom newtyped-Box with that impl if someone does need that behaviour.

@cramertj
Copy link
Member

LGTM aside from switching to a type parameter to allow the caller to specify the type of the SinkItem.

@cramertj cramertj merged commit cd6983f into rust-lang:master Jun 17, 2019
@Nemo157 Nemo157 deleted the into-sink branch June 18, 2019 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add AsyncWriteExt::into_sink
2 participants