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

Consider adding Buf::into_bytes_mut() #228

Open
carllerche opened this issue Sep 6, 2018 · 2 comments
Open

Consider adding Buf::into_bytes_mut() #228

carllerche opened this issue Sep 6, 2018 · 2 comments

Comments

@carllerche
Copy link
Member

Motivation

The Buf trait is designed to provide an abstraction for reading data about of byte slice like types (&[u8], Vec<u8>, ropes, etc...).

A difficulty arises when one wishes to perform more complex operations with a data slice. For example, any sort of in-place data mutation.

In order to mutate the contents of a Buf, the Buf must be read into a storage type (such as Vec<u8> or BytesMut). The problem is that the Buf type might already be in an appropriate storage type.

Proposal

I propose to add the following to the Buf trait:

fn into_bytes_mut(self) -> BytesMut where Self: Sized {
    let mut ret = BytesMut::with_capacity(self.remaining());
    ret.put(self)
    ret
}

This would be a backwards compatible. It would also let types that are able to have improved implementations. For example:

impl Buf for BytesMut {
    fn into_bytes_mut(self) -> BytesMut where Self: Sized {
        self
    }
}

Although, in 0.4, Buf is not implemented for BytesMut, this will work in 0.5. That said, this change would still help today as this change is motivated by BufStream. An implementation of BufStream for BytesMut could use a (currently undefined) BytesBuf type that impls Buf and has an efficient into_bytes_mut impl.

@seanmonstar
Copy link
Member

The motivation makes sense, the only thing that feels off is that it ties BytesMut more directly into Buf, which feels like some sort of polution... I know they are in the same crate right now, but we've talked before about if Buf/BufMut should be in a base trait, so that they aren't tied to breaking changes of Bytes/BytesMut.

@carllerche
Copy link
Member Author

I was thinking about it more, I am currently of the mind that they should stay together.

First, there will doubtfully be many more breaking changes after this. We've been using it for a while and the only real issue has been the IntoBuf implementation. Second, this use case would require them to stay together.

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

No branches or pull requests

2 participants