Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Ref::try_into_{ref,mut} #1184

Draft
wants to merge 1 commit into
base: v0.8.x
Choose a base branch
from
Draft

Implement Ref::try_into_{ref,mut} #1184

wants to merge 1 commit into from

Conversation

jswrenn
Copy link
Collaborator

@jswrenn jswrenn commented May 3, 2024

Wip. Currently a borrow-checker puzzle.

@joshlf
Copy link
Member

joshlf commented May 4, 2024

Wip. Currently a borrow-checker puzzle.

I believe this is impossible to fix w/o adding bounds. IntoByteSlice doesn't imply Copy, and so self.0.into() irrevocably moves self.0. You'd need to add a Copy bound or some way of reversing the into() operation (ie, to reconstruct the value stored in self.0 given the error).

@jswrenn
Copy link
Collaborator Author

jswrenn commented May 4, 2024

Perhaps there's a way forward doing &*self.0 instead? We only need to call is_bit_valid, here.

@joshlf
Copy link
Member

joshlf commented May 4, 2024

Unfortunately that wouldn't live long enough. It would have the lifetime of the stack frame, but we need to return from the stack frame

@jswrenn
Copy link
Collaborator Author

jswrenn commented May 7, 2024

Unfortunately that wouldn't live long enough. It would have the lifetime of the stack frame, but we need to return from the stack frame

Would you like me to provide that hat you'll eat, or will you be providing your own? ;-)

@jswrenn jswrenn requested a review from joshlf May 7, 2024 12:13
@jswrenn jswrenn force-pushed the ref-try_into branch 2 times, most recently from 9f62f39 to fa5ec71 Compare May 7, 2024 12:29
src/lib.rs Outdated

/// Attempts to convert this `Ref<_, T>` into a `&T`.
///
/// `try_into_ref` consumes the `Ref`, and returns a reference to `T`.
Copy link
Member

Choose a reason for hiding this comment

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

Document why this could fail?

if !is_bit_valid {
Err(ValidityError::new(self))
} else {
match Ptr::from_ref(self.0.into()).try_cast_into_no_leftover::<T>() {
Copy link
Member

Choose a reason for hiding this comment

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

Is there any way we can keep the original Ptr and avoid calling constructing it twice?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not that I can find, unfortunately. It might be possible with more unsafe code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We might be able to do something like this, but lifetime extension makes me nervous:

    /// Attempts to convert this `Ref<_, T>` into a `&mut T`.
    ///
    /// `try_into_mut` consumes the `Ref`, and returns a reference to `T`.
    #[must_use = "has no side effects"]
    #[inline(always)]
    pub fn try_into_mut(mut self) -> Result<&'a mut T, ValidityError<Self, T>>
    where
        T: TryFromBytes,
    {
        // Extend the lifetime of `&mut *self.0` to `'a`.
        // SAFETY: Idk, this feels sketchy.
        let bytes = unsafe { mem::transmute::<&mut [u8], &'a mut [u8]>(&mut *self.0) };
        match Ptr::from_mut(bytes).try_cast_into_no_leftover::<T>() {
            Ok(candidate) => {
                match candidate.try_into_valid() {
                    Ok(candidate) => Ok(candidate.as_mut()),
                    Err(e) => Err(e.with_src(self)),
                }
            }
            Err(CastError::Validity(i)) => match i {},
            Err(CastError::Alignment(_) | CastError::Size(_)) => {
                // SAFETY: By invariant on `Ref::0`, the referenced byte slice
                // is aligned to `T`'s alignment and its size corresponds to a
                // valid size for `T`. Since properties are checked upon
                // constructing `Ref`, these failures are unreachable.
                unsafe { core::hint::unreachable_unchecked() }
            },
        }
    }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If IntoByteSlice documented that .into() behaved identically to .deref(), this might be fine?

src/lib.rs Outdated

/// Attempts to convert this `Ref<_, T>` into a `&mut T`.
///
/// `try_into_mut` consumes the `Ref`, and returns a reference to `T`.
Copy link
Member

Choose a reason for hiding this comment

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

Document why this could fail?

if !is_bit_valid {
Err(ValidityError::new(self))
} else {
match Ptr::from_mut(self.0.into()).try_cast_into_no_leftover::<T>() {
Copy link
Member

Choose a reason for hiding this comment

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

Is there any way we can keep the original Ptr and avoid calling constructing it twice?

@jswrenn jswrenn changed the base branch from main to v0.8.x October 17, 2024 13:21
jswrenn added a commit that referenced this pull request Oct 17, 2024
Only `Ref::try_as_mut` remains missing, probably pending polonius
landing in rustc.

Partially fixes #1865
Supersedes #1184
jswrenn added a commit that referenced this pull request Oct 18, 2024
Only `Ref::try_as_mut` remains missing, probably pending polonius
landing in rustc.

Partially fixes #1865
Supersedes #1184
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