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

Change From<&'a [u8]> to an explicit function #139

Closed
seanmonstar opened this issue Jun 22, 2017 · 3 comments · Fixed by #286
Closed

Change From<&'a [u8]> to an explicit function #139

seanmonstar opened this issue Jun 22, 2017 · 3 comments · Fixed by #286
Milestone

Comments

@seanmonstar
Copy link
Member

Most of the From conversions to Bytes are shallow copies, just grabbing a pointer and pulling it in. There is also the from_static function to save copying a static set of bytes. However, the From<&'a [u8]> impl will copy the slice into a new buffer (either inline or a new heap allocation, depending on size).

Unfortunately, that &'a [u8] means it applies to &'static [u8] as well. In cases where one is generic over Into<Bytes>, a user can easily and accidentally cause copies of static slices, instead of remembering to call Bytes::from_static. This issue proposes to change the defaults, so that the faster way is easier.

  • Remove From<&'a [u8]> for Bytes/BytesMut.
  • Add From<&'static [u8]> for Bytes/BytesMut.
  • Add a new constructor that can take &'a [u8], that explicitly shows a copy will happen.

The name of this new method could be a few things, these just some examples that occurred to me:

  • Bytes::copy(slice)
  • Bytes::copy_from(slice)
@seanmonstar
Copy link
Member Author

Oh, this would be a breaking change, so would require (block?) 0.5.

@carllerche carllerche added this to the v0.5 milestone Nov 7, 2017
@YetAnotherMinion
Copy link
Contributor

Just to clarify,

From<&'a [u8]> for BytesMut can stay because static slices are not mutable, so a copy has to happen from the 'static slice when creating BytesMut.

So only Bytes would have its From<&'a [u8]> removed.

@carllerche
Copy link
Member

Mentioned by @arthurprs:

While I understand the reasoning this severely limits Byte as a Vec replacement, like backing a https://crates.io/crates/string

Specifically, this issue: carllerche/string#6

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 a pull request may close this issue.

3 participants