Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task
Centril opened this issue Aug 14, 2019 · 21 comments
Open
1 task

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

Centril opened this issue Aug 14, 2019 · 21 comments
Labels
A-raw-pointers Area: raw pointers, MaybeUninit, NonNull A-slice Area: [T] B-unstable Feature: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. requires-nightly This issue requires a nightly compiler in some way. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Centril
Copy link
Contributor

Centril commented Aug 14, 2019

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 Centril added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Feature: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. requires-nightly This issue requires a nightly compiler in some way. labels Aug 14, 2019
@RalfJung
Copy link
Member

Note: #63291 contains some APIs for creating uninitialized slices, but still does not have a good story for working with them, AFAIK.

Cc @SimonSapin

Centril added a commit to Centril/rust that referenced this issue Aug 14, 2019
Centril added a commit to Centril/rust that referenced this issue Aug 14, 2019
@asomers
Copy link
Contributor

asomers commented Sep 1, 2019

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.

@RalfJung
Copy link
Member

RalfJung commented Sep 1, 2019

Cc @Shnatsel see above -- seems related to rust-lang/rust-clippy#4483

@SimonSapin
Copy link
Contributor

@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
Copy link
Contributor

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] {…}

@asomers
Copy link
Contributor

asomers commented Sep 3, 2019

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.

@SimonSapin
Copy link
Contributor

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

That’s Box::new_uninit #63291

@Shnatsel
Copy link
Member

Shnatsel commented Sep 3, 2019

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.

@asomers
Copy link
Contributor

asomers commented Sep 3, 2019

@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.

@Shnatsel
Copy link
Member

Shnatsel commented Sep 3, 2019

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.

@krsnik02
Copy link

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).

@RalfJung
Copy link
Member

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.

@SimonSapin
Copy link
Contributor

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:

rust/src/libcore/slice/mod.rs

Lines 2592 to 2594 in 21ed505

#[lang = "slice_u8"]
#[cfg(not(test))]
impl [u8] {

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
Copy link
Contributor

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?

@tspiteri
Copy link
Contributor

tspiteri commented Feb 20, 2020

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.

@KodrAus KodrAus added Libs-Tracked Libs issues that are tracked on the team's project board. A-raw-pointers Area: raw pointers, MaybeUninit, NonNull A-slice Area: [T] labels Jul 30, 2020
matklad added a commit to matklad/rust that referenced this issue Sep 4, 2020
rename MaybeUninit slice methods

The `first` methods conceptually point to the whole slice, not just its first element, so rename them to be consistent with the raw ptr methods on ref-slices.

Also, do the equivalent of rust-lang#76047 for the slice reference getters, and make them part of rust-lang#63569 (so far they somehow had no tracking issue).

* first_ptr -> slice_as_ptr
* first_ptr_mut -> slice_as_mut_ptr
* slice_get_ref -> slice_assume_init_ref
* slice_get_mut -> slice_assume_init_mut
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 5, 2020
rename MaybeUninit slice methods

The `first` methods conceptually point to the whole slice, not just its first element, so rename them to be consistent with the raw ptr methods on ref-slices.

Also, do the equivalent of rust-lang#76047 for the slice reference getters, and make them part of rust-lang#63569 (so far they somehow had no tracking issue).

* first_ptr -> slice_as_ptr
* first_ptr_mut -> slice_as_mut_ptr
* slice_get_ref -> slice_assume_init_ref
* slice_get_mut -> slice_assume_init_mut
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 5, 2020
rename MaybeUninit slice methods

The `first` methods conceptually point to the whole slice, not just its first element, so rename them to be consistent with the raw ptr methods on ref-slices.

Also, do the equivalent of rust-lang#76047 for the slice reference getters, and make them part of rust-lang#63569 (so far they somehow had no tracking issue).

* first_ptr -> slice_as_ptr
* first_ptr_mut -> slice_as_mut_ptr
* slice_get_ref -> slice_assume_init_ref
* slice_get_mut -> slice_assume_init_mut
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 5, 2020
rename MaybeUninit slice methods

The `first` methods conceptually point to the whole slice, not just its first element, so rename them to be consistent with the raw ptr methods on ref-slices.

Also, do the equivalent of rust-lang#76047 for the slice reference getters, and make them part of rust-lang#63569 (so far they somehow had no tracking issue).

* first_ptr -> slice_as_ptr
* first_ptr_mut -> slice_as_mut_ptr
* slice_get_ref -> slice_assume_init_ref
* slice_get_mut -> slice_assume_init_mut
@ttencate
Copy link

ttencate commented Aug 8, 2021

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.

@SUPERCILEX
Copy link
Contributor

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
Copy link
Contributor

Yeah those methods are objectively bad. Nuke: #103133

@ForrestOfBarnes
Copy link
Contributor

Any update on this feature?

@SUPERCILEX
Copy link
Contributor

It's stalled because we wish the language could generalize over containers: rust-lang/libs-team#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.

@ForrestOfBarnes
Copy link
Contributor

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. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-raw-pointers Area: raw pointers, MaybeUninit, NonNull A-slice Area: [T] B-unstable Feature: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. requires-nightly This issue requires a nightly compiler in some way. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests