Makes the reference types sound via a DST (#1532)

The current implementation of ArrayRef and its cousins has them as sized types, which turns out to be a critical and unsound mistake. This PR is large, but its heart is small: change the ArrayRef implementation to be unsized.

The approach this PR takes is to package the core array "metadata" - the pointer, dim, and strides - into a struct that can either be sized or unsized. This is done by appending a generic "_dst_control" field. For the "sized" version of the metadata, that field is a 0-length array. For the "unsized" version of the metadata, that sized field is a struct. This core type is private, so users cannot construct any other variants other than these two.

We then put the sized version into the ArrayBase types, put the unsized version into the reference types, and perform an unsizing coercion to convert from one to the other. Because Rust has no (safe, supported) "resizing" coercion, this switch is irreversible. Sized types cannot be recovered from the unsized reference types.

Author: akern40

examples/functions_and_traits.rs

@@ -139,7 +139,7 @@ fn takes_rawref_mut<A, D>(arr: &mut RawRef<A, D>)
 /// Immutable, take a generic that implements `AsRef` to `RawRef`
 #[allow(dead_code)]
 fn takes_rawref_asref<T, A, D>(_arr: &T)
-where T: AsRef<RawRef<A, D>>
+where T: AsRef<RawRef<A, D>> + ?Sized
 {
     takes_layout(_arr.as_ref());
     takes_layout_asref(_arr.as_ref());
@@ -148,7 +148,7 @@ where T: AsRef<RawRef<A, D>>
 /// Mutable, take a generic that implements `AsMut` to `RawRef`
 #[allow(dead_code)]
 fn takes_rawref_asmut<T, A, D>(_arr: &mut T)
-where T: AsMut<RawRef<A, D>>
+where T: AsMut<RawRef<A, D>> + ?Sized
 {
     takes_layout_mut(_arr.as_mut());
     takes_layout_asmut(_arr.as_mut());
@@ -169,10 +169,16 @@ fn takes_layout_mut<A, D>(_arr: &mut LayoutRef<A, D>) {}
 
 /// Immutable, take a generic that implements `AsRef` to `LayoutRef`
 #[allow(dead_code)]
-fn takes_layout_asref<T: AsRef<LayoutRef<A, D>>, A, D>(_arr: &T) {}
+fn takes_layout_asref<T, A, D>(_arr: &T)
+where T: AsRef<LayoutRef<A, D>> + ?Sized
+{
+}
 
 /// Mutable, take a generic that implements `AsMut` to `LayoutRef`
 #[allow(dead_code)]
-fn takes_layout_asmut<T: AsMut<LayoutRef<A, D>>, A, D>(_arr: &mut T) {}
+fn takes_layout_asmut<T, A, D>(_arr: &mut T)
+where T: AsMut<LayoutRef<A, D>> + ?Sized
+{
+}
 
 fn main() {}

scripts/all-tests.sh

@@ -30,5 +30,8 @@ fi
 # Examples
 cargo nextest run --examples
 
+# Doc tests
+cargo test --doc
+
 # Benchmarks
 ([ "$CHANNEL" != "nightly" ] || cargo bench --no-run --verbose --features "$FEATURES")

src/arraytraits.rs

@@ -19,7 +19,6 @@ use std::{iter::FromIterator, slice};
 use crate::imp_prelude::*;
 use crate::Arc;
 
-use crate::LayoutRef;
 use crate::{
     dimension,
     iter::{Iter, IterMut},
@@ -38,12 +37,14 @@ pub(crate) fn array_out_of_bounds() -> !
 }
 
 #[inline(always)]
-pub fn debug_bounds_check<A, D, I>(_a: &LayoutRef<A, D>, _index: &I)
+pub fn debug_bounds_check<A, D, I, T>(_a: &T, _index: &I)
 where
     D: Dimension,
     I: NdIndex<D>,
+    T: AsRef<LayoutRef<A, D>> + ?Sized,
 {
-    debug_bounds_check!(_a, *_index);
+    let _layout_ref = _a.as_ref();
+    debug_bounds_check_ref!(_layout_ref, *_index);
 }
 
 /// Access the element at **index**.
@@ -59,11 +60,11 @@ where
     #[inline]
     fn index(&self, index: I) -> &Self::Output
     {
-        debug_bounds_check!(self, index);
+        debug_bounds_check_ref!(self, index);
         unsafe {
-            &*self.ptr.as_ptr().offset(
+            &*self._ptr().as_ptr().offset(
                 index
-                    .index_checked(&self.dim, &self.strides)
+                    .index_checked(self._dim(), self._strides())
                     .unwrap_or_else(|| array_out_of_bounds()),
             )
         }
@@ -81,11 +82,11 @@ where
     #[inline]
     fn index_mut(&mut self, index: I) -> &mut A
     {
-        debug_bounds_check!(self, index);
+        debug_bounds_check_ref!(self, index);
         unsafe {
             &mut *self.as_mut_ptr().offset(
                 index
-                    .index_checked(&self.dim, &self.strides)
+                    .index_checked(self._dim(), self._strides())
                     .unwrap_or_else(|| array_out_of_bounds()),
             )
         }
@@ -581,7 +582,7 @@ where D: Dimension
     {
         let data = OwnedArcRepr(Arc::new(arr.data));
         // safe because: equivalent unmoved data, ptr and dims remain valid
-        unsafe { ArrayBase::from_data_ptr(data, arr.layout.ptr).with_strides_dim(arr.layout.strides, arr.layout.dim) }
+        unsafe { ArrayBase::from_data_ptr(data, arr.parts.ptr).with_strides_dim(arr.parts.strides, arr.parts.dim) }
     }
 }
 

src/data_traits.rs

@@ -251,7 +251,7 @@ where A: Clone
         if Arc::get_mut(&mut self_.data.0).is_some() {
             return;
         }
-        if self_.layout.dim.size() <= self_.data.0.len() / 2 {
+        if self_.parts.dim.size() <= self_.data.0.len() / 2 {
             // Clone only the visible elements if the current view is less than
             // half of backing data.
             *self_ = self_.to_owned().into_shared();
@@ -260,13 +260,13 @@ where A: Clone
         let rcvec = &mut self_.data.0;
         let a_size = mem::size_of::<A>() as isize;
         let our_off = if a_size != 0 {
-            (self_.layout.ptr.as_ptr() as isize - rcvec.as_ptr() as isize) / a_size
+            (self_.parts.ptr.as_ptr() as isize - rcvec.as_ptr() as isize) / a_size
         } else {
             0
         };
         let rvec = Arc::make_mut(rcvec);
         unsafe {
-            self_.layout.ptr = rvec.as_nonnull_mut().offset(our_off);
+            self_.parts.ptr = rvec.as_nonnull_mut().offset(our_off);
         }
     }
 
@@ -287,7 +287,7 @@ unsafe impl<A> Data for OwnedArcRepr<A>
         let data = Arc::try_unwrap(self_.data.0).ok().unwrap();
         // safe because data is equivalent
         unsafe {
-            ArrayBase::from_data_ptr(data, self_.layout.ptr).with_strides_dim(self_.layout.strides, self_.layout.dim)
+            ArrayBase::from_data_ptr(data, self_.parts.ptr).with_strides_dim(self_.parts.strides, self_.parts.dim)
         }
     }
 
@@ -297,14 +297,14 @@ unsafe impl<A> Data for OwnedArcRepr<A>
         match Arc::try_unwrap(self_.data.0) {
             Ok(owned_data) => unsafe {
                 // Safe because the data is equivalent.
-                Ok(ArrayBase::from_data_ptr(owned_data, self_.layout.ptr)
-                    .with_strides_dim(self_.layout.strides, self_.layout.dim))
+                Ok(ArrayBase::from_data_ptr(owned_data, self_.parts.ptr)
+                    .with_strides_dim(self_.parts.strides, self_.parts.dim))
             },
             Err(arc_data) => unsafe {
                 // Safe because the data is equivalent; we're just
                 // reconstructing `self_`.
-                Err(ArrayBase::from_data_ptr(OwnedArcRepr(arc_data), self_.layout.ptr)
-                    .with_strides_dim(self_.layout.strides, self_.layout.dim))
+                Err(ArrayBase::from_data_ptr(OwnedArcRepr(arc_data), self_.parts.ptr)
+                    .with_strides_dim(self_.parts.strides, self_.parts.dim))
             },
         }
     }
@@ -603,9 +603,9 @@ where A: Clone
             CowRepr::View(_) => {
                 let owned = ArrayRef::to_owned(array);
                 array.data = CowRepr::Owned(owned.data);
-                array.layout.ptr = owned.layout.ptr;
-                array.layout.dim = owned.layout.dim;
-                array.layout.strides = owned.layout.strides;
+                array.parts.ptr = owned.parts.ptr;
+                array.parts.dim = owned.parts.dim;
+                array.parts.strides = owned.parts.strides;
             }
             CowRepr::Owned(_) => {}
         }
@@ -666,8 +666,7 @@ unsafe impl<'a, A> Data for CowRepr<'a, A>
             CowRepr::View(_) => self_.to_owned(),
             CowRepr::Owned(data) => unsafe {
                 // safe because the data is equivalent so ptr, dims remain valid
-                ArrayBase::from_data_ptr(data, self_.layout.ptr)
-                    .with_strides_dim(self_.layout.strides, self_.layout.dim)
+                ArrayBase::from_data_ptr(data, self_.parts.ptr).with_strides_dim(self_.parts.strides, self_.parts.dim)
             },
         }
     }
@@ -679,8 +678,8 @@ unsafe impl<'a, A> Data for CowRepr<'a, A>
             CowRepr::View(_) => Err(self_),
             CowRepr::Owned(data) => unsafe {
                 // safe because the data is equivalent so ptr, dims remain valid
-                Ok(ArrayBase::from_data_ptr(data, self_.layout.ptr)
-                    .with_strides_dim(self_.layout.strides, self_.layout.dim))
+                Ok(ArrayBase::from_data_ptr(data, self_.parts.ptr)
+                    .with_strides_dim(self_.parts.strides, self_.parts.dim))
             },
         }
     }

