Repository: rust-ndarray/ndarray
Pull request #893 (2021-01-13 03:45:04)

Fix to_shared to avoid cloning when it is already ArcArray
From FIXME in to_shared. The current implementation simply calls to_owned().into_shared(), so cloning cannot be avoided when the array is already ArcArray. 
So use to_owned().shared() in a method of Data trait (just like into_owned()) and add a default implementation to it, which will not affect the existing structure. Then modify its implementation in OwnedArcRepr, which implements the Data trait, to avoid cloning when the array is already ArcArray.


Patch:
diff --git a/src/data_traits.rs b/src/data_traits.rs
index 3d6ce88b..ef8a3cea 100644
--- a/src/data_traits.rs
+++ b/src/data_traits.rs
@@ -96,6 +96,18 @@ pub unsafe trait Data: RawData {
     where
         Self::Elem: Clone,
         D: Dimension;
+
+    /// Return a shared ownership (copy on write) array based on the existing one,
+    /// cloning elements if necessary.
+    #[doc(hidden)]
+    fn to_shared<D>(self_: &ArrayBase<Self, D>) -> ArrayBase<OwnedArcRepr<Self::Elem>, D>
+    where
+        Self::Elem: Clone,
+        D: Dimension,
+    {
+        // clone to shared
+        self_.to_owned().into_shared()
+    }
 }
 
 /// Array representation trait.
@@ -238,6 +250,15 @@ unsafe impl<A> Data for OwnedArcRepr<A> {
             strides: self_.strides,
         }
     }
+
+    fn to_shared<D>(self_: &ArrayBase<Self, D>) -> ArrayBase<OwnedArcRepr<Self::Elem>, D>
+    where
+        Self::Elem: Clone,
+        D: Dimension,
+    {
+        // to shared using clone of OwnedArcRepr without clone of raw data.
+        self_.clone()
+    }
 }
 
 unsafe impl<A> DataMut for OwnedArcRepr<A> where A: Clone {}
diff --git a/src/dimension/dimension_trait.rs b/src/dimension/dimension_trait.rs
index d98eedd6..aa1f7f95 100644
--- a/src/dimension/dimension_trait.rs
+++ b/src/dimension/dimension_trait.rs
@@ -287,17 +287,16 @@ pub trait Dimension:
             return true;
         }
         if dim.ndim() == 1 {
-            return false;
+            return strides[0] as isize == -1;
         }
         let order = strides._fastest_varying_stride_order();
         let strides = strides.slice();
 
