Add io::Read::read_le and io::Read::read_be#156983
Conversation
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
These functions make it easy to read a fixed-size type as little-endian or big-endian. They're trivial wrappers around the combination of `io::Read::read_array` and `T::from_le_bytes`/`T::from_be_bytes`. The implementation uses a sealed trait `FromEndianBytes`. That trait is currently in `std` and accepts a `&mut io::Read`. Once we can use associated consts in the types of method parameters, we can change this trait to have `from_le_bytes` and `from_be_bytes` methods, move it to `core`, and make it public.
|
Just a warning: My experience from having methods similar to these in the bytes crate is that you will receive endless requests to add more and more variations of them. Just look at how many methods |
|
@Darksonn I'm hoping that having generic methods (such that we don't need a separate method per return type) will avoid the combinatorial explosion. We also don't need the |
|
Is there any other discussions that might be relevant to link about this? I figure there isn't an ACP, but I saw that a bunch of RustWeek meetings seemed to have involved various documents people made, and wonder if there's an associated one for this. |
|
Yes, the trait does help somewhat. |
| /// Trait for types that can be converted from a fixed-size byte array with a specified endianness | ||
| #[unstable(feature = "read_le_be_internals", reason = "internals", issue = "none")] | ||
| // Once we can use associated consts in the types of method parameters, rewrite this to have | ||
| // `from_le_bytes` and `from_be_bytes` methods, move it to `core`, and make it public. | ||
| pub trait FromEndianBytes: crate::sealed::Sealed + Sized { | ||
| #[doc(hidden)] | ||
| fn read_le_from(r: &mut impl Read) -> Result<Self>; | ||
|
|
||
| #[doc(hidden)] | ||
| fn read_be_from(r: &mut impl Read) -> Result<Self>; | ||
| } |
There was a problem hiding this comment.
I assume this was discussed in-person at Rust week, but I think it would be nicer to introduce BigEndian and LittleEndian wrappers so we could omit this trait and instead use TryFrom directly. I have an example here. The biggest downsides are:
- It requires adding a
From<Infallible>implementation forstd::io::Error. But I think that's fine, and also means primitives can have infallibleFrom<XEndian<[u8; _]>>implementations that naturally get exposed asTryFromimplementations for the sake ofread_le/read_be. - My example has an explicit
Nparameter fir the same reasons described in the PR description, and could be removed under the same conditions. I do think it's noteworthy that the compiler is able to successfully inferNeven with the level of indirection seen here. Could a shorter-term fix for the usability of these methods just be the compiler allowing these parameters to be omitted, similar to how lifetimes that can be inferred are allowed to be omitted?
These functions make it easy to read a fixed-size type as little-endian
or big-endian. They're trivial wrappers around the combination of
io::Read::read_arrayandT::from_le_bytes/T::from_be_bytes.The implementation uses a sealed trait
FromEndianBytes. That trait iscurrently in
stdand accepts a&mut io::Read. Once we can useassociated consts in the types of method parameters, we can change this
trait to have
from_le_bytesandfrom_be_bytesmethods, move it tocore, and make it public.As discussed at RustWeek.