`NonNull::{from_ref, from_mut}`

[jhpratt]

Proposal

Problem statement

The only way to construct a NonNull from a (mutable) reference is to either use NonNull::new_unchecked or NonNull::from. The former requires unsafe in addition to core::ptr::from_ref or casts for shared references, while the latter does not necessarily guarantee that there is no type conversion taking place.

Motivating examples or use cases

use std::ptr::NonNull;

struct Foo;
struct Bar;

impl From<&Foo> for NonNull<Bar> {
    fn from(_: &Foo) -> Self {
        unimplemented!()
    }
}

We are trying to avoid a situation where adding an implementation like this in conjunction with refactoring would result in a silent change to NonNull::from. For this reason we are currently using new_unchecked, though this is naturally less than ideal. Internally, I will be introducing an extension trait identical to the below to avoid both unsafe and the drawbacks.

Solution sketch

impl<T: ?Sized> NonNull<T> {
    #[unstable(feature = "nonnull_from_ref", issue = "TODO")]
    #[rustc_const_unstable(feature = "nonnull_from_ref", issue = "TODO")]
    pub const fn from_ref(r: &T) -> Self {
        // Safety: `r` is a reference that is guaranteed to be non-null.
        unsafe { NonNull::new_unchecked(core::ptr::from_ref(r).cast_mut()) }
    }

    #[unstable(feature = "nonnull_from_ref", issue = "TODO")]
    #[rustc_const_unstable(feature = "nonnull_from_ref", issue = "TODO")]
    pub const fn from_mut(r: &mut T) -> Self {
        // Safety: `r` is a reference that is guaranteed to be non-null.
        unsafe { NonNull::new_unchecked(core::ptr::from_mut(r)) }
    }
}

Alternatives

As with most library changes, the only alternatives are to not doing this and to leave it to the ecosystem.

Links and related work

N/A

[BurntSushi]

If you're concerned about the types changing, what do you think about specifying the types in the from call? Does that help? I realize though that this may not be an entirely satisfactory answer.

[jhpratt]

As in From::<&T>::from(r)? That would rely on type inference for the return type, barring some way to do NonNull::from::<&T>(r) or r.into::<NonNull>(). While that would be wonderful, I don't see it happening any time soon. I'm looking at this API in the same light as the now-stabilized core::ptr::{from_ref, from_mut}.

While not the motivating factor, having an API like this would also be const-compatible, unlike a From-based approach.

[kennytm]

As in From::<&T>::from(r)? That would rely on type inference for the return type, barring some way to do NonNull::from::<&T>(r) or r.into::<NonNull>().

NonNull::<T>::from(r)?

[jhpratt]

Yes, but that doesn't guarantee that the type it's being converted from doesn't change.

[jhpratt]

Another one I just ran across: NonNull::from_box would be great, and that currently cannot be done without unsafe.

[cuviper]

NonNull::from_box can be safely written as NonNull::from(Box::leak(b)).

[jhpratt]

Thanks @cuviper. In any situation, the fact that I didn't recognize that should demonstrate some level of need (or at the least documentation).

[kennytm]

@jhpratt you could write <NonNull<T> as From<&T>>::from(r) to absolutely use no inference.

[jhpratt]

That is extremely verbose for something that (imo) should be quite simple.

[kennytm]

That impl is only possible within the crate that defines Bar.

And when using NonNull::<_>::from the only case you are going to accidentally invoke that impl only if the type of r is never explicitly specified or inferred to be &T throughout the entire function.

I think these explicit constructors are fine but the motivation is rather weak.

[jhpratt]

That is the exact situation we found ourselves in when writing code. We didn't want adding an impl within the crate to cause issues, which led to the unsafe indirection. It is admittedly odd (and I brought that up), but I felt an appropriate change would be to be explicit about what is happening.