Skip to content

Use AlignmentError in Unalign's failure conditions#1198

Merged
jswrenn merged 1 commit intomainfrom
unalign-err
May 6, 2024
Merged

Use AlignmentError in Unalign's failure conditions#1198
jswrenn merged 1 commit intomainfrom
unalign-err

Conversation

@jswrenn
Copy link
Collaborator

@jswrenn jswrenn commented May 6, 2024

This make's Unalign's methods consistent with zerocopy's other methods, and, in the case of Unalign::try_deref_mut, allows the original &mut Unalign<T> to be reused in the event of failure.

Makes progress towards #1139

@jswrenn jswrenn mentioned this pull request May 6, 2024
3 tasks
I: Invariants,
{
/// Converts a `Ptr` to a `Unalign<T>` into an `Ptr` to an unaligned `T`.
pub(crate) fn into_unaligned(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub(crate) fn into_unaligned(
pub(crate) fn into_unalign(

}
}

impl<'a, T, I> Ptr<'a, T, I>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fancy doc comment here with the UTF-8 arrow character?

where
I: Invariants,
{
/// Converts a `Ptr` to a `Unalign<T>` into an `Ptr` to an unaligned `T`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Converts a `Ptr` to a `Unalign<T>` into an `Ptr` to an unaligned `T`.
/// Converts an unaligned `Ptr<T>` to an aligned `Ptr<Unalign<T>>`.

Copy link
Member

@joshlf joshlf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the outstanding comments, but feel free to merge after that.

This make's `Unalign`'s methods consistent with zerocopy's other
methods, and, in the case of `Unalign::try_deref_mut`, allows the
original `&mut Unalign<T>` to be reused in the event of failure.

Makes progress towards #1139
@jswrenn jswrenn enabled auto-merge May 6, 2024 17:35
@jswrenn jswrenn added this pull request to the merge queue May 6, 2024
Merged via the queue into main with commit 1d90765 May 6, 2024
@jswrenn jswrenn deleted the unalign-err branch May 6, 2024 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants