Make BorrowedBuf and BorrowedCursor generic over the data
#149749
Conversation
Replace uses of `u8` with a generic type. This will allow reusing `BorrowedBuf` elsewhere in the standard library, for purposes other than byte buffers.
rustbot
added
the
S-waiting-on-review
rustbot
added
the
T-libs
joshtriplett
added
T-libs-api
| pub struct BorrowedBuf<'data> { | ||
| /// | ||
| /// The type defaults to managing bytes, but can manage any type of data. | ||
| pub struct BorrowedBuf<'data, T = u8> { |
There was a problem hiding this comment.
I would prefer not having u8 as a default. It's not a huge deal to have users write it out and we don't really have a precedent for hiding such things.
There was a problem hiding this comment.
The main motivation for doing so, apart from convenience, was to localize the changes so everything depending on BorrowedBuf and BorrowedCursor didn't all have to change at once.
I do think, though, that u8 is going to be by far the most common case here, given the hopefully widespread use of read_buf.
There was a problem hiding this comment.
While I agree that u8 will be the most common case, I don't think this is a good justification for hiding the type. I don't see any harm in requiring users to explicitly indicate that they are taking a buffer of u8. Just like how read/write explicitly take [u8] slice.
| /// on the data in that buffer by transitivity). | ||
| #[derive(Debug)] | ||
| pub struct BorrowedCursor<'a> { | ||
| pub struct BorrowedCursor<'a, T = u8> { |
|
☔ The latest upstream changes (presumably #150106) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
I think it'd be fine to land this PR for generic-izing the type, then we can land another PR right afterwards to remove the default and update the uses.
Code looks good to me, and I tried to scan for anything making unsaid assumptions about T (being Copy, Freeze, etc) and didn't find any that weren't already updated.
So r=me with conflicts fixed.
Aside: excited to try this out with generic types. Might be a great way to solve some of the Iterator::next_chunk or Vec::push_within_existing_capacity scenarios.
| } | ||
|
|
||
| impl Debug for BorrowedBuf<'_> { | ||
| impl<T> Debug for BorrowedBuf<'_, T> { |
There was a problem hiding this comment.
I was surprised to not see T: Debug, but I guess that's not a this-PR discussion.
5 participants
Replace uses of
u8with a generic type. This will allow reusingBorrowedBufelsewhere in the standard library, for purposes other thanbyte buffers.
As discussed in libs-api; cc @Amanieu.