-        // FIXME: Negative strides
         let dim_slice = dim.slice();
         let mut cstride = 1;
         for &i in order.slice() {
             // a dimension of length 1 can have unequal strides
-            if dim_slice[i] != 1 && strides[i] != cstride {
+            if dim_slice[i] != 1 && (strides[i] as isize).abs() as usize != cstride {
                 return false;
             }
             cstride *= dim_slice[i];
@@ -308,8 +307,7 @@ pub trait Dimension:
     /// Return the axis ordering corresponding to the fastest variation
     /// (in ascending order).
     ///
-    /// Assumes that no stride value appears twice. This cannot yield the correct
-    /// result the strides are not positive.
+    /// Assumes that no stride value appears twice.
     #[doc(hidden)]
     fn _fastest_varying_stride_order(&self) -> Self {
         let mut indices = self.clone();
@@ -317,7 +315,9 @@ pub trait Dimension:
             *elt = i;
         }
         let strides = self.slice();
-        indices.slice_mut().sort_by_key(|&i| strides[i]);
+        indices
+            .slice_mut()
+            .sort_by_key(|&i| (strides[i] as isize).abs());
         indices
     }
 
@@ -646,7 +646,7 @@ impl Dimension for Dim<[Ix; 2]> {
 
     #[inline]
     fn _fastest_varying_stride_order(&self) -> Self {
-        if get!(self, 0) as Ixs <= get!(self, 1) as Ixs {
+        if (get!(self, 0) as Ixs).abs() <= (get!(self, 1) as Ixs).abs() {
             Ix2(0, 1)
         } else {
             Ix2(1, 0)
@@ -806,7 +806,7 @@ impl Dimension for Dim<[Ix; 3]> {
         let mut order = Ix3(0, 1, 2);
         macro_rules! swap {
             ($stride:expr, $order:expr, $x:expr, $y:expr) => {
-                if $stride[$x] > $stride[$y] {
+                if ($stride[$x] as isize).abs() > ($stride[$y] as isize).abs() {
                     $stride.swap($x, $y);
                     $order.ixm().swap($x, $y);
                 }
diff --git a/src/dimension/mod.rs b/src/dimension/mod.rs
index b4610f0d..3b14ea22 100644
--- a/src/dimension/mod.rs
+++ b/src/dimension/mod.rs
@@ -46,15 +46,12 @@ pub fn stride_offset(n: Ix, stride: Ix) -> isize {
 /// There is overlap if, when iterating through the dimensions in order of
 /// increasing stride, the current stride is less than or equal to the maximum
 /// possible offset along the preceding axes. (Axes of length ≤1 are ignored.)
-///
-/// The current implementation assumes that strides of axes with length > 1 are
-/// nonnegative. Additionally, it does not check for overflow.
 pub fn dim_stride_overlap<D: Dimension>(dim: &D, strides: &D) -> bool {
     let order = strides._fastest_varying_stride_order();
     let mut sum_prev_offsets = 0;
     for &index in order.slice() {
         let d = dim[index];
-        let s = strides[index] as isize;
+        let s = (strides[index] as isize).abs();
         match d {
             0 => return false,
             1 => {}
@@ -210,11 +207,7 @@ where
 ///
 /// 2. The product of non-zero axis lengths must not exceed `isize::MAX`.
 ///
-/// 3. For axes with length > 1, the stride must be nonnegative. This is
-///    necessary to make sure the pointer cannot move backwards 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
@@ -225,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,
@@ -248,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));
@@ -257,15 +256,7 @@ fn can_index_slice_impl<D: Dimension>(
         return Err(from_kind(ErrorKind::OutOfBounds));
     }
 
-    // Check condition 3.
-    for (&d, &s) in izip!(dim.slice(), strides.slice()) {
-        let s = s as isize;
-        if d > 1 && s < 0 {
-            return Err(from_kind(ErrorKind::Unsupported));
-        }
-    }
-
-    // Check condition 5.
+    // Check condition 4.
     if !is_empty && dim_stride_overlap(dim, strides) {
         return Err(from_kind(ErrorKind::Unsupported));
     }
@@ -289,6 +280,19 @@ pub fn stride_offset_checked(dim: &[Ix], strides: &[Ix], index: &[Ix]) -> Option
     Some(offset)
 }
 
+/// Checks if strides are non-negative.
+pub fn strides_non_negative<D>(strides: &D) -> Result<(), ShapeError>
+where
+    D: Dimension,
+{
+    for &stride in strides.slice() {
+        if (stride as isize) < 0 {
+            return Err(from_kind(ErrorKind::Unsupported));
+        }
+    }
+    Ok(())
+}
+
 /// Implementation-specific extensions to `Dimension`
 pub trait DimensionExt {
     // note: many extensions go in the main trait if they need to be special-
@@ -394,6 +398,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<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 {
+            _offset
+        }
+    });
+    offset
+}
+
 /// Modify dimension, stride and return data pointer offset
 ///
 /// **Panics** if stride is 0 or if any index is out of bounds.
@@ -693,13 +710,21 @@ mod test {
         let dim = (2, 3, 2).into_dimension();
         let strides = (5, 2, 1).into_dimension();
         assert!(super::dim_stride_overlap(&dim, &strides));
+        let strides = (-5isize as usize, 2, -1isize as usize).into_dimension();
+        assert!(super::dim_stride_overlap(&dim, &strides));
         let strides = (6, 2, 1).into_dimension();
         assert!(!super::dim_stride_overlap(&dim, &strides));
+        let strides = (6, -2isize as usize, 1).into_dimension();
+        assert!(!super::dim_stride_overlap(&dim, &strides));
         let strides = (6, 0, 1).into_dimension();
         assert!(super::dim_stride_overlap(&dim, &strides));
+        let strides = (-6isize as usize, 0, 1).into_dimension();
+        assert!(super::dim_stride_overlap(&dim, &strides));
         let dim = (2, 2).into_dimension();
         let strides = (3, 2).into_dimension();
         assert!(!super::dim_stride_overlap(&dim, &strides));
+        let strides = (3, -2isize as usize).into_dimension();
+        assert!(!super::dim_stride_overlap(&dim, &strides));
     }
 
     #[test]
@@ -736,7 +761,7 @@ mod test {
         can_index_slice::<i32, _>(&[1], &Ix1(2), &Ix1(1)).unwrap_err();
         can_index_slice::<i32, _>(&[1, 2], &Ix1(2), &Ix1(0)).unwrap_err();
         can_index_slice::<i32, _>(&[1, 2], &Ix1(2), &Ix1(1)).unwrap();
-        can_index_slice::<i32, _>(&[1, 2], &Ix1(2), &Ix1(-1isize as usize)).unwrap_err();
+        can_index_slice::<i32, _>(&[1, 2], &Ix1(2), &Ix1(-1isize as usize)).unwrap();
     }
 
     #[test]
diff --git a/src/impl_constructors.rs b/src/impl_constructors.rs
index 4d76490c..622448c5 100644
--- a/src/impl_constructors.rs
+++ b/src/impl_constructors.rs
@@ -19,6 +19,7 @@ use alloc::vec;
 use alloc::vec::Vec;
 
 use crate::dimension;
+use crate::dimension::offset_from_ptr_to_memory;
 use crate::error::{self, ShapeError};
 use crate::extension::nonnull::nonnull_from_vec_data;
 use crate::imp_prelude::*;
@@ -30,6 +31,7 @@ use crate::iterators::to_vec_mapped;
 use crate::StrideShape;
 #[cfg(feature = "std")]
 use crate::{geomspace, linspace, logspace};
+use rawpointer::PointerExt;
 
 
 /// # Constructor Methods for Owned Arrays
@@ -442,7 +444,8 @@ where
     ///
     /// 2. The product of non-zero axis lengths must not exceed `isize::MAX`.
     ///
-    /// 3. For axes with length > 1, the stride must be nonnegative.
+    /// 3. For axes with length > 1, the pointer cannot move outside the
+    ///    slice.
     ///
     /// 4. If the array will be empty (any axes are zero-length), the
     ///    difference between the least address and greatest address accessible
@@ -468,7 +471,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),
+            ptr: nonnull_from_vec_data(&mut v).offset(-offset_from_ptr_to_memory(&dim, &strides)),
             data: DataOwned::new(v),
             strides,
             dim,
@@ -494,7 +497,7 @@ where
     ///
     /// This constructor is limited to elements where `A: Copy` (no destructors)
     /// to avoid users shooting themselves too hard in the foot.
-    /// 
+    ///
     /// (Also note that the constructors `from_shape_vec` and
     /// `from_shape_vec_unchecked` allow the user yet more control, in the sense
     /// that Arrays can be created from arbitrary vectors.)
diff --git a/src/impl_methods.rs b/src/impl_methods.rs
index 3afb6814..c9ad9bb0 100644
--- a/src/impl_methods.rs
+++ b/src/impl_methods.rs
@@ -18,7 +18,8 @@ use crate::arraytraits;
 use crate::dimension;
 use crate::dimension::IntoDimension;
 use crate::dimension::{
-    abs_index, axes_of, do_slice, merge_axes, size_of_shape_checked, stride_offset, Axes,
+    abs_index, axes_of, do_slice, merge_axes, offset_from_ptr_to_memory, size_of_shape_checked,
+    stride_offset, Axes,
 };
 use crate::error::{self, ErrorKind, ShapeError};
 use crate::math_cell::MathCell;
@@ -169,12 +170,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::*;
@@ -214,14 +215,14 @@ where
         }
     }
 
-    /// Return a shared ownership (copy on write) array.
+    /// Return a shared ownership (copy on write) array, cloning the array
+    /// elements if necessary.
     pub fn to_shared(&self) -> ArcArray<A, D>
     where
         A: Clone,
         S: Data,
     {
-        // FIXME: Avoid copying if it’s already an ArcArray.
-        self.to_owned().into_shared()
+        S::to_shared(self)
     }
 
     /// Turn the array into a uniquely owned array, cloning the array elements
@@ -1296,9 +1297,6 @@ where
     }
 
     /// Return true if the array is known to be contiguous.
-    ///
-    /// Will detect c- and f-contig arrays correctly, but otherwise
-    /// There are some false negatives.
     pub(crate) fn is_contiguous(&self) -> bool {
         D::is_contiguous(&self.dim, &self.strides)
     }
@@ -1420,14 +1418,18 @@ where
     ///
     /// If this function returns `Some(_)`, then the elements in the slice
     /// have whatever order the elements have in memory.
-    ///
-    /// Implementation notes: Does not yet support negatively strided arrays.
     pub fn as_slice_memory_order(&self) -> Option<&[A]>
     where
         S: Data,
     {
         if self.is_contiguous() {
-            unsafe { Some(slice::from_raw_parts(self.ptr.as_ptr(), self.len())) }
+            let offset = offset_from_ptr_to_memory(&self.dim, &self.strides);
+            unsafe {
+                Some(slice::from_raw_parts(
+                    self.ptr.offset(offset).as_ptr(),
+                    self.len(),
+                ))
+            }
         } else {
             None
         }
@@ -1441,7 +1443,13 @@ where
     {
         if self.is_contiguous() {
             self.ensure_unique();
-            unsafe { Some(slice::from_raw_parts_mut(self.ptr.as_ptr(), self.len())) }
+            let offset = offset_from_ptr_to_memory(&self.dim, &self.strides);
+            unsafe {
+                Some(slice::from_raw_parts_mut(
+                    self.ptr.offset(offset).as_ptr(),
+                    self.len(),
+                ))
+            }
         } else {
             None
         }
diff --git a/src/impl_raw_views.rs b/src/impl_raw_views.rs
index 8377dc93..e3447cac 100644
--- a/src/impl_raw_views.rs
+++ b/src/impl_raw_views.rs
@@ -62,6 +62,11 @@ where
     ///     [`.offset()`] regardless of the starting point due to past offsets.
     ///
     /// * The product of non-zero axis lengths must not exceed `isize::MAX`.
+    /// 
+    /// * Strides must be non-negative.
+    ///
+    /// This function can use debug assertions to check some of these requirements,
+    /// but it's not a complete check.
     ///
     /// [`.offset()`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.offset
     pub unsafe fn from_shape_ptr<Sh>(shape: Sh, ptr: *const A) -> Self
@@ -73,6 +78,7 @@ where
         if cfg!(debug_assertions) {
             assert!(!ptr.is_null(), "The pointer must be non-null.");
             if let Strides::Custom(strides) = &shape.strides {
+                dimension::strides_non_negative(strides).unwrap();
                 dimension::max_abs_offset_check_overflow::<A, _>(&dim, &strides).unwrap();
             } else {
                 dimension::size_of_shape_checked(&dim).unwrap();
@@ -202,6 +208,11 @@ where
     ///     [`.offset()`] regardless of the starting point due to past offsets.
     ///
     /// * The product of non-zero axis lengths must not exceed `isize::MAX`.
+    /// 
+    /// * Strides must be non-negative.
+    ///
+    /// This function can use debug assertions to check some of these requirements,
+    /// but it's not a complete check.
     ///
     /// [`.offset()`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.offset
     pub unsafe fn from_shape_ptr<Sh>(shape: Sh, ptr: *mut A) -> Self
@@ -213,6 +224,7 @@ where
         if cfg!(debug_assertions) {
             assert!(!ptr.is_null(), "The pointer must be non-null.");
             if let Strides::Custom(strides) = &shape.strides {
+                dimension::strides_non_negative(strides).unwrap();
                 dimension::max_abs_offset_check_overflow::<A, _>(&dim, &strides).unwrap();
             } else {
                 dimension::size_of_shape_checked(&dim).unwrap();
diff --git a/src/impl_views/constructors.rs b/src/impl_views/constructors.rs
index bc1602af..c6e5f998 100644
--- a/src/impl_views/constructors.rs
+++ b/src/impl_views/constructors.rs
@@ -96,6 +96,11 @@ where
     ///
     /// * The product of non-zero axis lengths must not exceed `isize::MAX`.
     ///
+    /// * Strides must be non-negative.
+    ///
+    /// This function can use debug assertions to check some of these requirements,
+    /// but it's not a complete check.
+    ///
     /// [`.offset()`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.offset
     pub unsafe fn from_shape_ptr<Sh>(shape: Sh, ptr: *const A) -> Self
     where
@@ -188,6 +193,11 @@ where
     ///
     /// * The product of non-zero axis lengths must not exceed `isize::MAX`.
     ///
+    /// * Strides must be non-negative.
+    ///
+    /// This function can use debug assertions to check some of these requirements,
+    /// but it's not a complete check.
+    ///
     /// [`.offset()`]: https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.offset
     pub unsafe fn from_shape_ptr<Sh>(shape: Sh, ptr: *mut A) -> Self
     where


Test patch:
diff --git a/blas-tests/tests/oper.rs b/blas-tests/tests/oper.rs
index 2741123f..b7c2d769 100644
--- a/blas-tests/tests/oper.rs
+++ b/blas-tests/tests/oper.rs
@@ -173,7 +173,7 @@ where
     S2: Data<Elem = A>,
 {
     let ((m, _), k) = (lhs.dim(), rhs.dim());
-    reference_mat_mul(lhs, &rhs.to_owned().into_shape((k, 1)).unwrap())
+    reference_mat_mul(lhs, &rhs.as_standard_layout().into_shape((k, 1)).unwrap())
         .into_shape(m)
         .unwrap()
 }
@@ -186,7 +186,7 @@ where
     S2: Data<Elem = A>,
 {
     let (m, (_, n)) = (lhs.dim(), rhs.dim());
-    reference_mat_mul(&lhs.to_owned().into_shape((1, m)).unwrap(), rhs)
+    reference_mat_mul(&lhs.as_standard_layout().into_shape((1, m)).unwrap(), rhs)
         .into_shape(n)
         .unwrap()
 }
diff --git a/tests/array.rs b/tests/array.rs
index c1976511..6581e572 100644
--- a/tests/array.rs
+++ b/tests/array.rs
@@ -1722,10 +1722,18 @@ fn to_owned_memory_order() {
     // input.
     let c = arr2(&[[1, 2, 3], [4, 5, 6]]);
     let mut f = c.view();
+
+    // transposed array
     f.swap_axes(0, 1);
     let fo = f.to_owned();
     assert_eq!(f, fo);
     assert_eq!(f.strides(), fo.strides());
+
+    // negated stride axis
+    f.invert_axis(Axis(1));
+    let fo2 = f.to_owned();
+    assert_eq!(f, fo2);
+    assert_eq!(f.strides(), fo2.strides());
 }
 
 #[test]
@@ -1734,6 +1742,7 @@ fn to_owned_neg_stride() {
     c.slice_collapse(s![.., ..;-1]);
     let co = c.to_owned();
     assert_eq!(c, co);
+    assert_eq!(c.strides(), co.strides());
 }
 
 #[test]
@@ -1791,6 +1800,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]]);
diff --git a/tests/dimension.rs b/tests/dimension.rs
index cfc122e4..7f1ee7ec 100644
--- a/tests/dimension.rs
+++ b/tests/dimension.rs
@@ -118,19 +118,36 @@ fn fastest_varying_order() {
     let order = strides._fastest_varying_stride_order();
     assert_eq!(order.slice(), &[3, 0, 2, 1]);
 
+    let strides = Dim([-2isize as usize, 8, -4isize as usize, -1isize as usize]);
+    let order = strides._fastest_varying_stride_order();
+    assert_eq!(order.slice(), &[3, 0, 2, 1]);
+
     assert_eq!(Dim([1, 3])._fastest_varying_stride_order(), Dim([0, 1]));
+    assert_eq!(
+        Dim([1, -3isize as usize])._fastest_varying_stride_order(),
+        Dim([0, 1])
+    );
     assert_eq!(Dim([7, 2])._fastest_varying_stride_order(), Dim([1, 0]));
+    assert_eq!(
+        Dim([-7isize as usize, 2])._fastest_varying_stride_order(),
+        Dim([1, 0])
+    );
     assert_eq!(
         Dim([6, 1, 3])._fastest_varying_stride_order(),
         Dim([1, 2, 0])
     );
+    assert_eq!(
+        Dim([-6isize as usize, 1, -3isize as usize])._fastest_varying_stride_order(),
+        Dim([1, 2, 0])
+    );
 
     // it's important that it produces distinct indices. Prefer the stable order
     // where 0 is before 1 when they are equal.
     assert_eq!(Dim([2, 2])._fastest_varying_stride_order(), [0, 1]);
     assert_eq!(Dim([2, 2, 1])._fastest_varying_stride_order(), [2, 0, 1]);
     assert_eq!(
-        Dim([2, 2, 3, 1, 2])._fastest_varying_stride_order(),
+        Dim([-2isize as usize, -2isize as usize, 3, 1, -2isize as usize])
+            ._fastest_varying_stride_order(),
         [3, 0, 1, 4, 2]
     );
 }
diff --git a/tests/oper.rs b/tests/oper.rs
index a3d17976..66c24c7f 100644
--- a/tests/oper.rs
+++ b/tests/oper.rs
@@ -707,7 +707,7 @@ fn gen_mat_vec_mul() {
         S2: Data<Elem = A>,
     {
         let ((m, _), k) = (lhs.dim(), rhs.dim());
-        reference_mat_mul(lhs, &rhs.to_owned().into_shape((k, 1)).unwrap())
+        reference_mat_mul(lhs, &rhs.as_standard_layout().into_shape((k, 1)).unwrap())
             .into_shape(m)
             .unwrap()
     }
@@ -772,7 +772,7 @@ fn vec_mat_mul() {
         S2: Data<Elem = A>,
     {
         let (m, (_, n)) = (lhs.dim(), rhs.dim());
-        reference_mat_mul(&lhs.to_owned().into_shape((1, m)).unwrap(), rhs)
+        reference_mat_mul(&lhs.as_standard_layout().into_shape((1, m)).unwrap(), rhs)
             .into_shape(n)
             .unwrap()
     }
diff --git a/tests/raw_views.rs b/tests/raw_views.rs
index b63e4292..b955cac6 100644
--- a/tests/raw_views.rs
+++ b/tests/raw_views.rs
@@ -81,3 +81,18 @@ fn raw_view_deref_into_view_misaligned() {
     let data: [u16; 2] = [0x0011, 0x2233];
     misaligned_deref(&data);
 }
+
+#[test]
+#[cfg(debug_assertions)]
+#[should_panic = "Unsupported"]
+fn raw_view_negative_strides() {
+    fn misaligned_deref(data: &[u16; 2]) -> ArrayView1<'_, u16> {
+        let ptr: *const u16 = data.as_ptr();
+        unsafe {
+            let raw_view = RawArrayView::from_shape_ptr(1.strides((-1isize) as usize), ptr);
+            raw_view.deref_into_view()
+        }
+    }
+    let data: [u16; 2] = [0x0011, 0x2233];
+    misaligned_deref(&data);
+}