`uninit_vector` should return `Vec<MaybeUninit<T>>`

[plafer]

The problem

The current implementation of uninit_vector() causes undefined behavior, and is especially dangerous to use with types that manage resources (and thus implement Drop) such as Box or Arc.

This is due to how the API forces you to initialize the vector:

let v: Vec<Box<T>> = uninit_vector(10);

for idx in 0..10 {
  // Rust thinks that `v` is initialized, and so drops the previous value stored at `v[idx]`
  // which in our case is uninitialized, and hence causes undefined behavior.
  v[idx] = Box::new(T);
}

You can test this for yourself with the following snippet

pub struct MyStruct(u32);

impl Drop for MyStruct {
    fn drop(&mut self) {
        std::println!("MyStruct is being dropped, with value {}", self.0);
    }
}

// Output:
// before assignment
// MyStruct is being dropped, with value <undefined; depends on your system/environment>
// after assignment
// MyStruct is being dropped, with value 42
fn main() {
    let mut v: Vec<MyStruct> = unsafe { uninit_vector(1) };

    std::println!("before assignment");
    v[0] = MyStruct(42);
    std::println!("after assignment");
}

The first call the MyStruct::drop() is due to the assignment dropping the former (uninitialized) value in the vector; the second call happens at the end of main() when v is dropped (and all values in the vector).

We probably never noticed anything bad happening before, since the main use of uninit_vector is with Field types that don't implement Drop. However, using it with any type still causes undefined behavior and we should move away from it.

The solution

The solution is to use MaybeUninit which was designed for this use case. So we should mark uninit_vector as deprecated and provide a new

pub unsafe fn uninit_vector2<T>(length: usize) -> Vec<core::mem::MaybeUninit<T>> {
    let mut vector = Vec::with_capacity(length);
    vector.set_len(length);
    vector
}

Then callers use MaybeUninit::write() to set the value (which very importantly does not call drop() on the old value as the assignment operator does). The previous broken example now becomes

// Output:
// before assignment
// after assignment
// MyStruct is being dropped, with value 42
fn main() {
    let v: Vec<MyStruct> = {
        let mut v: Vec<MaybeUninit<MyStruct>> = unsafe { uninit_vector2(1) };

        std::println!("before assignment");
        v[0].write(MyStruct(42));
        std::println!("after assignment");

        // SAFETY: We have initialized all values in the vector, so it is safe to transmute.
        unsafe { core::mem::transmute(v) }
    };
}

Note that using transmute() is the documented way of doing this.

[Mirko-von-Leipzig]

Is there a point in keeping this function at all? Users should just use the built-in type as appropriate,

unsafe fn uninit_vector2<T>(length: usize) -> Vec<MaybeUninit<T>> {..}

// Is just this
vec![MaybeUninit::<T>::uninit(); length]

[plafer]

Indeed, if vec![MaybeUninit::<T>::uninit(); length] is equivalent (which I would like to benchmark to confirm), then we should probably just remove the function entirely

[plafer]

After further research and offline discussions, there are a few learnings.

First, my uninit_vector2 is also incorrect, since Vec::set_len() requires the elements to be initialized. My confusion was that using MaybeUninit in Vec<MaybeUninit<T>> followed by Vec::set_len() doesn't mean that the memory state is then properly modeled as "maybe uninitialized"; it is instead still just plain uninitialized.

There seem to be 2 proper ways to do this:

1. Using vec![MaybeUninit::<T>::uninit(); length] as suggested by @Mirko-von-Leipzig, or
2. using Box::new_uninit_slice followed by Box::assume_init() (and Vec::from(box) if we really need a Vec), as suggested by @bitwalker.

I benchmarked all the discussed options here. Naturally, the only options to actually consider are using the "traditional" Vec::with_capacity() followed by Vec::push(), or one of the 2 methods above. Running the above benchmarks on my machine gives:

• Box::new_uninit_slice(): 4.65ms
• vec![MaybeUninit::<T>::uninit(); length]: 4.75ms
• Vec::with_capacity(): 9.8ms

So both MaybeUninit-based approaches seem viable (with Box::new_uninit_slice() potentially a bit faster), and are indeed about 2x faster than running Vec::push() in a loop