std::io: migrate ReadBuf to BorrowedBuf/BorrowedCursor by nrc · Pull Request #97015 · rust-lang/rust (original) (raw)

Sorry for the delay in my answer, @nrc, and thank you for taking my input into consideration 🙏

lifetime names (and position)

buffer is such an overloaded term

Ah interesting; I was seeing buf as referring to the byte slice, but you use it to refer to the borrow of that byte slice, which is indeed just as legitimate. So I don't mind 'b then being the 'buf part, with 'a becoming 'data:

pub fn unfilled<'buf>(self: &'buf mut BorrowBuf<'data>) -> BorrowCursor<'buf, 'data>

• But the current 'this approach in your OP seems fine too, thanks for the rename!

[regarding the ordering of lifetime params]

I'm not sure I've come across it explicitly, is it a documented pattern?

Not that I know of, but having a choice of pattern be made consistently is useful, and this ordering is the most suitable one, since:

• Thing<&'lt T> can easily be refactored from/into ThingRef<'lt, T>
• and thus, more generally, Thing<'a, &'b T> from/into ThingRef<'a, 'b, T>

But just a very tiny nit, of course 🙂

clone or reborrow?

Is it common? Is it a convention we should encourage in std?

It's very uncommon indeed. But I think it being that uncommon plays a non-negligible role in its own demise, sort to speak: something is jargony if used rarely / only between the savy people. Things such as rust-lang/reference#788 or even this very recent post https://haibane-tenshi.github.io/rust-reborrowing hint at somewhere, in Rust, missing clearer documentation of reborrowing:

• its formal definition (I have to thank that post for bringing https://docs.rs/reborrow/0.2.0/reborrow/trait.ReborrowMut.html to my attention, which perfectly matches the signature here in question);
• how it implicitly occurs with &muts;
• how one would write lifetime-infected data types that would mimic this behavior.

In the stdlib, there is one instance where this explicit reborrowing comes in play:

impl<'orig> Pin<&'orig mut T> { fn as_mut<'r>(self: &'r mut Pin<&'orig mut T>) -> Pin<&'r mut T>

• the real impl is more generic and takes any P : DerefMut, so this reborrowing signature is even hidden behind generics 😅 (it's the P = &'orig mut T case).

and is indeed the reason for needing these pesky .as_mut()s calls when dealing with Pins.

See also:

• Suggest as_mut to reborrow Pin when calling method #65409
• Suggest as_mut when Pin is used after move #93181

So what I think would benefit the language would be for the Rust book, or some other documentation to be more forthcoming about the reborrowing of &muts, which is deeply related to the common beginner-ish surprise of: "implementing Iter is easy, but IterMut is not" (&'short mut &'long mut [T] only yields &'short mut [T] through reborrowing, whereas &'short &'long [T] can yield &'long [T] through Copy), which comes often on URLO / Discord.

With all that being said, if the word reborrow in and of itself, may be too scary for the stdlib to use, then I guess re-using Pin's as_mut() would at least lead to API consistency 🤷 [EDIT: duh, it's already being used, my bad]. The one word that worries me is clone, since people could end up surprised by the &mut receiver of it

• Aside, btw: how often would beginners be using BorrowBuf? That is, taking https://rust-lang.github.io/wg-async/vision/characters.html, I'd say BorrowBuf does not target Alan nor Niklaus, but Grace and Barbara, so our concerns ought to think of Grace, in this case (since Barbara gets it).

owned or borrowed receivers

In a world with enough self-based methods in both APIs (and the then necessary .reborrow()ing methods), it would be possible to write the following helper function:

fn get_full_buffer<'data>(mut buf: BorrowBuf<'data>) -> &'data mut [u8] { buf.clear(); let mut cursor = buf.unfilled_mut(); // self-based / consumes buf cursor.ensure_init(); cursor.init_mut() // self-based }

With the current API, this function is impossible to write. To make it possible, some of the functions would have to become self-based. Hence my raising awareness about this point, and interest in trying to fix this, if possible, with some self-based methods.

That being said, I've been looking more in detail into how to adapt your API exactly for this purpose, and the only way to achieve it would be with a revamp changing the very point of BorrowCursor (that of not allowing it to modify some data of the BorrowBuf).

• That's why I recant my suggestions for self-based methods: let's keep the API as you have it 🙂

It constrains callees to having write-only access to the buffer.

I see 👍 in that case, we could consider exposing an extra getter to get that split: -> (&[u8], BorrowCursor…), à la https://doc.rust-lang.org/1.61.0/std/vec/struct.Vec.html#method.split_at_spare_mut:

fn split_at_unfilled<'buf>( self: &'buf mut BorrowBuf<'data>, ) -> (&'buf [u8], BorrowCursor<'buf, 'data>)

• And I guess ditto for a .split_at_uninit() for BorrowCursor, although both of these could be added in follow-up PRs, if needed.

TL,DR

The API as it is now, seems fine to me ✅, although I'm still not 100% fond of the clone name for that reborrowing function.