diff --git a/examples/sort-axis.rs b/examples/sort-axis.rs index ee5ff3465..f8bfadc13 100644 --- a/examples/sort-axis.rs +++ b/examples/sort-axis.rs @@ -155,7 +155,7 @@ where }); debug_assert_eq!(result.len(), moved_elements); // forget the old elements but not the allocation - let mut old_storage = self.into_raw_vec(); + let mut old_storage = self.into_raw_vec().0; old_storage.set_len(0); // transfer ownership of the elements into the result diff --git a/src/arrayformat.rs b/src/arrayformat.rs index c1ca352e3..5998a0c1e 100644 --- a/src/arrayformat.rs +++ b/src/arrayformat.rs @@ -50,10 +50,8 @@ impl FormatOptions { self.axis_collapse_limit = std::usize::MAX; self.axis_collapse_limit_next_last = std::usize::MAX; self.axis_collapse_limit_last = std::usize::MAX; - self - } else { - self } + self } /// Axis length collapse limit before ellipsizing, where `axis_rindex` is diff --git a/src/data_traits.rs b/src/data_traits.rs index 7ac63d54e..7d9f15b0a 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -252,6 +252,7 @@ unsafe impl Data for OwnedArcRepr { } } + #[allow(clippy::wrong_self_convention)] fn to_shared(self_: &ArrayBase) -> ArrayBase, D> where Self::Elem: Clone, diff --git a/src/dimension/mod.rs b/src/dimension/mod.rs index f4f46e764..5f597efd1 100644 --- a/src/dimension/mod.rs +++ b/src/dimension/mod.rs @@ -232,11 +232,12 @@ where /// units of `A` and in units of bytes between the least address and greatest /// address accessible by moving along all axes does not exceed `isize::MAX`. /// -/// Warning: This function is sufficient to check the invariants of ArrayBase only -/// if the pointer to the first element of the array is chosen such that the element -/// with the smallest memory address is at the start of data. (In other words, the -/// pointer to the first element of the array must be computed using offset_from_ptr_to_memory -/// so that negative strides are correctly handled.) +/// Warning: This function is sufficient to check the invariants of ArrayBase +/// only if the pointer to the first element of the array is chosen such that +/// the element with the smallest memory address is at the start of allocation. +/// (In other words, the pointer to the first element of the array must be +/// computed using `offset_from_low_addr_ptr_to_logical_ptr` so that negative +/// strides are correctly handled.) pub(crate) fn can_index_slice( data: &[A], dim: &D, @@ -404,17 +405,18 @@ fn to_abs_slice(axis_len: usize, slice: Slice) -> (usize, usize, isize) { (start, end, step) } -/// This function computes the offset from the logically first element to the first element in -/// memory of the array. The result is always <= 0. -pub fn offset_from_ptr_to_memory(dim: &D, strides: &D) -> isize { +/// This function computes the offset from the lowest address element to the +/// logically first element. The result is always >= 0. +pub fn offset_from_low_addr_ptr_to_logical_ptr(dim: &D, strides: &D) -> usize { let offset = izip!(dim.slice(), strides.slice()).fold(0, |_offset, (d, s)| { if (*s as isize) < 0 { - _offset + *s as isize * (*d as isize - 1) + _offset - *s as isize * (*d as isize - 1) } else { _offset } }); - offset + debug_assert!(offset >= 0); + offset as usize } /// Modify dimension, stride and return data pointer offset diff --git a/src/impl_constructors.rs b/src/impl_constructors.rs index 3336ec336..40a54d1da 100644 --- a/src/impl_constructors.rs +++ b/src/impl_constructors.rs @@ -20,7 +20,7 @@ use alloc::vec; use alloc::vec::Vec; use crate::dimension; -use crate::dimension::offset_from_ptr_to_memory; +use crate::dimension::offset_from_low_addr_ptr_to_logical_ptr; use crate::error::{self, ShapeError}; use crate::extension::nonnull::nonnull_from_vec_data; use crate::imp_prelude::*; @@ -492,7 +492,7 @@ where // debug check for issues that indicates wrong use of this constructor debug_assert!(dimension::can_index_slice(&v, &dim, &strides).is_ok()); - let ptr = nonnull_from_vec_data(&mut v).offset(-offset_from_ptr_to_memory(&dim, &strides)); + let ptr = nonnull_from_vec_data(&mut v).add(offset_from_low_addr_ptr_to_logical_ptr(&dim, &strides)); ArrayBase::from_data_ptr(DataOwned::new(v), ptr).with_strides_dim(strides, dim) } diff --git a/src/impl_methods.rs b/src/impl_methods.rs index 9ef4277de..feada44ce 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -20,7 +20,7 @@ use crate::dimension; use crate::dimension::IntoDimension; use crate::dimension::{ abs_index, axes_of, do_slice, merge_axes, move_min_stride_axis_to_last, - offset_from_ptr_to_memory, size_of_shape_checked, stride_offset, Axes, + offset_from_low_addr_ptr_to_logical_ptr, size_of_shape_checked, stride_offset, Axes, }; use crate::dimension::broadcast::co_broadcast; use crate::error::{self, ErrorKind, ShapeError, from_kind}; @@ -1536,10 +1536,10 @@ where S: Data, { if self.is_contiguous() { - let offset = offset_from_ptr_to_memory(&self.dim, &self.strides); + let offset = offset_from_low_addr_ptr_to_logical_ptr(&self.dim, &self.strides); unsafe { Some(slice::from_raw_parts( - self.ptr.offset(offset).as_ptr(), + self.ptr.sub(offset).as_ptr(), self.len(), )) } @@ -1565,10 +1565,10 @@ where { if self.is_contiguous() { self.ensure_unique(); - let offset = offset_from_ptr_to_memory(&self.dim, &self.strides); + let offset = offset_from_low_addr_ptr_to_logical_ptr(&self.dim, &self.strides); unsafe { Ok(slice::from_raw_parts_mut( - self.ptr.offset(offset).as_ptr(), + self.ptr.sub(offset).as_ptr(), self.len(), )) } diff --git a/src/impl_owned_array.rs b/src/impl_owned_array.rs index 63af63917..4ac37e829 100644 --- a/src/impl_owned_array.rs +++ b/src/impl_owned_array.rs @@ -61,13 +61,90 @@ impl Array where D: Dimension, { + /// Returns the offset (in units of `A`) from the start of the allocation + /// to the first element, or `None` if the array is empty. + fn offset_from_alloc_to_logical_ptr(&self) -> Option { + if self.is_empty() { + return None; + } + if std::mem::size_of::() == 0 { + Some(dimension::offset_from_low_addr_ptr_to_logical_ptr( + &self.dim, + &self.strides, + )) + } else { + let offset = unsafe { self.as_ptr().offset_from(self.data.as_ptr()) }; + debug_assert!(offset >= 0); + Some(offset as usize) + } + } + /// Return a vector of the elements in the array, in the way they are - /// stored internally. + /// stored internally, and the index in the vector corresponding to the + /// logically first element of the array (or `None` if the array is empty). /// /// If the array is in standard memory layout, the logical element order /// of the array (`.iter()` order) and of the returned vector will be the same. - pub fn into_raw_vec(self) -> Vec { - self.data.into_vec() + /// + /// ``` + /// use ndarray::{array, Array2, Axis}; + /// + /// let mut arr: Array2 = array![[1., 2.], [3., 4.], [5., 6.]]; + /// arr.slice_axis_inplace(Axis(0), (1..).into()); + /// assert_eq!(arr[[0, 0]], 3.); + /// let copy = arr.clone(); + /// + /// let shape = arr.shape().to_owned(); + /// let strides = arr.strides().to_owned(); + /// let (v, offset) = arr.into_raw_vec(); + /// + /// assert_eq!(v, &[1., 2., 3., 4., 5., 6.]); + /// assert_eq!(offset, Some(2)); + /// assert_eq!(v[offset.unwrap()], 3.); + /// for row in 0..shape[0] { + /// for col in 0..shape[1] { + /// let index = ( + /// offset.unwrap() as isize + /// + row as isize * strides[0] + /// + col as isize * strides[1] + /// ) as usize; + /// assert_eq!(v[index], copy[[row, col]]); + /// } + /// } + /// ``` + /// + /// In the case of zero-sized elements, the offset to the logically first + /// element is somewhat meaningless. For convenience, an offset will be + /// returned such that all indices computed using the offset, shape, and + /// strides will be in-bounds for the `Vec`. Note that this offset won't + /// necessarily be the same as the offset for an array of nonzero-sized + /// elements sliced in the same way. + /// + /// ``` + /// use ndarray::{array, Array2, Axis}; + /// + /// let mut arr: Array2<()> = array![[(), ()], [(), ()], [(), ()]]; + /// arr.slice_axis_inplace(Axis(0), (1..).into()); + /// + /// let shape = arr.shape().to_owned(); + /// let strides = arr.strides().to_owned(); + /// let (v, offset) = arr.into_raw_vec(); + /// + /// assert_eq!(v, &[(), (), (), (), (), ()]); + /// for row in 0..shape[0] { + /// for col in 0..shape[1] { + /// let index = ( + /// offset.unwrap() as isize + /// + row as isize * strides[0] + /// + col as isize * strides[1] + /// ) as usize; + /// assert_eq!(v[index], ()); + /// } + /// } + /// ``` + pub fn into_raw_vec(self) -> (Vec, Option) { + let offset = self.offset_from_alloc_to_logical_ptr(); + (self.data.into_vec(), offset) } } @@ -491,13 +568,8 @@ impl Array unsafe { // grow backing storage and update head ptr - let data_to_array_offset = if std::mem::size_of::() != 0 { - self.as_ptr().offset_from(self.data.as_ptr()) - } else { - 0 - }; - debug_assert!(data_to_array_offset >= 0); - self.ptr = self.data.reserve(len_to_append).offset(data_to_array_offset); + let offset_from_alloc_to_logical = self.offset_from_alloc_to_logical_ptr().unwrap_or(0); + self.ptr = self.data.reserve(len_to_append).add(offset_from_alloc_to_logical); // clone elements from view to the array now // diff --git a/src/impl_views/constructors.rs b/src/impl_views/constructors.rs index fd9457fa1..98cb81fcc 100644 --- a/src/impl_views/constructors.rs +++ b/src/impl_views/constructors.rs @@ -13,7 +13,7 @@ use crate::error::ShapeError; use crate::extension::nonnull::nonnull_debug_checked_from_ptr; use crate::imp_prelude::*; use crate::{is_aligned, StrideShape}; -use crate::dimension::offset_from_ptr_to_memory; +use crate::dimension::offset_from_low_addr_ptr_to_logical_ptr; /// Methods for read-only array views. impl<'a, A, D> ArrayView<'a, A, D> @@ -57,7 +57,7 @@ where let dim = shape.dim; dimension::can_index_slice_with_strides(xs, &dim, &shape.strides)?; let strides = shape.strides.strides_for_dim(&dim); - unsafe { Ok(Self::new_(xs.as_ptr().offset(-offset_from_ptr_to_memory(&dim, &strides)), dim, strides)) } + unsafe { Ok(Self::new_(xs.as_ptr().add(offset_from_low_addr_ptr_to_logical_ptr(&dim, &strides)), dim, strides)) } } /// Create an `ArrayView` from shape information and a raw pointer to @@ -154,7 +154,7 @@ where let dim = shape.dim; dimension::can_index_slice_with_strides(xs, &dim, &shape.strides)?; let strides = shape.strides.strides_for_dim(&dim); - unsafe { Ok(Self::new_(xs.as_mut_ptr().offset(-offset_from_ptr_to_memory(&dim, &strides)), dim, strides)) } + unsafe { Ok(Self::new_(xs.as_mut_ptr().add(offset_from_low_addr_ptr_to_logical_ptr(&dim, &strides)), dim, strides)) } } /// Create an `ArrayViewMut` from shape information and a diff --git a/tests/array-construct.rs b/tests/array-construct.rs index 2b72ab039..3b4886ac5 100644 --- a/tests/array-construct.rs +++ b/tests/array-construct.rs @@ -21,9 +21,9 @@ fn test_from_shape_fn() { #[test] fn test_dimension_zero() { let a: Array2 = Array2::from(vec![[], [], []]); - assert_eq!(vec![0.; 0], a.into_raw_vec()); + assert_eq!(vec![0.; 0], a.into_raw_vec().0); let a: Array3 = Array3::from(vec![[[]], [[]], [[]]]); - assert_eq!(vec![0.; 0], a.into_raw_vec()); + assert_eq!(vec![0.; 0], a.into_raw_vec().0); } #[test] diff --git a/tests/array.rs b/tests/array.rs index 976824dfe..f19008ee9 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -1005,7 +1005,7 @@ fn array0_into_scalar() { // With this kind of setup, the `Array`'s pointer is not the same as the // underlying `Vec`'s pointer. let a: Array0 = array![4, 5, 6, 7].index_axis_move(Axis(0), 2); - assert_ne!(a.as_ptr(), a.into_raw_vec().as_ptr()); + assert_ne!(a.as_ptr(), a.into_raw_vec().0.as_ptr()); // `.into_scalar()` should still work correctly. let a: Array0 = array![4, 5, 6, 7].index_axis_move(Axis(0), 2); assert_eq!(a.into_scalar(), 6); @@ -1020,7 +1020,7 @@ fn array_view0_into_scalar() { // With this kind of setup, the `Array`'s pointer is not the same as the // underlying `Vec`'s pointer. let a: Array0 = array![4, 5, 6, 7].index_axis_move(Axis(0), 2); - assert_ne!(a.as_ptr(), a.into_raw_vec().as_ptr()); + assert_ne!(a.as_ptr(), a.into_raw_vec().0.as_ptr()); // `.into_scalar()` should still work correctly. let a: Array0 = array![4, 5, 6, 7].index_axis_move(Axis(0), 2); assert_eq!(a.view().into_scalar(), &6); @@ -1035,7 +1035,7 @@ fn array_view_mut0_into_scalar() { // With this kind of setup, the `Array`'s pointer is not the same as the // underlying `Vec`'s pointer. let a: Array0 = array![4, 5, 6, 7].index_axis_move(Axis(0), 2); - assert_ne!(a.as_ptr(), a.into_raw_vec().as_ptr()); + assert_ne!(a.as_ptr(), a.into_raw_vec().0.as_ptr()); // `.into_scalar()` should still work correctly. let mut a: Array0 = array![4, 5, 6, 7].index_axis_move(Axis(0), 2); assert_eq!(a.view_mut().into_scalar(), &6);