Tracking issue for vec_into_raw_parts

[shepmaster]

#65705 adds:

impl String {
    pub fn into_raw_parts(self) -> (*mut u8, usize, usize) {…}
}
impl<T> Vec<T> {
    pub fn into_raw_parts(self) -> (*mut T, usize, usize) {…}
}

Things to evaluate before stabilization

• Should they return NonNull<* mut T>?
• Should they be associated functions?

[SimonSapin]

Should the returned pointer be a NonNull instead? Box::raw doesn’t do that because it was stabilized before NonNull was.

[shepmaster]

Should these functions be associated functions like Box::into_raw? The Box case is required because we want to avoid colliding with methods exposed by Deref / DerefMut. This isn't a concern for String or Vec<T>, but it may be nice to be consistent across this family of similar functions.

[matklad]

Here's an example where this might have helped with avoiding UB: confio/go-rust-demo#1 (comment). There, the vector was destructed manually and mem::forgetted, however, len was used in place of both length and capacity. If Vec::from_raw_parts was a thing, it might have prevented this issue, as, with cap on your hands, you are more likely to do the right thing with it.

[Lokathor]

If we view this as being the opposite of Vec::from_raw_parts the answers become fairly obvious:

• The pointer shoud be a plain raw pointer, because that's the type that from_raw_parts takes.
• The method should be associated so that both visually it matches the style used for from_raw_parts (in a sense), and also because this is a "weird" method that editors don't need to be suggesting with auto-complete.

[SimonSapin]

I think it’s not at all obvious that deliberately making this method "weird" is a good thing.

[Lokathor]

The method is weird either way. "This uses the vec by self, but won't free the memory" is sufficiently out there.

[SimonSapin]

I don’t see how that makes it weird. Vec::into_boxed_slice is the same. It’s pretty normal for conversions to take self by value.

[Lokathor]

But into_boxed_slice doesn't leak the memory

[ghost]

Well,the documentation does a great job telling you how to avoid a leak and also leaking it is not unsafe.

[Lokathor]

I don't disagree with any of that, but also my opinion is unchanged.

It's not a big deal to have this as associated function vs method, but shepmaster is right that the associated version function makes this meta-family of functions feel consistent across the alloc crate.

But again it's a totally unimportant difference either way, and we should just stabilize either form and get on with the day.

[ghost]

In my opinion functions that start with into_ should be methods to match the definition of other methods and the Into trait unless can collide with methods on the Deref::Target while the rest can be left as associated functions if it is conventional.

[SimonSapin]

@rust-lang/libs Any thoughts on the unresolved questions in the issue description? I’d be inclined to say:

• This method should return a NonNull pointer: the conversion to a raw pointer is easy and safe, the reverse in either unsafe or has an unnecessary branch
• It should not be changed to an associated function: Deref::Target is not to an arbitrary type, so unlike Box there is no risk of shadowing another method.

[BurntSushi]

@SimonSapin SGTM. The missing parallelism between definitions of into_raw_parts and from_raw_parts is a little unfortunate, but I think NonNull is the right choice for the reasons you stated.

[LegionMammal978]

there is I think zero method of std that return a tuple of 3 elements.

Not quite; the slice API has a few of them:

• <[T]>::align_to::<U>() -> (&[T], &[U], &[T])
• <[T]>::align_to_mut::<U>() -> (&mut [T], &mut [U], &mut [T])
• <[T]>::select_nth_unstable() -> (&mut [T], &mut T, &mut [T])
• <[T]>::select_nth_unstable_by() -> (&mut [T], &mut T, &mut [T])
• <[T]>::select_nth_unstable_by_key() -> (&mut [T], &mut T, &mut [T])

To be fair, none of those methods have the same length vs. capacity confusion.

Then Rust is about structural programing, use a struct is THE way to go for Rust. If you want you can check https://doc.rust-lang.org/std/iter/index.html#structs that have like 30 structs. If you want a struct made for both perf and nice user API there is https://doc.rust-lang.org/std/collections/hash_map/enum.Entry.html or https://doc.rust-lang.org/std/fs/struct.OpenOptions.html.

Most of these are opaque types designed only to carry trait impls such as Iterator or Error. The remainder of transparent types have special functions: std::alloc::Layout has lots of methods for manipulating repr(C) layouts, std::fs::OpenOptions has several OS-dependent methods, std::cmp::Reverse and std::num::Wrapping modify the inner type's impls, etc. What kind of special API would the raw parts have to justify placing them in their own type?