src/free_functions.rs

@@ -16,7 +16,7 @@ use std::mem::{forget, size_of};
 use std::ptr::NonNull;
 
 use crate::{dimension, ArcArray1, ArcArray2};
-use crate::{imp_prelude::*, LayoutRef};
+use crate::{imp_prelude::*, ArrayPartsSized};
 
 /// Create an **[`Array`]** with one, two, three, four, five, or six dimensions.
 ///
@@ -109,12 +109,12 @@ pub const fn aview0<A>(x: &A) -> ArrayView0<'_, A>
 {
     ArrayBase {
         data: ViewRepr::new(),
-        layout: LayoutRef {
+        parts: ArrayPartsSized::new(
             // Safe because references are always non-null.
-            ptr: unsafe { NonNull::new_unchecked(x as *const A as *mut A) },
-            dim: Ix0(),
-            strides: Ix0(),
-        },
+            unsafe { NonNull::new_unchecked(x as *const A as *mut A) },
+            Ix0(),
+            Ix0(),
+        ),
     }
 }
 
@@ -149,12 +149,12 @@ pub const fn aview1<A>(xs: &[A]) -> ArrayView1<'_, A>
     }
     ArrayBase {
         data: ViewRepr::new(),
-        layout: LayoutRef {
+        parts: ArrayPartsSized::new(
             // Safe because references are always non-null.
-            ptr: unsafe { NonNull::new_unchecked(xs.as_ptr() as *mut A) },
-            dim: Ix1(xs.len()),
-            strides: Ix1(1),
-        },
+            unsafe { NonNull::new_unchecked(xs.as_ptr() as *mut A) },
+            Ix1(xs.len()),
+            Ix1(1),
+        ),
     }
 }
 
