rust icon indicating copy to clipboard operation
rust copied to clipboard

Tracking issue for `#![feature(maybe_uninit_slice)]`

Open Centril opened this issue 5 years ago • 18 comments

This is a tracking issue for the RFC "Deprecate uninitialized in favor of a new MaybeUninit type" (rust-lang/rfcs#1892).

This issue specifically tracks the following unstable methods:

impl<T> MaybeUninit<T> {
    pub fn first_ptr(this: &[MaybeUninit<T>]) -> *const T { ... }
    pub fn first_ptr_mut(this: &mut [MaybeUninit<T>]) -> *mut T { ... }
}

Open questions:

  • [ ] Better names? The methods correspond to slice::as_ptr and slice::as_mut_ptr, so maybe they should be called slice_as_ptr and slice_as_mut_ptr? That also better conveys that the raw ptr may indeed be used for the entire slice, and that the methods may be called on empty slices.

Centril avatar Aug 14 '19 18:08 Centril

Note: https://github.com/rust-lang/rust/issues/63291 contains some APIs for creating uninitialized slices, but still does not have a good story for working with them, AFAIK.

Cc @SimonSapin

RalfJung avatar Aug 14 '19 18:08 RalfJung

I think another missing function is a way to create a slice of uninitialized data from a Vec. Like this:

impl<'a, T> Vec<T> {
    pub fn as_uninit(&'a mut self) -> &'a mut [MaybeUninit<T>] {...}
}

Which would be used like this:

let v = Vec::<u8>::with_capacity(100);
let mut sl = v.as_uninit();
if 0 != some_ffi_func(sl.first_ptr_mut(), sl.len()) {
    return Err<()>;
} else {
    v.set_len(sl.len());
    return Ok(v);
}

There's no need for Vec::uninit() or Vec::assume_init(), because Vec::with_capacity and Vec::set_len already do pretty much the same thing. And for simple cases like this one Vec::as_mut_ptr() would suffice without the formality of creating a slice of MaybeUninits. However, the slice has the advantage that it knows its own length and it knows the lifetime of its data. The latter is especially useful when working with FFI functions that initialize the data asynchronously, such as aio_read.

asomers avatar Sep 01 '19 05:09 asomers

Cc @Shnatsel see above -- seems related to https://github.com/rust-lang/rust-clippy/issues/4483

RalfJung avatar Sep 01 '19 20:09 RalfJung

@asomers I agree that something like this would be good to have. However your proposal seems ambiguous: does the returned slice include the entire allocation (indices 0..capacity) or only the range that is not known to be initialized (indices len..capacity)?

Would an API that returns two disjoint (&mut [T], &mut [MaybeUninit<T>]) slices for indices 0..len and len..capacity also be useful?

SimonSapin avatar Sep 02 '19 14:09 SimonSapin

Back to the APIs tracked in this issue, could they be methods of [MaybeUninit<T>] that take &self instead of associated functions of MaybeUninit<T>? (Though it’s not obvious what they should be called in that case.)

We have precedent for a lang item for impl [u8] {…}

SimonSapin avatar Sep 02 '19 14:09 SimonSapin

Good questions @SimonSapin. I think that the returned slice should include the entire allocation (after all, elements that are already initialized can still be represented as MaybeUninit. Also, I realized that the method I proposed isn't even good enough for my particular use case. I need a way to create an owned, maybe-uninitialized slice in a known memory location, and then transform that into an initialized vec. This is important for Tokio, whose asynchronous nature can thwart the borrow-checker.

impl<T> Vec<MaybeUninit<T>> {
    /// Transmute this `Vec` of possibly uninitialized data into a normal `Vec`, assuming that the
    /// first `len` elements are properly initialized.
    pub unsafe fn assume_init(self, len: usize) -> Vec<T> {...}
}
/// I'm not 100% sure it's necessary, but define this for symmetry's sake:
impl<T> Vec<T> {
    /// Transmute this `Vec` into a `Vec` of possibly uninitialized data whose length is equal to
    /// the original `Vec`'s capacity.
    pub fn into_uninit(self) -> Vec<MaybeUninit<T>> {...}
}

And it would be used like this:

let v_uninit = Vec::<MaybeUninit<u8>>::with_capacity(1024);
let result = some_ffi_func(v_uninit.as_mut_ptr(), v_uninit.capacity());
match result {
    Ok(len) => Ok(v_uninit.assume_init(len)),
    Err(e) => Err(e)
}

Also, regarding the original methods, should the return type be Option<NonNull<T>>? That way they could return None for 0-length slices.

asomers avatar Sep 03 '19 15:09 asomers

I need a way to create an owned, maybe-uninitialized slice in a known memory location

That’s Box::new_uninit https://github.com/rust-lang/rust/issues/63291

SimonSapin avatar Sep 03 '19 16:09 SimonSapin

Why would you use Vec<MaybeUninit<T>>? Vec already encapsulates work with uninitialized memory - you can write to its capacity and then .set_len() to update the length. Or just use one of the myriad of methods that append to it safely.

Shnatsel avatar Sep 03 '19 17:09 Shnatsel

@Shnatsel because I need something that owns uninitialized memory, unlike a *mut u8. I'm using aio_read, which asynchronously initializes the memory. There's no way for me to supply a &mut Vec reference to aio_read and persuade the borrow checker that the Vec will still be around by the time that the operation is complete (with notification delivered via kqueue). Making matters more complicated is the fact that my stack is 5 crates deep.

Another option might be if [T] implemented BorrowMut<[MaybeUninit<T>]>. Then I think I could make things work.

asomers avatar Sep 03 '19 18:09 asomers

Ah, looks like you want aio_read to actually own the Vec instead of getting a mutable reference to it. This has already been brought up in relation to design of readers in async_std.

Shnatsel avatar Sep 03 '19 18:09 Shnatsel

Is there any reason these are not directly implemented on the slice?

impl<T> [MaybeUninit<T>] {
    pub fn first_ptr(&self) -> *const T { ... }
    pub fn first_ptr_mut(&mut self) -> *mut T { ... }
}

This would allow slice.first_ptr() instead of std::mem::MaybeUninit::first_ptr(&mut slice).

krsnik02 avatar Feb 12 '20 22:02 krsnik02

As far as I know, such inherent impls for primitive types don't actually work -- and making them work requires some magic to be added inside the compiler. I am not sure though.

RalfJung avatar Feb 14 '20 08:02 RalfJung

Yes, trait coherence rules would normally require this impl to be in the crate that defines the [_] slice type, but that is no crate. It is possible to basically change the rules of the language and make an exception, we do that here for [u8] for methods that were originally on the std::ascii::AsciiExt trait but that we later chose to make inherent:

https://github.com/rust-lang/rust/blob/21ed50522ddb998f5367229984a4510af578899f/src/libcore/slice/mod.rs#L2592-L2594

So it’s technically possible and there is precedent, but I don’t know if the Lang team is keen on expanding that pattern.

SimonSapin avatar Feb 14 '20 10:02 SimonSapin

The language change https://blog.rust-lang.org/2020/01/30/Rust-1.41.0.html#relaxed-restrictions-when-implementing-traits doesn’t apply to inherent methods, does it?

SimonSapin avatar Feb 14 '20 10:02 SimonSapin

I've just submitted PR #69319 to implement From<&[T]> for NonNull<T>, but what would also be useful to me is From<&[MaybeUninit<T>]> for NonNull<T>. That could even be a substitute for MaybeUninit::first_ptr_mut: something like NonNull::<T>::from(slice_of_uninit).as_ptr(). It wouldn't be a substitute for MaybeUninit::first_ptr, but I think that method seems less useful: when I want a pointer to uninitialized, it is to pass it to some function that initializes it.

tspiteri avatar Feb 20 '20 19:02 tspiteri

The MaybeUninit::as_mut_ptr documentation currently warns:

Reading from this pointer or turning it into a reference is undefined behavior unless the MaybeUninit<T> is initialized.

I suppose this applies to slice_as_mut_ptr as well, but there is no such warning in its documentation currently, so please consider adding it.

ttencate avatar Aug 08 '21 09:08 ttencate

Why do the slice_as[_mut]_ptr methods exist? I'm poking through MaybeUninit's API to see if anything else can be simplified, and those two methods seem to just be hiding a simple as *{const,mut} T cast.

SUPERCILEX avatar Sep 01 '22 04:09 SUPERCILEX

Yeah those methods are objectively bad. Nuke: #103133

SUPERCILEX avatar Oct 17 '22 03:10 SUPERCILEX

Any update on this feature?

ForrestOfBarnes avatar May 06 '23 00:05 ForrestOfBarnes

It's stalled because we wish the language could generalize over containers: https://github.com/rust-lang/libs-team/issues/122. Depending on how long that takes, it might make sense to stabilize the slice methods anyway, but these features haven't been discussed under the assumption that container generalization exists. That's probably the next step: see what makes sense to stabilize even if we get language features that makes some of the methods unnecessary. Then we can evaluate the tradeoffs of adding methods that will probably be pointless in the future but are useful now.

SUPERCILEX avatar May 06 '23 01:05 SUPERCILEX

Ahhh, I was just thinking it would be nice to implement this for Vec too. Anyone who really needs this functionality (might be me, idk) could just copy that one line of source code, so it probably makes sense to wait for the cleaner, container-based implementation. 👍

ForrestOfBarnes avatar May 06 '23 01:05 ForrestOfBarnes