[Stargateur]

@LegionMammal978 Fair would be to say it's just 2, variations doesn't count, align_to and select_nth_unstable are acceptable since there are naturally ordered, like split_at that return (prefix, sufix). Thus I would say my point remain it's pretty RARE and for a good reason, tuple doesn't express anything. It's nice for closure or specific use case but public API should avoid return tuple.

Most of these are opaque types designed only to carry trait impls such as Iterator or Error. The remainder of transparent types have special functions: std::alloc::Layout has lots of methods for manipulating repr(C) layouts, std::fs::OpenOptions has several OS-dependent methods, std::cmp::Reverse and std::num::Wrapping modify the inner type's impls, etc.

My first point was to say add a struct is common. I don't understand the rest of your point look like HS.

What kind of special API would the raw parts have to justify placing them in their own type?

The crate raw-parts clearly show advantage, have a "Raw builder" API for Vec allow to help user avoid mistake using from_vec, better this allow to add a nice into_vec that also remove the potentially mistake of from_raw_parts argument order. (thus raw-parts choice to use associate method for into_vec is weird to me)

This mean a user can get the struct, mutate only what needed, and use it to reconstruct the vec. That a BIG plus when the feature is mean to use unsafe to have every small help you can.

[anayw2001]

Hi everyone, just wanted to ask what the status on this issue was, and if the agreed-upon suggestion of this thread was to use the raw-parts crate for stable rust toolchains.

[christopinka]

upon suggestion of this thread was to use the raw-parts crate for stable rust toolchains.

Seems to work. Don't know what the tradeoffs are.

[LegionMammal978]

In stable Rust, you can safely decompose a Vec without any extra crates by using ManuallyDrop:

use std::mem::ManuallyDrop;

pub fn into_raw_parts<T>(vec: Vec<T>) -> (*mut T, usize, usize) {
    let mut vec = ManuallyDrop::new(vec);
    let length = vec.len();
    let capacity = vec.capacity();
    (vec.as_mut_ptr(), length, capacity)
}

The main downside (aside from verbosity) is that you have to be very careful: after calling vec.as_mut_ptr(), you cannot touch vec in any way, even to pass it to mem::forget(). As a corollary, you must call vec.len() and vec.capacity() before calling vec.as_mut_ptr(). (This is a safe function, so breaking this rule will not immediately cause UB, but any later unsafe code which accesses the pointer or calls Vec::from_raw_parts() will invariably cause UB. Note that this may change to no longer be UB in a future version of the language.)

Removing the need for this incantation is the purpose of the proposed Vec::into_raw_parts() API.

[cmazakas]

I'm sorry, what? UB? Can't touch? Discussing such things as UB or not-UB is out of scope for the issue.

[mina86]

after calling vec.as_mut_ptr(), you cannot touch vec in any way,

as_mut_ptr returns a pointer which doesn’t count as an exclusive borrow so you can continue using vec through a shared reference. It’s only using vec through exclusive reference that may lead to pointer being invalidated (and subsequent undefined behaviour when trying to dereference it).

[LegionMammal978]

I'm sorry, what? UB? Can't touch? Discussing such things as UB or not-UB is out of scope for the issue.

That into_raw_parts() function is what this feature intends to replace. Vec::into_raw_parts() is equivalent to mem::forget() in safe code; its only utility lies in unsafe code which accesses the pointer or calls Vec::from_raw_parts(), and into_raw_parts() must be written very carefully for that unsafe code to work properly.

after calling vec.as_mut_ptr(), you cannot touch vec in any way,

as_mut_ptr returns a pointer which doesn’t count as an exclusive borrow so you can continue using vec through a shared reference. It’s only using vec through exclusive reference that may lead to pointer being invalidated (and subsequent undefined behaviour when trying to dereference it).

The signature of Vec::as_mut_ptr() is pub fn as_mut_ptr(&mut self) -> *mut T. Note the &mut self, which is the problem here, since the *mut T pointer is derived from that &mut self reference and will be invalidated once &mut self is invalidated, if we decide to use noalias Vec in the standard library. Looking a bit closer, it might not be a problem in the particular case of calling len() or capacity(), but it's still very fragile, and would cause issues if we were to call as_mut_ptr() again, or mem::forget(). See #54470, #94421, and rust-lang/unsafe-code-guidelines#326 for more information.

[mina86]

I don’t quite see how what you wrote invalidates what I wrote. So long as you don’t modify vector after taking the pointer, you can do pretty much whatever you want with the vector and the pointer remains valid.

And to put it bluntly, if the following:

let mut vec = vec![1];
let ptr = vec.as_mut_ptr();
let len = vec.size();
core::mem::forget(vec);
println!("{}", unsafe { *ptr });

is undefined behaviour than that’s the language defect that needs to be addressed regardless of into_raw_parts.

[cmazakas]

Yes, I agree. The simple solution is for the stdlib to prevent the returned pointer from carrying noalias.

However, this is outside the scope of the issue because into_raw_parts() and from_raw_parts will continue to work correctly, regardless of what the stdlib decides to do.

Fwiw, Legion is likely alluding to https://llvm.org/docs/LangRef.html#noalias when applied to function return types. Yielding a noalias raw pointer from as_mut_ptr() means that all accesses must be done through the returned pointer or one derived from it.

But again, this isn't what's currently happening today. And it also doesn't really matter for this particular API because the usage is largely the same.

[LegionMammal978]

I don’t quite see how what you wrote invalidates what I wrote. So long as you don’t modify vector after taking the pointer, you can do pretty much whatever you want with the vector and the pointer remains valid.

This is false regardless of Vec's aliasing guarantees. For instance, accessing the pointer, even just to read it, will invalidate any exclusive references to its contents, even if those references are never written to (as can be seen with Miri on the Playground):

fn main() {
    let mut vec = vec![1];
    let ptr = vec.as_mut_ptr();
    let slice = vec.as_mut_slice();
    println!("{}", unsafe { *ptr });
    let _slice = slice; // UB
}

Forming and reborrowing references is always a potentially dangerous operation under Stacked Borrows, when they are mixed with raw pointers.

And to put it bluntly, if the following: [...] is undefined behaviour than that’s the language defect that needs to be addressed regardless of into_raw_parts.

Then take it up with the UCG WG in rust-lang/unsafe-code-guidelines#326 or on Zulip.

However, this is outside the scope of the issue because into_raw_parts() and from_raw_parts will continue to work correctly, regardless of what the stdlib decides to do.

Sure, it's just that someone was asking how to perform the equivalent of Vec::into_raw_parts() in stable Rust, so I gave the solution, and I warned of some pitfalls one might run into while trying to refactor that solution or adapt it to another situation.

[mina86]

I don’t quite see how what you wrote invalidates what I wrote. So long as you don’t modify vector after taking the pointer, you can do pretty much whatever you want with the vector and the pointer remains valid.

This is false regardless of Vec's aliasing guarantees. For instance, accessing the pointer, even just to read it, will invalidate any exclusive references to its contents, even if those references are never written to (as can be seen with Miri on the Playground):

fn main() {
    let mut vec = vec![1];
    let ptr = vec.as_mut_ptr();
    let slice = vec.as_mut_slice();
    println!("{}", unsafe { *ptr });
    let _slice = slice; // UB
}

This is UB because it creates two mutable references to the same object: one through slice and the other through *ptr. The issue isn’t that vec is touched after calling as_mut_ptr.

Forming and reborrowing references is always a potentially dangerous operation under Stacked Borrows, when they are mixed with raw pointers.

Which doesn’t change the fact that after calling vec.as_mut_ptr() you can touch vec in various ways including passing it to core::mem::forget(). Your assessment that you cannot call len or capacity after calling as_mut_ptr is incorrect as far as I can tell and Miri seems to agree.

And to put it bluntly, if the following: [...] is undefined behaviour than that’s the language defect that needs to be addressed regardless of into_raw_parts.

Then take it up with the UCG WG in rust-lang/unsafe-code-guidelines#326 or on Zulip.

Well, Miri doesn’t complain about the code so I don’t think I need to bring it up since the code appears to be sound.

[LegionMammal978]

Well, Miri doesn’t complain about the code so I don’t think I need to bring it up since the code appears to be sound.

That's because the current implementation does not, in fact, implement Vec using Box. But this is not a stable behavior; there have been no promises made yet regarding the aliasing properties of Vec. Miri can only detect immediate language-level UB like invalid pointer accesses, not violations of library rules nor dependency on unstable behavior.

In fact, PR #94421 experimented with making this exact change; unfortunately, it cannot be demonstrated directly, since Miri at the time of the PR did not yet support recursively retagging references in struct fields. But an equivalent issue can be demonstrated on current nightly with a Box<[T]> (Rust Playground), which is roughly what a noalias Vec would contain:

fn main() {
    let mut boxed_slice = vec![1].into_boxed_slice();
    let ptr = boxed_slice.as_mut_ptr();
    std::mem::forget(boxed_slice);
    println!("{}", unsafe { *ptr }); // UB
}

error: Undefined Behavior: attempting a read access using <3158> at alloc1501[0x0], but that tag does not exist in the borrow stack for this location
 --> src/main.rs:5:29
  |
5 |     println!("{}", unsafe { *ptr }); // UB
  |                             ^^^^
  |                             |
  |                             attempting a read access using <3158> at alloc1501[0x0], but that tag does not exist in the borrow stack for this location
  |                             this error occurs as part of an access at alloc1501[0x0..0x4]
  |
  = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
  = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <3158> was created by a SharedReadWrite retag at offsets [0x0..0x4]
 --> src/main.rs:3:15
  |
3 |     let ptr = boxed_slice.as_mut_ptr();
  |               ^^^^^^^^^^^^^^^^^^^^^^^^
help: <3158> was later invalidated at offsets [0x0..0x4] by a Unique retag
 --> src/main.rs:4:22
  |
4 |     std::mem::forget(boxed_slice);
  |                      ^^^^^^^^^^^
  = note: BACKTRACE (of the first span):
  = note: inside `main` at src/main.rs:5:29: 5:33

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

[mina86]

I stand corrected (and baffled at the same time). Thanks.

[cmazakas]

The clear solution is for Vec to never be noalias because otherwise it's terrible.

But this is the tracking issue for into_raw_parts.

The simple solution is this, if you genuinely have trouble remembering it's length then capacity, just write your own helper. There's no need to pollute the stdlib API itself because a caller can't remember off-hand that it's len-cap, not cap-len.

Now, let's stabilize the feature already.

[ChrisDenton]

The discussion above is an argument for stabilizing into_raw_parts. It demonstrates that the function is useful above and beyond doing it manually. And without such a justification the case for stabilizing this would be weaker.

[cmazakas]

The discussion above is an argument for stabilizing into_raw_parts. It demonstrates that the function is useful above and beyond doing it manually. And without such a justification the case for stabilizing this would be weaker.

Whatever it takes to get it stabilized. Now, I need to help the unsafe WG realize they shouldn't ruin Vec like Box.

[tae-soo-kim]

Should the returned pointer be a NonNull instead? Box::raw doesn’t do that because it was stabilized before NonNull was.

No one has ever mentioned NonNull is covariant. There are actually 2 differences between NonNull and *mut: non-null and covariance. I'm not inclined to the current design of NonNull since it loses orthogonality.

That being said, I think it is better to return a non-null pointer in this case. It's just that this should be a RawNonNull which is exactly the same as *mut except being non-null. I don't like the side-effect of making the pointer covariant (as the current NonNull would).

[Jan561]

Why would covariance be an issue here? The returned pointer is unique and represents ownership, in which case covariance should be fine, maybe even desirable. (with the very limited understanding I have on this topic, so take this with a grain of salt)

And I think this is one of the (two) usecases NonNull was originally designed for.

[robertbastian]

There's a somewhat related discussion in #119206 (comment).

[cmazakas]

I don't entirely think NonNull is necessary here. The stdlib already has a precedent of using raw pointers, even when they're intended to be non-null. It's important not to over-engineer and under-deliver. There's already an established convention and users can easily create a NonNull themselves.

[GoldsteinE]

(I’ve discussed this with @shepmaster for a bit before writing this)

Stabilization report

Implementation history

• Initially implemented in Add {String,Vec}::into_raw_parts #65705
• (Will be) moved into the correct impl block in Change {Box,Arc,Rc,Weak}::into_raw to only work with A = Global #141219
• (Will be, if this is accepted) changed to an associated function in Draft: Make into_parts methods on Vec associated functions #141509
• I also added a debug_assert!() assert_unsafe_precondition!() in the same PR that should hopefully catch mixing up length and capacity

API summary

impl<T> Vec<T, Global> {
    pub fn into_raw_parts(vec: Self) -> (*mut T, usize, usize) {…}
}

impl String {
    pub fn into_raw_parts(string: Self) -> (*mut u8, usize, usize) {…}
}

(Proposed) answers to the open questions