@@ -207,7 +207,7 @@ pub const fn aview2<A, const N: usize>(xs: &[[A; N]]) -> ArrayView2<'_, A>
     };
     ArrayBase {
         data: ViewRepr::new(),
-        layout: LayoutRef { ptr, dim, strides },
+        parts: ArrayPartsSized::new(ptr, dim, strides),
     }
 }
 

src/impl_clone.rs

@@ -7,7 +7,7 @@
 // except according to those terms.
 
 use crate::imp_prelude::*;
-use crate::LayoutRef;
+use crate::ArrayPartsSized;
 use crate::RawDataClone;
 
 impl<S: RawDataClone, D: Clone> Clone for ArrayBase<S, D>
@@ -16,14 +16,10 @@ impl<S: RawDataClone, D: Clone> Clone for ArrayBase<S, D>
     {
         // safe because `clone_with_ptr` promises to provide equivalent data and ptr
         unsafe {
-            let (data, ptr) = self.data.clone_with_ptr(self.layout.ptr);
+            let (data, ptr) = self.data.clone_with_ptr(self.parts.ptr);
             ArrayBase {
                 data,
-                layout: LayoutRef {
-                    ptr,
-                    dim: self.layout.dim.clone(),
-                    strides: self.layout.strides.clone(),
-                },
+                parts: ArrayPartsSized::new(ptr, self.parts.dim.clone(), self.parts.strides.clone()),
             }
         }
     }
@@ -34,9 +30,9 @@ impl<S: RawDataClone, D: Clone> Clone for ArrayBase<S, D>
     fn clone_from(&mut self, other: &Self)
     {
         unsafe {
-            self.layout.ptr = self.data.clone_from_with_ptr(&other.data, other.layout.ptr);
-            self.layout.dim.clone_from(&other.layout.dim);
-            self.layout.strides.clone_from(&other.layout.strides);
+            self.parts.ptr = self.data.clone_from_with_ptr(&other.data, other.parts.ptr);
+            self.parts.dim.clone_from(&other.parts.dim);
+            self.parts.strides.clone_from(&other.parts.strides);
         }
     }
 }

src/impl_cow.rs

@@ -34,8 +34,8 @@ where D: Dimension
     {
         // safe because equivalent data
         unsafe {
-            ArrayBase::from_data_ptr(CowRepr::View(view.data), view.ptr)
-                .with_strides_dim(view.layout.strides, view.layout.dim)
+            ArrayBase::from_data_ptr(CowRepr::View(view.data), view.parts.ptr)
+                .with_strides_dim(view.parts.strides, view.parts.dim)
         }
     }
 }
@@ -47,8 +47,8 @@ where D: Dimension
     {
         // safe because equivalent data
         unsafe {
-            ArrayBase::from_data_ptr(CowRepr::Owned(array.data), array.layout.ptr)
-                .with_strides_dim(array.layout.strides, array.layout.dim)
+            ArrayBase::from_data_ptr(CowRepr::Owned(array.data), array.parts.ptr)
+                .with_strides_dim(array.parts.strides, array.parts.dim)
         }
     }
 }

src/impl_dyn.rs

@@ -31,8 +31,8 @@ impl<A> LayoutRef<A, IxDyn>
     pub fn insert_axis_inplace(&mut self, axis: Axis)
     {
         assert!(axis.index() <= self.ndim());
-        self.dim = self.dim.insert_axis(axis);
-        self.strides = self.strides.insert_axis(axis);
+        self.0.dim = self._dim().insert_axis(axis);
+        self.0.strides = self._strides().insert_axis(axis);
     }
 
     /// Collapses the array to `index` along the axis and removes the axis,
@@ -54,8 +54,8 @@ impl<A> LayoutRef<A, IxDyn>
     pub fn index_axis_inplace(&mut self, axis: Axis, index: usize)
     {
         self.collapse_axis(axis, index);
-        self.dim = self.dim.remove_axis(axis);
-        self.strides = self.strides.remove_axis(axis);
+        self.0.dim = self._dim().remove_axis(axis);
+        self.0.strides = self._strides().remove_axis(axis);
     }
 }
 

src/impl_internal_constructors.rs

@@ -8,7 +8,7 @@
 
 use std::ptr::NonNull;
 
-use crate::{imp_prelude::*, LayoutRef};
+use crate::{imp_prelude::*, ArrayPartsSized};
 
 // internal "builder-like" methods
 impl<A, S> ArrayBase<S, Ix1>
@@ -27,11 +27,7 @@ where S: RawData<Elem = A>
     {
         let array = ArrayBase {
             data,
-            layout: LayoutRef {
-                ptr,
-                dim: Ix1(0),
-                strides: Ix1(1),
-            },
+            parts: ArrayPartsSized::new(ptr, Ix1(0), Ix1(1)),
         };
         debug_assert!(array.pointer_is_inbounds());
         array
@@ -60,11 +56,7 @@ where
         debug_assert_eq!(strides.ndim(), dim.ndim());
         ArrayBase {
             data: self.data,
-            layout: LayoutRef {
-                ptr: self.layout.ptr,
-                dim,
-                strides,
-            },
+            parts: ArrayPartsSized::new(self.parts.ptr, dim, strides),
         }
     }
 }