Added memory continuity test with negative step size. Update documentation

Author: SparrowLii

src/dimension/dimension_trait.rs

@@ -314,7 +314,8 @@ pub trait Dimension:
             *elt = i;
         }
         let strides = self.slice();
-        indices.slice_mut()
+        indices
+            .slice_mut()
             .sort_by_key(|&i| (strides[i] as isize).abs());
         indices
     }

src/dimension/mod.rs

@@ -207,10 +207,7 @@ where
 ///
 /// 2. The product of non-zero axis lengths must not exceed `isize::MAX`.
 ///
-/// 3. For axes with length > 1, the pointer cannot move outside the
-///    slice. For axes with length ≤ 1, the stride can be anything.
-///
-/// 4. If the array will be empty (any axes are zero-length), the difference
+/// 3. If the array will be empty (any axes are zero-length), the difference
 ///    between the least address and greatest address accessible by moving
 ///    along all axes must be ≤ `data.len()`. (It's fine in this case to move
 ///    one byte past the end of the slice since the pointers will be offset but
@@ -221,13 +218,19 @@ where
 ///    `data.len()`. This and #3 ensure that all dereferenceable pointers point
 ///    to elements within the slice.
 ///
-/// 5. The strides must not allow any element to be referenced by two different
+/// 4. The strides must not allow any element to be referenced by two different
 ///    indices.
 ///
 /// Note that since slices cannot contain more than `isize::MAX` bytes,
 /// condition 4 is sufficient to guarantee that the absolute difference in
 /// 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.)
 pub(crate) fn can_index_slice<A, D: Dimension>(
     data: &[A],
     dim: &D,
@@ -244,7 +247,7 @@ fn can_index_slice_impl<D: Dimension>(
     dim: &D,
     strides: &D,
 ) -> Result<(), ShapeError> {
-    // Check condition 4.
+    // Check condition 3.
     let is_empty = dim.slice().iter().any(|&d| d == 0);
     if is_empty && max_offset > data_len {
         return Err(from_kind(ErrorKind::OutOfBounds));
@@ -253,7 +256,7 @@ fn can_index_slice_impl<D: Dimension>(
         return Err(from_kind(ErrorKind::OutOfBounds));
     }
 
-    // Check condition 5.
+    // Check condition 4.
     if !is_empty && dim_stride_overlap(dim, strides) {
         return Err(from_kind(ErrorKind::Unsupported));
     }
@@ -384,8 +387,8 @@ fn to_abs_slice(axis_len: usize, slice: Slice) -> (usize, usize, isize) {
 
 /// 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: &[Ix], strides: &[Ix]) -> isize {
-    let offset = izip!(dim, strides).fold(0, |_offset, (d, s)| {
+pub fn offset_from_ptr_to_memory<D: Dimension>(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)
         } else {

src/impl_constructors.rs

@@ -460,8 +460,7 @@ where
         // 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.slice(), strides.slice()).abs()),
+            ptr: nonnull_from_vec_data(&mut v).offset(-offset_from_ptr_to_memory(&dim, &strides)),
             data: DataOwned::new(v),
             strides,
             dim,

src/impl_methods.rs

@@ -154,12 +154,12 @@ where
 
     /// Return an uniquely owned copy of the array.
     ///
-    /// If the input array is contiguous and its strides are positive, then the
-    /// output array will have the same memory layout. Otherwise, the layout of
-    /// the output array is unspecified. If you need a particular layout, you
-    /// can allocate a new array with the desired memory layout and
-    /// [`.assign()`](#method.assign) the data. Alternatively, you can collect
-    /// an iterator, like this for a result in standard layout:
+    /// If the input array is contiguous, then the output array will have the same
+    /// memory layout. Otherwise, the layout of the output array is unspecified.
+    /// If you need a particular layout, you can allocate a new array with the
+    /// desired memory layout and [`.assign()`](#method.assign) the data.
+    /// Alternatively, you can collectan iterator, like this for a result in
+    /// standard layout:
     ///
     /// ```
     /// # use ndarray::prelude::*;
@@ -1407,7 +1407,7 @@ where
         S: Data,
     {
         if self.is_contiguous() {
-            let offset = offset_from_ptr_to_memory(self.dim.slice(), self.strides.slice());
+            let offset = offset_from_ptr_to_memory(&self.dim, &self.strides);
             unsafe {
                 Some(slice::from_raw_parts(
                     self.ptr.offset(offset).as_ptr(),
@@ -1427,7 +1427,7 @@ where
     {
         if self.is_contiguous() {
             self.ensure_unique();
-            let offset = offset_from_ptr_to_memory(self.dim.slice(), self.strides.slice());
+            let offset = offset_from_ptr_to_memory(&self.dim, &self.strides);
             unsafe {
                 Some(slice::from_raw_parts_mut(
                     self.ptr.offset(offset).as_ptr(),

tests/array.rs

@@ -1786,6 +1786,64 @@ fn test_contiguous() {
     assert!(b.as_slice_memory_order().is_some());
 }
 
+#[test]
+fn test_contiguous_neg_strides() {
+    let s = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13];
+    let mut a = ArrayView::from_shape((2, 3, 2).strides((1, 4, 2)), &s).unwrap();
+    assert_eq!(
+        a,
+        arr3(&[[[0, 2], [4, 6], [8, 10]], [[1, 3], [5, 7], [9, 11]]])
+    );
+    assert!(a.as_slice_memory_order().is_some());
+
+    let mut b = a.slice(s![..;1, ..;-1, ..;-1]);
+    assert_eq!(
+        b,
+        arr3(&[[[10, 8], [6, 4], [2, 0]], [[11, 9], [7, 5], [3, 1]]])
+    );
+    assert!(b.as_slice_memory_order().is_some());
+
+    b.swap_axes(1, 2);
+    assert_eq!(b, arr3(&[[[10, 6, 2], [8, 4, 0]], [[11, 7, 3], [9, 5, 1]]]));
+    assert!(b.as_slice_memory_order().is_some());
+
+    b.invert_axis(Axis(0));
+    assert_eq!(b, arr3(&[[[11, 7, 3], [9, 5, 1]], [[10, 6, 2], [8, 4, 0]]]));
+    assert!(b.as_slice_memory_order().is_some());
+
+    let mut c = b.reversed_axes();
+    assert_eq!(
+        c,
+        arr3(&[[[11, 10], [9, 8]], [[7, 6], [5, 4]], [[3, 2], [1, 0]]])
+    );
+    assert!(c.as_slice_memory_order().is_some());
+
+    c.merge_axes(Axis(1), Axis(2));
+    assert_eq!(c, arr3(&[[[11, 10, 9, 8]], [[7, 6, 5, 4]], [[3, 2, 1, 0]]]));
+    assert!(c.as_slice_memory_order().is_some());
+
+    let d = b.remove_axis(Axis(1));
+    assert_eq!(d, arr2(&[[11, 7, 3], [10, 6, 2]]));
+    assert!(d.as_slice_memory_order().is_none());
+
+    let e = b.remove_axis(Axis(2));
+    assert_eq!(e, arr2(&[[11, 9], [10, 8]]));
+    assert!(e.as_slice_memory_order().is_some());
+
+    let f = e.insert_axis(Axis(2));
+    assert_eq!(f, arr3(&[[[11], [9]], [[10], [8]]]));
+    assert!(f.as_slice_memory_order().is_some());
+
+    let mut g = b.clone();
+    g.collapse_axis(Axis(1), 0);
+    assert_eq!(g, arr3(&[[[11, 7, 3]], [[10, 6, 2]]]));
+    assert!(g.as_slice_memory_order().is_none());
+
+    b.collapse_axis(Axis(2), 0);
+    assert_eq!(b, arr3(&[[[11], [9]], [[10], [8]]]));
+    assert!(b.as_slice_memory_order().is_some());
+}
+
 #[test]
 fn test_swap() {
     let mut a = arr2(&[[1, 2, 3], [4, 5, 6], [7, 8, 9]]);