Make `BorrowedBuf` and `BorrowedCursor` generic over the data#149749

joshtriplett · 2025-12-07T23:52:40Z

Replace uses of u8 with a generic type. This will allow reusing
BorrowedBuf elsewhere in the standard library, for purposes other than
byte buffers.

As discussed in libs-api; cc @Amanieu.

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 · 2025-12-07T23:52:45Z

r? @scottmcm

rustbot has assigned @scottmcm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Amanieu

r=me for the rest

View changes since this review

Amanieu · 2025-12-09T22:03:52Z

-pub struct BorrowedBuf<'data> {
+///
+/// The type defaults to managing bytes, but can manage any type of data.
+pub struct BorrowedBuf<'data, T = u8> {


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.

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.

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.

I think default type params can lead to frustrating and confusing bugs, particularly silent errors that work for upstream but break downstream. I have had several of these when refactoring data structures to use the unstable Allocator trait. A lint that demands all generic parameters be provided explicitly sounds too complex for the AST-based approach I believe clippy is limited to.

One particular concern I have with the u8 default is assuming that slice length in elements is always equivalent to the number of bytes, which could interact poorly with the unsafe set_init() method. I can very easily imagine the default leading to an upstream crate later deciding they want to support more types, and in the process failing to multiply by size_of::<T>(). I see alignment being potentially even trickier.

I do see #150129 potentially improving this, but I believe the concern remains for the BorrowedCursor struct.

Amanieu · 2025-12-09T22:04:42Z

 /// on the data in that buffer by transitivity).
 #[derive(Debug)]
-pub struct BorrowedCursor<'a> {
+pub struct BorrowedCursor<'a, T = u8> {


bors · 2025-12-17T23:10:15Z

☔ The latest upstream changes (presumably #150106) made this pull request unmergeable. Please resolve the merge conflicts.

scottmcm

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.

View changes since this review

scottmcm · 2025-12-21T20:48:39Z

 }

-impl Debug for BorrowedBuf<'_> {
+impl<T> Debug for BorrowedBuf<'_, T> {


I was surprised to not see T: Debug, but I guess that's not a this-PR discussion.

joshtriplett · 2026-03-09T23:11:55Z

I'm going to wait to update this until after #150129 is merged.

briansmith · 2026-04-13T18:58:48Z

 /// The lifetime `'data` is a bound on the lifetime of the underlying data.
-pub struct BorrowedBuf<'data> {
+///
+/// The type defaults to managing bytes, but can manage any type of data.


View changes since the review

It seems prudent to have tests with a T that isn't the same size as u8 to ensure that there are no byte-size vs. slice-length issues.

Similarly, it would be nice to have some test for non-Copy T. Since the BorrowedBuf doesn't own any T then no special drop handling is necessary. Is it worth calling this out? I seem to remember in the discussion of doing this that people thought that this could be done for only T: Copy presumably because of drop checking.

I think element is better than data here.

In particular, it would be good to clarify (improve documentation and add a test) of BorrowedBuf::clear() with a non-empty BorrowedBuf of T: Drop. It seems wrong to "forget" that some values that need to be dropped are there, as then a new BorrowedCursor would overwrite them without dropping them, right?

briansmith · 2026-04-13T19:06:09Z

 /// This type makes it safer to work with `MaybeUninit` buffers, such as to read into a buffer
-/// without having to initialize it first. It tracks the region of bytes that have been filled and
-/// the region that remains uninitialized.
+/// without having to initialize it first. It tracks the region of data that has been filled and the


View changes since the review

I think "elements that have" is better (clearer) than "data that has" here.

briansmith · 2026-04-13T19:06:41Z

 /// The lifetime `'data` is a bound on the lifetime of the underlying data.
-pub struct BorrowedBuf<'data> {
+///
+/// The type defaults to managing bytes, but can manage any type of data.


I think element is better than data here.

briansmith · 2026-04-13T19:07:40Z

    /// # Safety
    ///
-    /// The caller must not uninitialize any previously initialized bytes.
+    /// The caller must not uninitialize any previously initialized data.


View changes since the review

elements?

Consider the case where T: Drop. Then as_mut now has an additional hazard: For any initialized element, overwriting the element must drop the old one, right?

briansmith · 2026-04-13T19:28:45Z

    /// Panics if `self.capacity()` is less than `buf.len()`.
    #[inline]
-    pub fn append(&mut self, buf: &[u8]) {
+    pub fn append(&mut self, buf: &[T])


View changes since the review

Perhaps this should be renamed append_copy_of_slice, analogous to slice::write_copy_of_slice, to make room for a future method that doesn't require T: Copy?

briansmith · 2026-04-13T23:35:35Z

    /// # Safety
    ///
-    /// The caller must not uninitialize any previously initialized bytes.
+    /// The caller must not uninitialize any previously initialized data.


Consider the case where T: Drop. Then as_mut now has an additional hazard: For any initialized element, overwriting the element must drop the old one, right?

briansmith · 2026-04-13T23:37:06Z


 /// Creates a new `BorrowedBuf` from a fully initialized slice.
-impl<'data> From<&'data mut [u8]> for BorrowedBuf<'data> {
+impl<'data, T> From<&'data mut [T]> for BorrowedBuf<'data, T> {


View changes since the review

If T: Drop then this is pretty dangerous, because it immediately "forgets" that all the elements need to be dropped (before being overwritten).

Make BorrowedBuf and BorrowedCursor generic over the data

c150931

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 Status: Awaiting review from the assignee but also interested parties. label Dec 7, 2025

rustbot assigned scottmcm Dec 7, 2025

rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Dec 7, 2025

joshtriplett added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 7, 2025

Amanieu reviewed Dec 9, 2025

View reviewed changes

Amanieu mentioned this pull request Dec 12, 2025

Implement Random for array #136732

Closed

scottmcm mentioned this pull request Dec 21, 2025

Tracking Issue for core_io_borrowed_buf #117693

Open

5 tasks

scottmcm approved these changes Dec 21, 2025

View reviewed changes

scottmcm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 9, 2026

briansmith reviewed Apr 13, 2026

View reviewed changes

Uh oh!

Conversation

joshtriplett commented Dec 7, 2025

Uh oh!

rustbot commented Dec 7, 2025

Uh oh!

Amanieu left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bors commented Dec 17, 2025

Uh oh!

scottmcm left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshtriplett commented Mar 9, 2026

Uh oh!

briansmith Apr 13, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

briansmith Apr 13, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

briansmith Apr 13, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

briansmith Apr 13, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

briansmith Apr 13, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Amanieu left a comment •

edited by rustbot

Loading

scottmcm left a comment •

edited by rustbot

Loading

briansmith Apr 13, 2026 •

edited by rustbot

Loading

briansmith Apr 13, 2026 •

edited by rustbot

Loading

briansmith Apr 13, 2026 •

edited by rustbot

Loading

briansmith Apr 13, 2026 •

edited by rustbot

Loading

briansmith Apr 13, 2026 •

edited by rustbot

Loading