diff --git a/src/dimension/mod.rs b/src/dimension/mod.rs index 4aa7c6641..732006e2b 100644 --- a/src/dimension/mod.rs +++ b/src/dimension/mod.rs @@ -235,11 +235,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 the +/// 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, @@ -407,17 +408,19 @@ 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 { - 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) +/// Returns the offset from the lowest-address element to the logically first +/// element. +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)| { + let s = s as isize; + if s < 0 && d > 1 { + _offset - s * (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 6c51b4515..c441efccd 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::dimension::reshape_dim; @@ -1539,10 +1539,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(), )) } @@ -1568,10 +1568,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_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..c8948d1a3 100644 --- a/tests/array-construct.rs +++ b/tests/array-construct.rs @@ -7,6 +7,7 @@ use defmac::defmac; use ndarray::prelude::*; +use ndarray::arr3; use ndarray::Zip; #[test] @@ -164,6 +165,43 @@ fn test_ones() { assert_eq!(a, b); } +#[test] +fn test_from_shape_empty_with_neg_stride() { + // Issue #998, negative strides for an axis where it doesn't matter. + let s = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]; + let v = s[..12].to_vec(); + let v_ptr = v.as_ptr(); + let a = Array::from_shape_vec((2, 0, 2).strides((1, -4isize as usize, 2)), v).unwrap(); + assert_eq!(a, arr3(&[[[0; 2]; 0]; 2])); + assert_eq!(a.as_ptr(), v_ptr); +} + +#[test] +fn test_from_shape_with_neg_stride() { + // Issue #998, negative strides for an axis where it doesn't matter. + let s = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]; + let v = s[..12].to_vec(); + let v_ptr = v.as_ptr(); + let a = Array::from_shape_vec((2, 1, 2).strides((1, -4isize as usize, 2)), v).unwrap(); + assert_eq!(a, arr3(&[[[0, 2]], + [[1, 3]]])); + assert_eq!(a.as_ptr(), v_ptr); +} + +#[test] +fn test_from_shape_2_2_2_with_neg_stride() { + // Issue #998, negative strides for an axis where it doesn't matter. + let s = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]; + let v = s[..12].to_vec(); + let v_ptr = v.as_ptr(); + let a = Array::from_shape_vec((2, 2, 2).strides((1, -4isize as usize, 2)), v).unwrap(); + assert_eq!(a, arr3(&[[[4, 6], + [0, 2]], + [[5, 7], + [1, 3]]])); + assert_eq!(a.as_ptr(), v_ptr.wrapping_add(4)); +} + #[should_panic] #[test] fn deny_wraparound_zeros() {