Encode infallible alignment errors in types#1718
Conversation
|
Still need to bikeshed naming and how to write the doc comment, but otherwise it's done. |
jswrenn
left a comment
There was a problem hiding this comment.
This is really elegant! I particularly like the error-ization of validate_aligned_to.
1b8c3f7 to
e9e640d
Compare
jswrenn
left a comment
There was a problem hiding this comment.
A few #[inline(always)] nits, but then I'm happy to merge this.
I wonder if this is sufficiently elegant that we should remove the unaligned methods from Ref!
src/error.rs
Outdated
| } | ||
|
|
||
| impl<Src, Dst: ?Sized + Unaligned> Into<SizeError<Src, Dst>> for CastError<Src, Dst> { | ||
| fn into(self) -> SizeError<Src, Dst> { |
There was a problem hiding this comment.
Slap an #[inline(always)] on this — we want the effects of unreachable_unchecked to propagate up the call chain.
src/error.rs
Outdated
| } | ||
|
|
||
| impl<Src, Dst: ?Sized + Unaligned> AlignmentError<Src, Dst> { | ||
| fn into_infallible(self) -> Infallible { |
There was a problem hiding this comment.
Slap an #[inline(always)] on this — we want the effects of unreachable_unchecked to propagate up the call chain.
src/error.rs
Outdated
| } | ||
| } | ||
|
|
||
| impl<Src, Dst: ?Sized + Unaligned, S, V> Into<ConvertError<Infallible, S, V>> |
There was a problem hiding this comment.
Why are we implementing Into here instead of From?
src/error.rs
Outdated
| } | ||
|
|
||
| impl<Src, Dst: ?Sized + Unaligned> AlignmentError<Src, Dst> { | ||
| fn into_infallible(self) -> Infallible { |
There was a problem hiding this comment.
For consistency, should this be an implementation of From? Then we could have a consistent messaging that calling .into() at all levels makes impossible error branches Infallible.
e9e640d to
df8a716
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1718 +/- ##
==========================================
- Coverage 88.79% 88.35% -0.45%
==========================================
Files 16 16
Lines 5846 5889 +43
==========================================
+ Hits 5191 5203 +12
- Misses 655 686 +31 ☔ View full report in Codecov by Sentry. |
df8a716 to
22e78b9
Compare
Permit callers to prove at compile time that alignment errors are unreachable for unaligned destination types. This permits them to infallibly ignore this error condition.
22e78b9 to
761d909
Compare
Probably, but let's test it in Fuchsia first. |
| /// Infallibly discards the alignment error from this `ConvertError` since | ||
| /// `Dst` is unaligned. | ||
| /// | ||
| /// Since [`Dst: Unaligned`], it is impossible to encounter an alignment | ||
| /// error. This method permits discarding that alignment error infallibly | ||
| /// and replacing it with [`Infallible`]. | ||
| /// | ||
| /// [`Dst: Unaligned`]: crate::Unaligned | ||
| /// | ||
| /// # Examples | ||
| /// | ||
| /// ``` | ||
| /// use core::convert::Infallible; | ||
| /// use zerocopy::*; | ||
| /// # use zerocopy_derive::*; | ||
| /// | ||
| /// #[derive(TryFromBytes, KnownLayout, Unaligned, Immutable)] | ||
| /// #[repr(C, packed)] | ||
| /// struct Bools { | ||
| /// one: bool, | ||
| /// two: bool, | ||
| /// many: [bool], | ||
| /// } | ||
| /// | ||
| /// impl Bools { | ||
| /// fn parse(bytes: &[u8]) -> Result<&Bools, UnalignedTryCastError<&[u8], Bools>> { | ||
| /// // Since `Bools: Unaligned`, we can infallibly discard | ||
| /// // the alignment error. | ||
| /// Bools::try_ref_from_bytes(bytes).map_err(Into::into) | ||
| /// } | ||
| /// } | ||
| /// ``` |
There was a problem hiding this comment.
I don't think users are primed to expect bespoke documentation on trait impls. Let's move this documentation to the error module docs.
There was a problem hiding this comment.
Eh, without the generalization recommendation (which is impossible), I'm okay having these docs here.
Permit callers to prove at compile time that alignment errors are unreachable for unaligned destination types. This permits them to infallibly ignore this error condition.