Rust: Tracking issue for Box::into_raw_non_null

[SimonSapin]

This API was introduced in #46952:

impl<T: ?Sized> Box<T> {
    pub fn into_raw_non_null(b: Box<T>) -> NonNull<T> { /* … */ }
}

It is intended as an eventually-stable replacement for Box::into_unique, since there is no plan to stabilize Unique<T> at the moment.

See also discussion at #27730.

[SimonSapin]

It would be nice for this method to be an impl of the From trait instead, but coherence does not allow it since ptr::NonNull and Box are defined in separate crates. Having an Into impl without the corresponding From impl was frowned upon in the PR discussion.

This method can be replaced with unsafe { NonNull::new_unchecked(Box::into_raw(b)) }. This unsafe block is sound because Box is never null. Having a dedicated API removes the need for that unsafe block.

So while we can live without it, I’m still in favor of stabilizing this. It’s small and won’t be a maintenance burden.

[nvzqz]

I'm in favor of Box::into_non_null since we have Box::into_unique. Feels more consistent that way.

[SimonSapin]

@nvzqz Box::into_unique is unstable though, and I don’t know if it (or Unique) well even be stabilized.

[nvzqz]

I'm acutely aware it is. If we choose to stabilize both, then their names should be considered against each other regarding convention.

[mtak-]

Any movement on this? A cursory search of github for "new_unchecked into_raw" shows examples of the unsafe { NonNull::new_unchecked(Box::into_raw(b)) } pattern.

[luojia65]

Any further movements? This could be okay in my projects.

[zakarumych]

This method would be more ergonomic but isn't

unsafe { NonNull::new_unchecked(Box::into_raw(b)) }

without unsafe block is possible today as

NonNull::from(Box::leak(b))

[SimonSapin]

Oooh that’s is a very interesting one, thanks @omni-viral. Box::leak even already documents Box::from_raw as a valid way to drop (or presumably recover) a box that has previously been leaked.

I think I’d be happy with:

• Deprecate into_raw_non_null, with a warning message that says to use NonNull::from(Box::leak(b)) instead
• Change all internal uses to do so
• After some time, remove this deprecated unstable method.

How does this sound?

@rfcbot fcp close

[rfcbot]

Team member @SimonSapin has proposed to close this. The next step is review by the rest of the tagged team members:

• [x] @Amanieu
• [x] @KodrAus
• [x] @SimonSapin
• [x] @dtolnay
• [ ] @sfackler
• [ ] @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[dtolnay]

The coherence comment in https://github.com/rust-lang/rust/issues/47336#issuecomment-373941458 is outdated as of rust 1.41. How about adding the From impl now?

[SimonSapin]

Good point, but I’m reconsidering my previous statement that this would be ideal. Unlike e.g. From<&mut T> for NonNull<T>, this conversion gives away ownership of a Box, which would otherwise automatically deallocate when dropped, to a pointer that doesn’t do any ownership tracking and is Copy. So I’m a bit hesitant about enabling it in generic contexts.

Maybe it’s fine, though?

[dtolnay]

Fair enough. I do like NonNull::from(Box::leak(b)) over NonNull::from(b) but I wish there were a way it could be more discoverable.

[Lokathor]

Having it just be longer is definitely worse.

[Mark-Simulacrum]

I would personally disagree that the length is an issue here, and AFAICT, we can always add the shorter one later, right?

We could also plausibly call this NonNull::leak(Box<T>), right? I'm not sure about the generic being useful here, do we have examples of code with a desire for an Into/From bound with NonNull?

[SimonSapin]

Not really, I was initially tempted to use From mostly to avoid having to come up with an awkward name like into_raw_non_null

[SimonSapin]

~I’ve removed KodrAus’ checkbox~, per https://github.com/rust-lang/team/commit/45013d114d89480c33cab2a1a99828e4aacb8aee.

Edit: that didn’t have the expected effect. rfcbot added it back. I’ve checked it instead.

[rfcbot]

:bell: This is now entering its final comment period, as per the review above. :bell:

[SimonSapin]

PR https://github.com/rust-lang/rust/pull/57934 has added Rc::into_raw_non_null and Arc::into_raw_non_null, reusing this tracking issue. But nobody involved commented here or updated the issue description, so the proposed plan at https://github.com/rust-lang/rust/issues/47336#issuecomment-586589016 does not account for them.

Unlike Box, Rc and Arc do not have a leak method that could be combined with NonNull::from.

I’m inclined to deprecate anyway, even though the replacement unsafe { NonNull::new_unchecked(Rc::into_raw(x) as *mut _) } requires unsafe code. These methods seem very niche, and the implementation PR does not discuss any motivation for adding them.

@dwijnand, given the rest of this thread, what do you think?

[dwijnand]

the implementation PR does not discuss any motivation for adding them

The previous formulation did (https://github.com/rust-lang/rust/pull/56998):

/cc @withoutboats who mentioned to me this might be worth adding to the standard library, in withoutboats/smart#4

@dwijnand, given the rest of this thread, what do you think?

I trust your call if you think it's too niche to be worth keeping.

[SimonSapin]

@rust-lang/libs Any thoughts on Rc::into_raw_non_null and Arc::into_raw_non_null?

[maxeonyx]

Have just come across this function, while reading the source for Rc. My 2 cents: At some point(s) after making a NonNull<T>, I am going to have to use an unsafe block to dereference it. I am OK with using one more unsafe block (unsafe { NonNull::new_unchecked(Box::into_raw(b)) }) to create it, especially since I can see that it is sound (as stated above). I also think it's niche.

[Kixunil]

Would it make sense to add leak to Rc and Arc? This is useful in some exotic cases. E.g. sending such pointer over a pipe. (I've seen such thing in some C++ codebase.)

[rfcbot]

The final comment period, with a disposition to close, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

[KodrAus]

Since we've shipped the deprecation of these methods I'll go ahead and close this tracking issue now, but please re-open if we'd like to keep it active until the unstable methods have been removed entirely!

[SimonSapin]

I think we should keep tracking issues open as long as there are stable items pointing to them in the tree. But this case can be resolved quickly, I filed a removal PR: https://github.com/rust-lang/rust/pull/74902