Merge pull request #902 from rust-ndarray/better-uninit

A better `Array::uninit` and deprecate `Array::uninitialized`

Author: bluss

.github/workflows/ci.yml

@@ -20,7 +20,7 @@ jobs:
           - stable
           - beta
           - nightly
-          - 1.42.0  # MSRV
+          - 1.49.0  # MSRV
 
     steps:
       - uses: actions/checkout@v2

benches/bench1.rs

@@ -271,7 +271,7 @@ fn add_2d_alloc_zip_uninit(bench: &mut test::Bencher) {
     let a = Array::<i32, _>::zeros((ADD2DSZ, ADD2DSZ));
     let b = Array::<i32, _>::zeros((ADD2DSZ, ADD2DSZ));
     bench.iter(|| unsafe {
-        let mut c = Array::<MaybeUninit<i32>, _>::maybe_uninit(a.dim());
+        let mut c = Array::<i32, _>::uninit(a.dim());
         azip!((&a in &a, &b in &b, c in c.raw_view_mut().cast::<i32>())
             c.write(a + b)
         );

examples/sort-axis.rs

@@ -102,7 +102,7 @@ where
         assert_eq!(axis_len, perm.indices.len());
         debug_assert!(perm.correct());
 
-        let mut result = Array::maybe_uninit(self.dim());
+        let mut result = Array::uninit(self.dim());
 
         unsafe {
             // logically move ownership of all elements from self into result

src/data_traits.rs

@@ -9,7 +9,9 @@
 //! The data (inner representation) traits for ndarray
 
 use rawpointer::PointerExt;
+
 use std::mem::{self, size_of};
+use std::mem::MaybeUninit;
 use std::ptr::NonNull;
 use alloc::sync::Arc;
 use alloc::vec::Vec;
@@ -400,7 +402,18 @@ unsafe impl<'a, A> DataMut for ViewRepr<&'a mut A> {}
 /// A representation that is a unique or shared owner of its data.
 ///
 /// ***Internal trait, see `Data`.***
+// The owned storage represents the ownership and allocation of the array's elements.
+// The storage may be unique or shared ownership style; it must be an aliasable owner
+// (permit aliasing pointers, such as our separate array head pointer).
+//
+// The array storage must be initially mutable - copy on write arrays may require copying for
+// unsharing storage before mutating it. The initially allocated storage must be mutable so
+// that it can be mutated directly - through .raw_view_mut_unchecked() - for initialization.
 pub unsafe trait DataOwned: Data {
+    /// Corresponding owned data with MaybeUninit elements
+    type MaybeUninit: DataOwned<Elem = MaybeUninit<Self::Elem>>
+        + RawDataSubst<Self::Elem, Output=Self>;
+
     #[doc(hidden)]
     fn new(elements: Vec<Self::Elem>) -> Self;
 
@@ -421,6 +434,8 @@ unsafe impl<A> DataShared for OwnedArcRepr<A> {}
 unsafe impl<'a, A> DataShared for ViewRepr<&'a A> {}
 
 unsafe impl<A> DataOwned for OwnedRepr<A> {
+    type MaybeUninit = OwnedRepr<MaybeUninit<A>>;
+
     fn new(elements: Vec<A>) -> Self {
         OwnedRepr::from(elements)
     }
@@ -430,6 +445,8 @@ unsafe impl<A> DataOwned for OwnedRepr<A> {
 }
 
 unsafe impl<A> DataOwned for OwnedArcRepr<A> {
+    type MaybeUninit = OwnedArcRepr<MaybeUninit<A>>;
+
     fn new(elements: Vec<A>) -> Self {
         OwnedArcRepr(Arc::new(OwnedRepr::from(elements)))
     }

src/impl_constructors.rs

@@ -478,47 +478,6 @@ where
         }
     }
 
-    /// Create an array with uninitalized elements, shape `shape`.
-    ///
-    /// Prefer to use [`maybe_uninit()`](ArrayBase::maybe_uninit) if possible, because it is
-    /// easier to use correctly.
-    ///
-    /// **Panics** if the number of elements in `shape` would overflow isize.
-    ///
-    /// ### Safety
-    ///
-    /// Accessing uninitalized values is undefined behaviour. You must overwrite *all* the elements
-    /// in the array after it is created; for example using
-    /// [`raw_view_mut`](ArrayBase::raw_view_mut) or other low-level element access.
-    ///
-    /// The contents of the array is indeterminate before initialization and it
-    /// is an error to perform operations that use the previous values. For
-    /// example it would not be legal to use `a += 1.;` on such an array.
-    ///
-    /// This constructor is limited to elements where `A: Copy` (no destructors)
-    /// to avoid users shooting themselves too hard in the foot.
-    ///
-    /// (Also note that the constructors `from_shape_vec` and
-    /// `from_shape_vec_unchecked` allow the user yet more control, in the sense
-    /// that Arrays can be created from arbitrary vectors.)
-    pub unsafe fn uninitialized<Sh>(shape: Sh) -> Self
-    where
-        A: Copy,
-        Sh: ShapeBuilder<Dim = D>,
-    {
-        let shape = shape.into_shape();
-        let size = size_of_shape_checked_unwrap!(&shape.dim);
-        let mut v = Vec::with_capacity(size);
-        v.set_len(size);
-        Self::from_shape_vec_unchecked(shape, v)
-    }
-}
-
-impl<S, A, D> ArrayBase<S, D>
-where
-    S: DataOwned<Elem = MaybeUninit<A>>,
-    D: Dimension,
-{
     /// Create an array with uninitalized elements, shape `shape`.
     ///
     /// The uninitialized elements of type `A` are represented by the type `MaybeUninit<A>`,
@@ -550,7 +509,7 @@ where
     ///
     /// fn shift_by_two(a: &Array2<f32>) -> Array2<f32> {
     ///     // create an uninitialized array
-    ///     let mut b = Array2::maybe_uninit(a.dim());
+    ///     let mut b = Array2::uninit(a.dim());
     ///
     ///     // two first columns in b are two last in a
     ///     // rest of columns in b are the initial columns in a
@@ -580,6 +539,100 @@ where
     ///
     /// # shift_by_two(&Array2::zeros((8, 8)));
     /// ```
+    pub fn uninit<Sh>(shape: Sh) -> ArrayBase<S::MaybeUninit, D>
+    where
+        Sh: ShapeBuilder<Dim = D>,
+    {
+        unsafe {
+            let shape = shape.into_shape();
+            let size = size_of_shape_checked_unwrap!(&shape.dim);
+            let mut v = Vec::with_capacity(size);
+            v.set_len(size);
+            ArrayBase::from_shape_vec_unchecked(shape, v)
+        }
+    }
+
+    /// Create an array with uninitalized elements, shape `shape`.
+    ///
+    /// The uninitialized elements of type `A` are represented by the type `MaybeUninit<A>`,
+    /// an easier way to handle uninit values correctly.
+    ///
+    /// The `builder` closure gets unshared access to the array through a raw view
+    /// and can use it to modify the array before it is returned. This allows initializing
+    /// the array for any owned array type (avoiding clone requirements for copy-on-write,
+    /// because the array is unshared when initially created).
+    ///
+    /// Only *when* the array is completely initialized with valid elements, can it be
+    /// converted to an array of `A` elements using [`.assume_init()`].
+    ///
+    /// **Panics** if the number of elements in `shape` would overflow isize.
+    ///
+    /// ### Safety
+    ///
+    /// The whole of the array must be initialized before it is converted
+    /// using [`.assume_init()`] or otherwise traversed.
+    ///
+    pub(crate) fn build_uninit<Sh, F>(shape: Sh, builder: F) -> ArrayBase<S::MaybeUninit, D>
+    where
+        Sh: ShapeBuilder<Dim = D>,
+        F: FnOnce(RawArrayViewMut<MaybeUninit<A>, D>),
+    {
+        let mut array = Self::uninit(shape);
+        // Safe because: the array is unshared here
+        unsafe {
+            builder(array.raw_view_mut_unchecked());
+        }
+        array
+    }
+
+    #[deprecated(note = "This method is hard to use correctly. Use `uninit` instead.",
+                 since = "0.15.0")]
+    /// Create an array with uninitalized elements, shape `shape`.
+    ///
+    /// Prefer to use [`uninit()`](ArrayBase::uninit) if possible, because it is
+    /// easier to use correctly.
+    ///
+    /// **Panics** if the number of elements in `shape` would overflow isize.
+    ///
+    /// ### Safety
+    ///
+    /// Accessing uninitalized values is undefined behaviour. You must overwrite *all* the elements
+    /// in the array after it is created; for example using
+    /// [`raw_view_mut`](ArrayBase::raw_view_mut) or other low-level element access.
+    ///
+    /// The contents of the array is indeterminate before initialization and it
+    /// is an error to perform operations that use the previous values. For
+    /// example it would not be legal to use `a += 1.;` on such an array.
+    ///
+    /// This constructor is limited to elements where `A: Copy` (no destructors)
+    /// to avoid users shooting themselves too hard in the foot.
+    ///
+    /// (Also note that the constructors `from_shape_vec` and
+    /// `from_shape_vec_unchecked` allow the user yet more control, in the sense
+    /// that Arrays can be created from arbitrary vectors.)
+    pub unsafe fn uninitialized<Sh>(shape: Sh) -> Self
+    where
+        A: Copy,
+        Sh: ShapeBuilder<Dim = D>,
+    {
+        let shape = shape.into_shape();
+        let size = size_of_shape_checked_unwrap!(&shape.dim);
+        let mut v = Vec::with_capacity(size);
+        v.set_len(size);
+        Self::from_shape_vec_unchecked(shape, v)
+    }
+
+}
+
+impl<S, A, D> ArrayBase<S, D>
+where
+    S: DataOwned<Elem = MaybeUninit<A>>,
+    D: Dimension,
+{
+    /// Create an array with uninitalized elements, shape `shape`.
+    ///
+    /// This method has been renamed to `uninit`
+    #[deprecated(note = "Renamed to `uninit`", since = "0.15.0")]
     pub fn maybe_uninit<Sh>(shape: Sh) -> Self
     where
         Sh: ShapeBuilder<Dim = D>,

src/impl_methods.rs

@@ -1383,6 +1383,17 @@ where
         unsafe { RawArrayViewMut::new(self.ptr, self.dim.clone(), self.strides.clone()) }
     }
 
+    /// Return a raw mutable view of the array.
+    ///
+    /// Safety: The caller must ensure that the owned array is unshared when this is called
+    #[inline]
+    pub(crate) unsafe fn raw_view_mut_unchecked(&mut self) -> RawArrayViewMut<A, D>
+    where
+        S: DataOwned,
+    {
+        RawArrayViewMut::new(self.ptr, self.dim.clone(), self.strides.clone())
+    }
+
     /// Return the array’s data as a slice, if it is contiguous and in standard order.
     /// Return `None` otherwise.
     ///

src/lib.rs

@@ -62,7 +62,7 @@
 //!     needs matching memory layout to be efficient (with some exceptions).
 //!   + Efficient floating point matrix multiplication even for very large
 //!     matrices; can optionally use BLAS to improve it further.
-//! - **Requires Rust 1.42 or later**
+//! - **Requires Rust 1.49 or later**
 //!
 //! ## Crate Feature Flags
 //!

src/linalg/impl_linalg.rs

@@ -326,7 +326,7 @@ where
 
         // Avoid initializing the memory in vec -- set it during iteration
         unsafe {
-            let mut c = Array1::maybe_uninit(m);
+            let mut c = Array1::uninit(m);
             general_mat_vec_mul_impl(A::one(), self, rhs, A::zero(), c.raw_view_mut().cast::<A>());
             c.assume_init()
         }

src/stacking.rs

@@ -91,7 +91,7 @@ where
 
     // we can safely use uninitialized values here because we will
     // overwrite every one of them.
-    let mut res = Array::maybe_uninit(res_dim);
+    let mut res = Array::uninit(res_dim);
 
     {
         let mut assign_view = res.view_mut();
@@ -159,7 +159,7 @@ where
 
     // we can safely use uninitialized values here because we will
     // overwrite every one of them.
-    let mut res = Array::maybe_uninit(res_dim);
+    let mut res = Array::uninit(res_dim);
 
     res.axis_iter_mut(axis)
         .zip(arrays.iter())

src/zip/mod.rs

@@ -9,6 +9,7 @@
 #[macro_use]
 mod zipmacro;
 
+#[cfg(feature = "rayon")]
 use std::mem::MaybeUninit;
 use alloc::vec::Vec;
 
@@ -811,10 +812,11 @@ where
         FoldWhile::Continue(acc)
     }
 
+    #[cfg(feature = "rayon")]
     pub(crate) fn uninitalized_for_current_layout<T>(&self) -> Array<MaybeUninit<T>, D>
     {
         let is_f = self.prefer_f();
-        Array::maybe_uninit(self.dimension.clone().set_f(is_f))
+        Array::uninit(self.dimension.clone().set_f(is_f))
     }
 }
 
@@ -1079,17 +1081,27 @@ macro_rules! map_impl {
             ///
             /// If all inputs are c- or f-order respectively, that is preserved in the output.
             pub fn map_collect<R>(self, f: impl FnMut($($p::Item,)* ) -> R) -> Array<R, D> {
-                // Make uninit result
-                let mut output = self.uninitalized_for_current_layout::<R>();
+                self.map_collect_owned(f)
+            }
 
-                // Use partial to counts the number of filled elements, and can drop the right
-                // number of elements on unwinding (if it happens during apply/collect).
+            pub(crate) fn map_collect_owned<S, R>(self, f: impl FnMut($($p::Item,)* ) -> R)
+                -> ArrayBase<S, D>
+                where S: DataOwned<Elem = R>
+            {
+                // safe because: all elements are written before the array is completed
+
+                let shape = self.dimension.clone().set_f(self.prefer_f());
+                let output = <ArrayBase<S, D>>::build_uninit(shape, |output| {
+                    // Use partial to count the number of filled elements, and can drop the right
+                    // number of elements on unwinding (if it happens during apply/collect).
+                    unsafe {
+                        let output_view = output.cast::<R>();
+                        self.and(output_view)
+                            .collect_with_partial(f)
+                            .release_ownership();
+                    }
+                });
                 unsafe {
-                    let output_view = output.raw_view_mut().cast::<R>();
-                    self.and(output_view)
-                        .collect_with_partial(f)
-                        .release_ownership();
-
                     output.assume_init()
                 }
             }