diff --git a/src/data_traits.rs b/src/data_traits.rs index 9e814a28c..2b59fcb8d 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -245,11 +245,10 @@ unsafe impl Data for OwnedArcRepr { { Self::ensure_unique(&mut self_); let data = Arc::try_unwrap(self_.data.0).ok().unwrap(); - ArrayBase { - data, - ptr: self_.ptr, - dim: self_.dim, - strides: self_.strides, + // safe because data is equivalent + unsafe { + ArrayBase::from_data_ptr(data, self_.ptr) + .with_strides_dim(self_.strides, self_.dim) } } @@ -544,11 +543,10 @@ unsafe impl<'a, A> Data for CowRepr<'a, A> { { match self_.data { CowRepr::View(_) => self_.to_owned(), - CowRepr::Owned(data) => ArrayBase { - data, - ptr: self_.ptr, - dim: self_.dim, - strides: self_.strides, + CowRepr::Owned(data) => unsafe { + // safe because the data is equivalent so ptr, dims remain valid + ArrayBase::from_data_ptr(data, self_.ptr) + .with_strides_dim(self_.strides, self_.dim) }, } } diff --git a/src/impl_clone.rs b/src/impl_clone.rs index 603708849..e2e111a12 100644 --- a/src/impl_clone.rs +++ b/src/impl_clone.rs @@ -11,6 +11,7 @@ use crate::RawDataClone; impl Clone for ArrayBase { fn clone(&self) -> ArrayBase { + // safe because `clone_with_ptr` promises to provide equivalent data and ptr unsafe { let (data, ptr) = self.data.clone_with_ptr(self.ptr); ArrayBase { diff --git a/src/impl_constructors.rs b/src/impl_constructors.rs index d556934d9..9af8048f6 100644 --- a/src/impl_constructors.rs +++ b/src/impl_constructors.rs @@ -470,12 +470,9 @@ where unsafe fn from_vec_dim_stride_unchecked(dim: D, strides: D, mut v: Vec) -> Self { // debug check for issues that indicates wrong use of this constructor debug_assert!(dimension::can_index_slice(&v, &dim, &strides).is_ok()); - ArrayBase { - ptr: nonnull_from_vec_data(&mut v).offset(-offset_from_ptr_to_memory(&dim, &strides)), - data: DataOwned::new(v), - strides, - dim, - } + + let ptr = nonnull_from_vec_data(&mut v).offset(-offset_from_ptr_to_memory(&dim, &strides)); + ArrayBase::from_data_ptr(DataOwned::new(v), ptr).with_strides_dim(strides, dim) } /// Create an array with uninitalized elements, shape `shape`. diff --git a/src/impl_cow.rs b/src/impl_cow.rs index 52a06bfa2..b880fd62c 100644 --- a/src/impl_cow.rs +++ b/src/impl_cow.rs @@ -33,11 +33,10 @@ where D: Dimension, { fn from(view: ArrayView<'a, A, D>) -> CowArray<'a, A, D> { - ArrayBase { - data: CowRepr::View(view.data), - ptr: view.ptr, - dim: view.dim, - strides: view.strides, + // safe because equivalent data + unsafe { + ArrayBase::from_data_ptr(CowRepr::View(view.data), view.ptr) + .with_strides_dim(view.strides, view.dim) } } } @@ -47,11 +46,10 @@ where D: Dimension, { fn from(array: Array) -> CowArray<'a, A, D> { - ArrayBase { - data: CowRepr::Owned(array.data), - ptr: array.ptr, - dim: array.dim, - strides: array.strides, + // safe because equivalent data + unsafe { + ArrayBase::from_data_ptr(CowRepr::Owned(array.data), array.ptr) + .with_strides_dim(array.strides, array.dim) } } } diff --git a/src/impl_internal_constructors.rs b/src/impl_internal_constructors.rs new file mode 100644 index 000000000..0ed60a622 --- /dev/null +++ b/src/impl_internal_constructors.rs @@ -0,0 +1,66 @@ +// Copyright 2021 bluss and ndarray developers. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use std::ptr::NonNull; + +use crate::imp_prelude::*; + +// internal "builder-like" methods +impl ArrayBase +where + S: RawData, +{ + /// Create an (initially) empty one-dimensional array from the given data and array head + /// pointer + /// + /// ## Safety + /// + /// The caller must ensure that the data storage and pointer is valid. + /// + /// See ArrayView::from_shape_ptr for general pointer validity documentation. + pub(crate) unsafe fn from_data_ptr(data: S, ptr: NonNull) -> Self { + let array = ArrayBase { + data: data, + ptr: ptr, + dim: Ix1(0), + strides: Ix1(1), + }; + debug_assert!(array.pointer_is_inbounds()); + array + } +} + +// internal "builder-like" methods +impl ArrayBase +where + S: RawData, + D: Dimension, +{ + + /// Set strides and dimension of the array to the new values + /// + /// The argument order with strides before dimensions is used because strides are often + /// computed as derived from the dimension. + /// + /// ## Safety + /// + /// The caller needs to ensure that the new strides and dimensions are correct + /// for the array data. + pub(crate) unsafe fn with_strides_dim(self, strides: E, dim: E) -> ArrayBase + where + E: Dimension + { + debug_assert_eq!(strides.ndim(), dim.ndim()); + ArrayBase { + data: self.data, + ptr: self.ptr, + dim, + strides, + } + } +} diff --git a/src/impl_methods.rs b/src/impl_methods.rs index 7b6f3f6f5..9eea42d15 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -242,11 +242,9 @@ where S: DataOwned, { let data = self.data.into_shared(); - ArrayBase { - data, - ptr: self.ptr, - dim: self.dim, - strides: self.strides, + // safe because: equivalent unmoved data, ptr and dims remain valid + unsafe { + ArrayBase::from_data_ptr(data, self.ptr).with_strides_dim(self.strides, self.dim) } } @@ -434,11 +432,9 @@ where *new_s = *s; }); - ArrayBase { - ptr: self.ptr, - data: self.data, - dim: new_dim, - strides: new_strides, + // safe because new dimension, strides allow access to a subset of old data + unsafe { + self.with_strides_dim(new_strides, new_dim) } } @@ -757,11 +753,9 @@ where self.collapse_axis(axis, index); let dim = self.dim.remove_axis(axis); let strides = self.strides.remove_axis(axis); - ArrayBase { - ptr: self.ptr, - data: self.data, - dim, - strides, + // safe because new dimension, strides allow access to a subset of old data + unsafe { + self.with_strides_dim(strides, dim) } } @@ -1244,11 +1238,9 @@ where /// Return the diagonal as a one-dimensional array. pub fn into_diag(self) -> ArrayBase { let (len, stride) = self.diag_params(); - ArrayBase { - data: self.data, - ptr: self.ptr, - dim: Ix1(len), - strides: Ix1(stride as Ix), + // safe because new len stride allows access to a subset of the current elements + unsafe { + self.with_strides_dim(Ix1(stride as Ix), Ix1(len)) } } @@ -1498,22 +1490,15 @@ where return Err(error::incompatible_shapes(&self.dim, &shape)); } // Check if contiguous, if not => copy all, else just adapt strides - if self.is_standard_layout() { - Ok(ArrayBase { - data: self.data, - ptr: self.ptr, - strides: shape.default_strides(), - dim: shape, - }) - } else if self.ndim() > 1 && self.raw_view().reversed_axes().is_standard_layout() { - Ok(ArrayBase { - data: self.data, - ptr: self.ptr, - strides: shape.fortran_strides(), - dim: shape, - }) - } else { - Err(error::from_kind(error::ErrorKind::IncompatibleLayout)) + unsafe { + // safe because arrays are contiguous and len is unchanged + if self.is_standard_layout() { + Ok(self.with_strides_dim(shape.default_strides(), shape)) + } else if self.ndim() > 1 && self.raw_view().reversed_axes().is_standard_layout() { + Ok(self.with_strides_dim(shape.fortran_strides(), shape)) + } else { + Err(error::from_kind(error::ErrorKind::IncompatibleLayout)) + } } } @@ -1554,11 +1539,9 @@ where // Check if contiguous, if not => copy all, else just adapt strides if self.is_standard_layout() { let cl = self.clone(); - ArrayBase { - data: cl.data, - ptr: cl.ptr, - strides: shape.default_strides(), - dim: shape, + // safe because array is contiguous and shape has equal number of elements + unsafe { + cl.with_strides_dim(shape.default_strides(), shape) } } else { let v = self.iter().cloned().collect::>(); @@ -1576,11 +1559,10 @@ where /// [3, 4]]).into_dyn(); /// ``` pub fn into_dyn(self) -> ArrayBase { - ArrayBase { - data: self.data, - ptr: self.ptr, - dim: self.dim.into_dyn(), - strides: self.strides.into_dyn(), + // safe because new dims equivalent + unsafe { + ArrayBase::from_data_ptr(self.data, self.ptr) + .with_strides_dim(self.strides.into_dyn(), self.dim.into_dyn()) } } @@ -1604,27 +1586,19 @@ where where D2: Dimension, { - if D::NDIM == D2::NDIM { - // safe because D == D2 - unsafe { + unsafe { + if D::NDIM == D2::NDIM { + // safe because D == D2 let dim = unlimited_transmute::(self.dim); let strides = unlimited_transmute::(self.strides); - return Ok(ArrayBase { - data: self.data, - ptr: self.ptr, - dim, - strides, - }); - } - } else if D::NDIM == None || D2::NDIM == None { // one is dynamic dim - if let Some(dim) = D2::from_dimension(&self.dim) { - if let Some(strides) = D2::from_dimension(&self.strides) { - return Ok(ArrayBase { - data: self.data, - ptr: self.ptr, - dim, - strides, - }); + return Ok(ArrayBase::from_data_ptr(self.data, self.ptr) + .with_strides_dim(strides, dim)); + } else if D::NDIM == None || D2::NDIM == None { // one is dynamic dim + // safe because dim, strides are equivalent under a different type + if let Some(dim) = D2::from_dimension(&self.dim) { + if let Some(strides) = D2::from_dimension(&self.strides) { + return Ok(self.with_strides_dim(strides, dim)); + } } } } @@ -1792,10 +1766,9 @@ where new_strides[new_axis] = strides[axis]; } } - ArrayBase { - dim: new_dim, - strides: new_strides, - ..self + // safe because axis invariants are checked above; they are a permutation of the old + unsafe { + self.with_strides_dim(new_strides, new_dim) } } @@ -1915,17 +1888,11 @@ where /// ***Panics*** if the axis is out of bounds. pub fn insert_axis(self, axis: Axis) -> ArrayBase { assert!(axis.index() <= self.ndim()); - let ArrayBase { - ptr, - data, - dim, - strides, - } = self; - ArrayBase { - ptr, - data, - dim: dim.insert_axis(axis), - strides: strides.insert_axis(axis), + // safe because a new axis of length one does not affect memory layout + unsafe { + let strides = self.strides.insert_axis(axis); + let dim = self.dim.insert_axis(axis); + self.with_strides_dim(strides, dim) } } @@ -1942,7 +1909,7 @@ where self.index_axis_move(axis, 0) } - fn pointer_is_inbounds(&self) -> bool { + pub(crate) fn pointer_is_inbounds(&self) -> bool { match self.data._data_slice() { None => { // special case for non-owned views diff --git a/src/impl_raw_views.rs b/src/impl_raw_views.rs index e3447cac5..2ac5c08c7 100644 --- a/src/impl_raw_views.rs +++ b/src/impl_raw_views.rs @@ -17,12 +17,8 @@ where /// meet all of the invariants of the `ArrayBase` type. #[inline] pub(crate) unsafe fn new(ptr: NonNull, dim: D, strides: D) -> Self { - RawArrayView { - data: RawViewRepr::new(), - ptr, - dim, - strides, - } + RawArrayView::from_data_ptr(RawViewRepr::new(), ptr) + .with_strides_dim(strides, dim) } unsafe fn new_(ptr: *const A, dim: D, strides: D) -> Self { @@ -163,12 +159,8 @@ where /// meet all of the invariants of the `ArrayBase` type. #[inline] pub(crate) unsafe fn new(ptr: NonNull, dim: D, strides: D) -> Self { - RawArrayViewMut { - data: RawViewRepr::new(), - ptr, - dim, - strides, - } + RawArrayViewMut::from_data_ptr(RawViewRepr::new(), ptr) + .with_strides_dim(strides, dim) } unsafe fn new_(ptr: *mut A, dim: D, strides: D) -> Self { diff --git a/src/impl_special_element_types.rs b/src/impl_special_element_types.rs index e41177dbf..bf4384e42 100644 --- a/src/impl_special_element_types.rs +++ b/src/impl_special_element_types.rs @@ -40,12 +40,6 @@ where // "transmute" from storage of MaybeUninit to storage of A let data = S::data_subst(data); let ptr = ptr.cast::(); - - ArrayBase { - data, - ptr, - dim, - strides, - } + ArrayBase::from_data_ptr(data, ptr).with_strides_dim(strides, dim) } } diff --git a/src/impl_views/constructors.rs b/src/impl_views/constructors.rs index c6e5f9988..a133ee1d1 100644 --- a/src/impl_views/constructors.rs +++ b/src/impl_views/constructors.rs @@ -230,12 +230,7 @@ where assert!(is_aligned(ptr.as_ptr()), "The pointer must be aligned."); dimension::max_abs_offset_check_overflow::(&dim, &strides).unwrap(); } - ArrayView { - data: ViewRepr::new(), - ptr, - dim, - strides, - } + ArrayView::from_data_ptr(ViewRepr::new(), ptr).with_strides_dim(strides, dim) } /// Unsafe because: `ptr` must be valid for the given dimension and strides. @@ -258,12 +253,7 @@ where assert!(is_aligned(ptr.as_ptr()), "The pointer must be aligned."); dimension::max_abs_offset_check_overflow::(&dim, &strides).unwrap(); } - ArrayViewMut { - data: ViewRepr::new(), - ptr, - dim, - strides, - } + ArrayViewMut::from_data_ptr(ViewRepr::new(), ptr).with_strides_dim(strides, dim) } /// Create a new `ArrayView` diff --git a/src/lib.rs b/src/lib.rs index c71f09aaf..0c7caf735 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1488,6 +1488,7 @@ impl<'a, A> CowRepr<'a, A> { mod impl_clone; +mod impl_internal_constructors; mod impl_constructors; mod impl_methods; @@ -1564,11 +1565,9 @@ where fn try_remove_axis(self, axis: Axis) -> ArrayBase { let d = self.dim.try_remove_axis(axis); let s = self.strides.try_remove_axis(axis); - ArrayBase { - ptr: self.ptr, - data: self.data, - dim: d, - strides: s, + // safe because new dimension, strides allow access to a subset of old data + unsafe { + self.with_strides_dim(s, d) } }