Make BorrowedBuf and BorrowedCursor generic over the data#149749
Open
joshtriplett wants to merge 6 commits into
Open
Make BorrowedBuf and BorrowedCursor generic over the data#149749joshtriplett wants to merge 6 commits into
BorrowedBuf and BorrowedCursor generic over the data#149749joshtriplett wants to merge 6 commits into
Conversation
Collaborator
Amanieu
reviewed
Dec 9, 2025
Collaborator
|
☔ The latest upstream changes (presumably #150106) made this pull request unmergeable. Please resolve the merge conflicts. |
5 tasks
scottmcm
approved these changes
Dec 21, 2025
Member
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.
Member
Author
|
I'm going to wait to update this until after #150129 is merged. |
briansmith
reviewed
Apr 13, 2026
briansmith
reviewed
Apr 13, 2026
briansmith
reviewed
Apr 13, 2026
briansmith
reviewed
Apr 13, 2026
Replace uses of `u8` with a generic type. This will allow reusing `BorrowedBuf` elsewhere in the standard library, for purposes other than byte buffers. This currently requires `T: Copy`, to prevent `T: Drop`, which would require additional careful handling. We can consider relaxing that restriction in the future if we make `T: Drop` work (without pessimizing `Copy` types like `u8`). `ensure_init` currently continues to require `u8`. We could potentially generalize it to `T: Default`, but we'd need to ensure it produced the same or better assembly for `u8`.
c150931 to
f7b1581
Compare
Collaborator
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
briansmith
reviewed
May 26, 2026
This default simplified the commit making these types generic, but we don't actually want the default. Rewrite every user to specify `u8` explicitly.
Member
Author
|
@rustbot ready This is ready for review. I would suggest looking at it commit by commit, as the change to remove the default |
Member
Author
|
r? libs |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
View all comments
Replace uses of
u8with a generic type. This will allow reusingBorrowedBufelsewhere in the standard library, for purposes other thanbyte buffers.
This currently requires
T: Copy, to preventT: Drop, which would require additional careful handling. We can consider relaxing that restriction in the future if we makeT: Dropwork (without pessimizingCopytypes likeu8).ensure_initcurrently continues to requireu8. We could potentially generalize it toT: Default, but we'd need to ensure it produced the same or better assembly foru8.As discussed in libs-api; cc @Amanieu.