ReadBuf's API

[nrc]

cc #78485

ReadBuf is unstable, part of the read-buf feature. I believe the API might be improved by some of the following ways (I've grouped methods by theme):

Construction

• remove new (it should create an empty ReadBuf, but that is useless, I think)
• Should implement From<&'a mut [u8]> for ReadBuf<'a> to create a ReadBuf from a byte slice (rather than new)
• Should implement From<&'a mut [MaybeUninit<u8>]> for ReadBuf<'a> to create a ReadBuf from a byte slice (rather than uninit)

Length functions

• Keep capacity
• filled_len -> len to match Vec etc.
• remove filled_len (use filled().len())
• remove remaining (can use capacity() - len() or unfilled().len())
• remove initialized_len (can use initialized().len())

Slicing functions

• Keep filled, initialized, unfilled, uninitialized, and mut variants
• Remove filled and filled_mut, replace by implementing Into<&'a mut [u8]> for ReadBuf<'a>

Initializing data

• Remove initialize_unfilled and initialize_unfilled_to, unless these are super useful
  • rationale, getting unfilled slice is easy, initialising is easy, so it seems the only benefit is tracking this
• keep assume_init

Misc

• Keep clear and append
• Remove add_filled
• Rename set_filled to assume_filled (to match assume_init)

The current API is documented at https://doc.rust-lang.org/nightly/std/io/struct.ReadBuf.html

This issue continues a discussion from Zulip (https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/ReadBuf.20API), there was not much discussion relevant to the points above.

[djc]

filled_len -> len to match Vec etc.

Not sure about this one. If I convert a &mut [u8] to ReadBuf (with let mut buf = ReadBuf::from(slice);, say), suddenly the buf.len() is shorter than the slice.len(). IMO we should either stick with filled_len() or ditch it in favor of filled().len() (which is nicely symmetric with unfilled().len()).

Remove filled and filled_mut, replace by implementing Into<&'a mut [u8]> for ReadBuf<'a>

This one also doesn't make sense to me. filled() and filled_mut() reborrow the buffer from the ReadBuf, which is very different from giving away ownership of the ReadBuf in order to get a slice. IIRC it's at least somewhat useful to be able to get access to filled() while keeping the ReadBuf intact for further use.

[joshtriplett]

I agree that we shouldn't have unqualified methods like len affect part (not all) of the buffer; too much potential for confusion.

[nrc]

suddenly the buf.len() is shorter than the slice.len()

Do you think this will have practical problems? To me it seems that creating a ReadBuf from a slice means we change the semantics of how we interpret the slice and thus len could have a different value.

filled() and filled_mut() reborrow the buffer from the ReadBuf, which is very different from giving away ownership of the ReadBuf in order to get a slice. IIRC it's at least somewhat useful to be able to get access to filled() while keeping the ReadBuf intact for further use.

That makes sense, does Into<&'a mut [u8]> for &'a mut ReadBuf<'a> work better? (I had also previously suggested using Deref, though I think that is too magical) My underlying philosophy here is that only the user wants to get access to the filled slice, and if the user wants the filled slice, then the filled slice is the part the 'useful' part.

I agree that we shouldn't have unqualified methods like len affect part (not all) of the buffer; too much potential for confusion.

My analogy here is Vec where len is only the 'useful' part of the available buffer, and capacity is the whole part.

[beepster4096]

I don't like having From/Into be the only way to do this. I think it's always better to have explicit methods of doing things, in addition to the conversion traits.

[djc]

Do you think this will have practical problems? To me it seems that creating a ReadBuf from a slice means we change the semantics of how we interpret the slice and thus len could have a different value.

I guess my concern is that there are two lengths in play here, the filled length and the initialized length, and because there are two it would be better to disambiguate between them explicitly. The same is not true for Vec, where all filled values are also initialized.

That makes sense, does Into<&'a mut [u8]> for &'a mut ReadBuf<'a> work better? (I had also previously suggested using Deref, though I think that is too magical) My underlying philosophy here is that only the user wants to get access to the filled slice, and if the user wants the filled slice, then the filled slice is the part the 'useful' part.

IMO this proposed Into argument loogs pretty convoluted and not at all intuitive. I don't think conversions into slices like this are very common? They would almost always be represented by a method instead. And to turn it the other way around, what is gained by changing this into an Into impl? Having methods is perfectly fine, their documentation is generally easier to locate and their name provides an explicit hint about what's going on.

My analogy here is Vec where len is only the 'useful' part of the available buffer, and capacity is the whole part.

See my point above about there being a third boundary in play in this case, vs two for Vec.

[nrc]

OK, I've removed the controversial changes: lets keep filled and not implement deref/into for getting the the filled part. I've suggested removing filled_len and using filled().len(). (fwiw, I think the distinction between filled and unfilled should not be important for the user (only to a function which is filling the buffer), so I think the analogy to Vec holds, but operating directly on the slices seems fine).

I'm still unsure about initialize_unfilled - I'd like to remove it since it seems like a weird mashing together of functionality, however, I can also believe it might be convenient.