• Should they return NonNull<T>? I’d say that they shouldn’t, for these reasons:

  1. Most common use for this method is for “transmuting” the vector, which takes a raw pointer. Other possible uses probably involve doing something with the data behind the pointer, which also doesn’t require NonNull<_>. Primary usecase for NonNull<_> is for Option<_> size optimization, which is probably rarely relevant here.
  2. It’s more consistent with other ::into_raw() functions in std, like Box::into_raw(), Rc::into_raw() and CString::into_raw().
  3. We already have methods returning NonNull<_> as a separate feature: Tracking Issue for box_vec_non_null #130364

• Should they be associated functions rather than methods? I’d argue that yes, they should, for consistency with other similar functions. This also resolves possible confusion on whether this converts the vector itself or the slice it derefs to. I’ve also read the thread and this seems to be a more common opinion, although that’s purely a guesstimate.

• Should they return some struct with named fields instead of a tuple? While possibility for confusing len and capacity is annoying, I’d argue that probability of this mistake not being caught in testing is not that high, especially with assert_unsafe_precondition!(len <= capacity) in Vec::from_raw_parts(). Returning a struct with named fields would be pretty unique for std which does not have a lot of POD types. Based on this, I’d argue that a tuple is fine.

I do not feel strongly about either of these and I’m ready to write a patch to change it if t-libs-api has a different opinion. I’d just like to see this stabilized, because doing this manually is error-prone and can easily lead to UB.

Experience report

Example usages

• In implementation of CString, to “transmute” a vector (note that this already uses associated function syntax, despite this currently being a method).
• In implementation of VecDeque, to convert from a vector. This technically uses .into_raw_parts_with_alloc(), because VecDeque is generic over allocator, but similar code outside of std would obviously use just ::into_raw_parts(), as custom allocators are unstable.
• In implementation of Vec::into_flattened(), to “transmute” a vector. Also techically uses .into_raw_parts_with_alloc(), same reasoning applies.

---

I don’t think I have access to ping the team here, so I would be glad if someone did it for me. I would like to know if someone from t-libs-api feels strongly on either of the open issues, so I can change it if needed and move this forward.

[Stargateur]

I’d argue that probability of this mistake not being caught in testing is not that high

This feature is a helper method to avoid, as_ptr(), len(), capacity(), and so mistake, return a tuple and not named field will make this feature not more safe than call these 3 methods. I don't really see the strong point of "catch in test".

Returning a struct with named fields would be pretty unique for std which does not have a lot of POD types.

That on contrary more and more a thing with modern API added to std.

I repeat myself, but I can only warn that a tuple with two field of same type will cause trouble as a return value of a function that doesn't give any hint about what is what.

[GoldsteinE]

This feature is a helper method to avoid, as_ptr(), len(), capacity(), and so mistake, return a tuple and not named field will make this feature not more safe than call these 3 methods.

It is more safe, as you don’t need to suppress drop, which is easy to do incorrectly (e.g. by using mem::forget() after getting the pointer).

That on contrary more and more a thing with modern API added to std.

Can you name an example of a similar POD type in std?

doesn't give any hint about what is what

It is the first line of its documentation.

In any case, I’m happy to change it to a POD type if t-libs-api feels the same way. As I have said, I really don’t care that much about exact API of this function if it helps to get it stabilized.

[mina86]

Should they return NonNull?

This is a chicken and egg problem. Because NonNull is underutilised in the standard library, no new methods are created to work with NonNull, which leads to NonNull being underutilised.

Primary usecase for NonNull<> is for Option<> size optimization, which is probably rarely relevant here.

No. Primary use case for NonNull is to indicate that the pointer is not null. This allows encoding additional invariants in the type system.

We already have methods returning NonNull<_> as a separate feature:

This isn’t an argument in favour of not returning NonNull. We should have this xor the other feature. Not both of them.

Having said that, I don’t have strong opinions here. I would like to more use of NonNull (and similarly better support for NonZero* but that’s a completely separate discussion), but I also see arguments for consistency with existing status quo.

I’d argue that yes, they should, for consistency with other similar functions.

What functions are you referring to? Other *to_raw_parts symbols in standard library are methods. In stable land, there are methods like String::into_bytes or PathBuf::into_os_string. None of them are associated functions. The consistent thing would be for any into_* symbols to be methods which take self and convert it into a different representation.

Should they return some struct with named fields instead of a tuple?

For me, ergonomics of a tuple is just much better at the moment. I really don’t want to write let std::vec::VecParts { ptr, len, capacity } = blahs.into_raw_parts().

[shepmaster]

don’t think I have access to ping the team here, so I would be glad if someone did it for me

@rust-lang/libs-api