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

Kill array_assume_init #103134

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion library/alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@
#![feature(iter_advance_by)]
#![feature(iter_next_chunk)]
#![feature(layout_for_ptr)]
#![feature(maybe_uninit_array_assume_init)]
#![feature(maybe_uninit_slice)]
#![feature(maybe_uninit_uninit_array)]
#![feature(maybe_uninit_uninit_array_transpose)]
#![cfg_attr(test, feature(new_uninit))]
#![feature(nonnull_slice_from_raw_parts)]
#![feature(pattern)]
Expand Down
4 changes: 2 additions & 2 deletions library/alloc/src/vec/into_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {

self.ptr = self.ptr.wrapping_byte_add(N);
// Safety: ditto
return Ok(unsafe { MaybeUninit::array_assume_init(raw_ary) });
return Ok(unsafe { raw_ary.transpose().assume_init() });
}

if len < N {
Expand All @@ -241,7 +241,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
return unsafe {
ptr::copy_nonoverlapping(self.ptr, raw_ary.as_mut_ptr() as *mut T, N);
self.ptr = self.ptr.add(N);
Ok(MaybeUninit::array_assume_init(raw_ary))
Ok(raw_ary.transpose().assume_init())
};
}

Expand Down
5 changes: 2 additions & 3 deletions library/core/src/array/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,7 @@ impl<T, const N: usize> IntoIter<T, N> {
///
/// ```
/// #![feature(array_into_iter_constructors)]
///
/// #![feature(maybe_uninit_array_assume_init)]
/// #![feature(maybe_uninit_uninit_array_transpose)]
/// #![feature(maybe_uninit_uninit_array)]
/// use std::array::IntoIter;
/// use std::mem::MaybeUninit;
Expand Down Expand Up @@ -134,7 +133,7 @@ impl<T, const N: usize> IntoIter<T, N> {
/// }
///
/// // SAFETY: We've initialized all N items
/// unsafe { Ok(MaybeUninit::array_assume_init(buffer)) }
/// unsafe { Ok(buffer.transpose().assume_init()) }
/// }
///
/// let r: [_; 4] = next_chunk(&mut (10..16)).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ where

mem::forget(guard);
// SAFETY: All elements of the array were populated in the loop above.
let output = unsafe { MaybeUninit::array_assume_init(array) };
let output = unsafe { array.transpose().assume_init() };
Ok(Try::from_output(output))
}

Expand Down
46 changes: 0 additions & 46 deletions library/core/src/mem/maybe_uninit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -910,52 +910,6 @@ impl<T> MaybeUninit<T> {
}
}

/// Extracts the values from an array of `MaybeUninit` containers.
///
/// # Safety
///
/// It is up to the caller to guarantee that all elements of the array are
/// in an initialized state.
///
/// # Examples
///
/// ```
/// #![feature(maybe_uninit_uninit_array)]
/// #![feature(maybe_uninit_array_assume_init)]
/// use std::mem::MaybeUninit;
///
/// let mut array: [MaybeUninit<i32>; 3] = MaybeUninit::uninit_array();
/// array[0].write(0);
/// array[1].write(1);
/// array[2].write(2);
///
/// // SAFETY: Now safe as we initialised all elements
/// let array = unsafe {
/// MaybeUninit::array_assume_init(array)
/// };
///
/// assert_eq!(array, [0, 1, 2]);
/// ```
#[unstable(feature = "maybe_uninit_array_assume_init", issue = "96097")]
#[rustc_const_unstable(feature = "const_maybe_uninit_array_assume_init", issue = "96097")]
#[inline(always)]
#[track_caller]
pub const unsafe fn array_assume_init<const N: usize>(array: [Self; N]) -> [T; N] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I've mentioned in some other PRs, I'm not willing to take an API removal (even an unstable one) without libs-api team oversight.

Up to you if you want to bring this up in an ACP to see if T-libs-api agrees, or if you want to make this just a T-libs change that updates the implementations of things to use transpose without actually removing things.

@rustbot author

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, I want to get rid of these. Created rust-lang/libs-team#122

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this is probably what you were trying to tell me but I think it'll be easier to split stdlib usage removal from api removal, so I'll pull out the usage removals.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, it's always easiest to split things up by decision makers.

Such as, where possible:

  • changes to compiler separate from library
  • additions separate from removals
  • internal details separate from anything impacting callers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha yeah, thanks. I feel like I've learned this lesson at least a dozen times by now, but the initial ease of piling a bunch of crap together is too good a fool. :)

// SAFETY:
// * The caller guarantees that all elements of the array are initialized
// * `MaybeUninit<T>` and T are guaranteed to have the same layout
// * `MaybeUninit` does not drop, so there are no double-frees
// And thus the conversion is safe
let ret = unsafe {
intrinsics::assert_inhabited::<[T; N]>();
scottmcm marked this conversation as resolved.
Show resolved Hide resolved
(&array as *const _ as *const [T; N]).read()
};

// FIXME: required to avoid `~const Destruct` bound
super::forget(array);
ret
}

/// Assuming all the elements are initialized, get a slice to them.
///
/// # Safety
Expand Down
2 changes: 1 addition & 1 deletion library/core/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@
#![feature(slice_from_ptr_range)]
#![feature(split_as_slice)]
#![feature(maybe_uninit_uninit_array)]
#![feature(maybe_uninit_array_assume_init)]
#![feature(maybe_uninit_write_slice)]
#![feature(maybe_uninit_uninit_array_transpose)]
#![feature(min_specialization)]
#![feature(numfmt)]
#![feature(step_trait)]
Expand Down
6 changes: 3 additions & 3 deletions library/core/tests/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,18 +163,18 @@ fn assume_init_good() {

#[test]
fn uninit_array_assume_init() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an equivalent test for transpose already?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe? There are doc tests, but again transpose is NOT assume_init so I'm not sure what we'd be testing (that transmute works?).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd be testing that this same behavior still works with the new methods, which sounds useful to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, added back.

let mut array: [MaybeUninit<i16>; 5] = MaybeUninit::uninit_array();
let mut array = [MaybeUninit::<i16>::uninit(); 5];
array[0].write(3);
array[1].write(1);
array[2].write(4);
array[3].write(1);
array[4].write(5);

let array = unsafe { MaybeUninit::array_assume_init(array) };
let array = unsafe { array.transpose().assume_init() };

assert_eq!(array, [3, 1, 4, 1, 5]);

let [] = unsafe { MaybeUninit::<!>::array_assume_init([]) };
let [] = unsafe { [MaybeUninit::<!>::uninit(); 0].transpose().assume_init() };
}

#[test]
Expand Down