From b228e31e016bbe7fdafb22dd99e906c1ac4dd3df Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Wed, 18 Sep 2019 18:27:36 -0400 Subject: [PATCH 01/10] Add multi_slice_* methods --- src/impl_methods.rs | 34 +++++++ src/impl_views/splitting.rs | 26 +++++ src/lib.rs | 21 ++++ src/slice.rs | 189 +++++++++++++++++++++++++++++++++++- tests/array.rs | 65 +++++++++++++ 5 files changed, 334 insertions(+), 1 deletion(-) diff --git a/src/impl_methods.rs b/src/impl_methods.rs index 97193de36..e68c5c42f 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -28,6 +28,7 @@ use crate::iter::{ AxisChunksIter, AxisChunksIterMut, AxisIter, AxisIterMut, ExactChunks, ExactChunksMut, IndexedIter, IndexedIterMut, Iter, IterMut, Lanes, LanesMut, Windows, }; +use crate::slice::MultiSlice; use crate::stacking::stack; use crate::{NdIndex, Slice, SliceInfo, SliceOrIndex}; @@ -350,6 +351,39 @@ where self.view_mut().slice_move(info) } + /// Return multiple disjoint, sliced, mutable views of the array. + /// + /// See [*Slicing*](#slicing) for full documentation. + /// See also [`SliceInfo`] and [`D::SliceArg`]. + /// + /// [`SliceInfo`]: struct.SliceInfo.html + /// [`D::SliceArg`]: trait.Dimension.html#associatedtype.SliceArg + /// + /// **Panics** if any of the following occur: + /// + /// * if any of the views would intersect (i.e. if any element would appear in multiple slices) + /// * if an index is out of bounds or step size is zero + /// * if `D` is `IxDyn` and `info` does not match the number of array axes + /// + /// # Example + /// + /// ``` + /// use ndarray::{arr2, s}; + /// + /// let mut a = arr2(&[[1, 2, 3], [4, 5, 6]]); + /// let (mut edges, mut middle) = a.multi_slice_mut((s![.., ..;2], s![.., 1])); + /// edges.fill(1); + /// middle.fill(0); + /// assert_eq!(a, arr2(&[[1, 0, 1], [1, 0, 1]])); + /// ``` + pub fn multi_slice_mut<'a, M>(&'a mut self, info: M) -> M::Output + where + M: MultiSlice<'a, A, D>, + S: DataMut, + { + unsafe { info.slice_and_deref(self.raw_view_mut()) } + } + /// Slice the array, possibly changing the number of dimensions. /// /// See [*Slicing*](#slicing) for full documentation. diff --git a/src/impl_views/splitting.rs b/src/impl_views/splitting.rs index e9fcfcb64..e72a8d44f 100644 --- a/src/impl_views/splitting.rs +++ b/src/impl_views/splitting.rs @@ -7,6 +7,7 @@ // except according to those terms. use crate::imp_prelude::*; +use crate::slice::MultiSlice; /// Methods for read-only array views. impl<'a, A, D> ArrayView<'a, A, D> @@ -109,4 +110,29 @@ where (left.deref_into_view_mut(), right.deref_into_view_mut()) } } + + /// Split the view into multiple disjoint slices. + /// + /// This is similar to [`.multi_slice_mut()`], but `.multi_slice_move()` + /// consumes `self` and produces views with lifetimes matching that of + /// `self`. + /// + /// See [*Slicing*](#slicing) for full documentation. + /// See also [`SliceInfo`] and [`D::SliceArg`]. + /// + /// [`.multi_slice_mut()`]: struct.ArrayBase.html#method.multi_slice_mut + /// [`SliceInfo`]: struct.SliceInfo.html + /// [`D::SliceArg`]: trait.Dimension.html#associatedtype.SliceArg + /// + /// **Panics** if any of the following occur: + /// + /// * if any of the views would intersect (i.e. if any element would appear in multiple slices) + /// * if an index is out of bounds or step size is zero + /// * if `D` is `IxDyn` and `info` does not match the number of array axes + pub fn multi_slice_move(mut self, info: M) -> M::Output + where + M: MultiSlice<'a, A, D>, + { + unsafe { info.slice_and_deref(self.raw_view_mut()) } + } } diff --git a/src/lib.rs b/src/lib.rs index 35d1e1aab..badb9dcb0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -475,6 +475,13 @@ pub type Ixs = isize; /// [`.slice_move()`]: #method.slice_move /// [`.slice_collapse()`]: #method.slice_collapse /// +/// It's possible to take multiple simultaneous *mutable* slices with the +/// [`.multi_slice_mut()`] or (for [`ArrayViewMut`] only) +/// [`.multi_slice_move()`]. +/// +/// [`.multi_slice_mut()`]: #method.multi_slice_mut +/// [`.multi_slice_move()`]: type.ArrayViewMut.html#method.multi_slice_move +/// /// ``` /// extern crate ndarray; /// @@ -525,6 +532,20 @@ pub type Ixs = isize; /// [12, 11, 10]]); /// assert_eq!(f, g); /// assert_eq!(f.shape(), &[2, 3]); +/// +/// // Let's take two disjoint, mutable slices of a matrix with +/// // +/// // - One containing all the even-index columns in the matrix +/// // - One containing all the odd-index columns in the matrix +/// let mut h = arr2(&[[0, 1, 2, 3], +/// [4, 5, 6, 7]]); +/// let (s0, s1) = h.multi_slice_mut((s![.., ..;2], s![.., 1..;2])); +/// let i = arr2(&[[0, 2], +/// [4, 6]]); +/// let j = arr2(&[[1, 3], +/// [5, 7]]); +/// assert_eq!(s0, i); +/// assert_eq!(s1, j); /// } /// ``` /// diff --git a/src/slice.rs b/src/slice.rs index 1d0dfa2b0..b2731e1cf 100644 --- a/src/slice.rs +++ b/src/slice.rs @@ -5,8 +5,9 @@ // , at your // option. This file may not be copied, modified, or distributed // except according to those terms. +use crate::dimension::slices_intersect; use crate::error::{ErrorKind, ShapeError}; -use crate::Dimension; +use crate::{ArrayViewMut, Dimension, RawArrayViewMut}; use std::fmt; use std::marker::PhantomData; use std::ops::{Deref, Range, RangeFrom, RangeFull, RangeInclusive, RangeTo, RangeToInclusive}; @@ -629,3 +630,189 @@ macro_rules! s( &*&$crate::s![@parse ::std::marker::PhantomData::<$crate::Ix0>, [] $($t)*] }; ); + +/// Slicing information describing multiple mutable, disjoint slices. +/// +/// It's unfortunate that we need `'out` and `A` to be parameters of the trait, +/// but they're necessary until Rust supports generic associated types. +/// +/// # Safety +/// +/// Implementers of this trait must ensure that: +/// +/// * `.slice_and_deref()` panics or aborts if the slices would intersect, and +/// +/// * the `.intersects_self()`, `.intersects_indices()`, and +/// `.intersects_other()` implementations are correct. +pub unsafe trait MultiSlice<'out, A, D> +where + A: 'out, + D: Dimension, +{ + /// The type of the slices created by `.slice_and_deref()`. + type Output; + + /// Slice the raw view into multiple raw views, and dereference them. + /// + /// **Panics** if performing any individual slice panics or if the slices + /// are not disjoint (i.e. if they intersect). + /// + /// # Safety + /// + /// The caller must ensure that it is safe to mutably dereference the view + /// using the lifetime `'out`. + unsafe fn slice_and_deref(&self, view: RawArrayViewMut) -> Self::Output; + + /// Returns `true` if slicing an array of the specified `shape` with `self` + /// would result in intersecting slices. + /// + /// If `self.intersects_self(&view.raw_dim())` is `true`, then + /// `self.slice_and_deref(view)` must panic. + fn intersects_self(&self, shape: &D) -> bool; + + /// Returns `true` if any slices created by slicing an array of the + /// specified `shape` with `self` would intersect with the specified + /// indices. + /// + /// Note that even if this returns `false`, `self.intersects_self(shape)` + /// may still return `true`. (`.intersects_indices()` doesn't check for + /// intersections within `self`; it only checks for intersections between + /// `self` and `indices`.) + fn intersects_indices(&self, shape: &D, indices: &D::SliceArg) -> bool; + + /// Returns `true` if any slices created by slicing an array of the + /// specified `shape` with `self` would intersect any slices created by + /// slicing the array with `other`. + /// + /// Note that even if this returns `false`, `self.intersects_self(shape)` + /// or `other.intersects_self(shape)` may still return `true`. + /// (`.intersects_other()` doesn't check for intersections within `self` or + /// within `other`; it only checks for intersections between `self` and + /// `other`.) + fn intersects_other(&self, shape: &D, other: impl MultiSlice<'out, A, D>) -> bool; +} + +unsafe impl<'out, A, D, Do> MultiSlice<'out, A, D> for SliceInfo +where + A: 'out, + D: Dimension, + Do: Dimension, +{ + type Output = ArrayViewMut<'out, A, Do>; + + unsafe fn slice_and_deref(&self, view: RawArrayViewMut) -> Self::Output { + view.slice_move(self).deref_into_view_mut() + } + + fn intersects_self(&self, _shape: &D) -> bool { + false + } + + fn intersects_indices(&self, shape: &D, indices: &D::SliceArg) -> bool { + slices_intersect(shape, &*self, indices) + } + + fn intersects_other(&self, shape: &D, other: impl MultiSlice<'out, A, D>) -> bool { + other.intersects_indices(shape, &*self) + } +} + +unsafe impl<'out, A, D> MultiSlice<'out, A, D> for () +where + A: 'out, + D: Dimension, +{ + type Output = (); + + unsafe fn slice_and_deref(&self, _view: RawArrayViewMut) -> Self::Output {} + + fn intersects_self(&self, _shape: &D) -> bool { + false + } + + fn intersects_indices(&self, _shape: &D, _indices: &D::SliceArg) -> bool { + false + } + + fn intersects_other(&self, _shape: &D, _other: impl MultiSlice<'out, A, D>) -> bool { + false + } +} + +macro_rules! impl_multislice_tuple { + ($($T:ident,)*) => { + unsafe impl<'out, A, D, $($T,)*> MultiSlice<'out, A, D> for ($($T,)*) + where + A: 'out, + D: Dimension, + $($T: MultiSlice<'out, A, D>,)* + { + type Output = ($($T::Output,)*); + + unsafe fn slice_and_deref(&self, view: RawArrayViewMut) -> Self::Output { + assert!(!self.intersects_self(&view.raw_dim())); + + #[allow(non_snake_case)] + let ($($T,)*) = self; + ($($T.slice_and_deref(view.clone()),)*) + } + + fn intersects_self(&self, shape: &D) -> bool { + #[allow(non_snake_case)] + let ($($T,)*) = self; + impl_multislice_tuple!(@intersects_self shape, ($($T,)*)) + } + + fn intersects_indices(&self, shape: &D, indices: &D::SliceArg) -> bool { + #[allow(non_snake_case)] + let ($($T,)*) = self; + $($T.intersects_indices(shape, indices)) ||* + } + + fn intersects_other(&self, shape: &D, other: impl MultiSlice<'out, A, D>) -> bool { + #[allow(non_snake_case)] + let ($($T,)*) = self; + $($T.intersects_other(shape, &other)) ||* + } + } + }; + (@intersects_self $shape:expr, ($head:expr,)) => { + $head.intersects_self($shape) + }; + (@intersects_self $shape:expr, ($head:expr, $($tail:expr,)*)) => { + $head.intersects_self($shape) || + $($head.intersects_other($shape, &$tail)) ||* || + impl_multislice_tuple!(@intersects_self $shape, ($($tail,)*)) + }; +} +impl_multislice_tuple!(T0,); +impl_multislice_tuple!(T0, T1,); +impl_multislice_tuple!(T0, T1, T2,); +impl_multislice_tuple!(T0, T1, T2, T3,); +impl_multislice_tuple!(T0, T1, T2, T3, T4,); +impl_multislice_tuple!(T0, T1, T2, T3, T4, T5,); + +unsafe impl<'out, A, D, T> MultiSlice<'out, A, D> for &'_ T +where + A: 'out, + D: Dimension, + T: MultiSlice<'out, A, D>, +{ + type Output = T::Output; + + unsafe fn slice_and_deref(&self, view: RawArrayViewMut) -> Self::Output { + T::slice_and_deref(self, view) + } + + fn intersects_self(&self, shape: &D) -> bool { + T::intersects_self(self, shape) + } + + fn intersects_indices(&self, shape: &D, indices: &D::SliceArg) -> bool { + T::intersects_indices(self, shape, indices) + } + + fn intersects_other(&self, shape: &D, other: impl MultiSlice<'out, A, D>) -> bool { + T::intersects_other(self, shape, other) + } +} diff --git a/tests/array.rs b/tests/array.rs index ea374bc7c..8aaa697c2 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -15,6 +15,20 @@ use ndarray::{arr3, rcarr2}; use ndarray::{Slice, SliceInfo, SliceOrIndex}; use std::iter::FromIterator; +macro_rules! assert_panics { + ($body:expr) => { + if let Ok(v) = ::std::panic::catch_unwind(|| $body) { + panic!("assertion failed: should_panic; \ + non-panicking result: {:?}", v); + } + }; + ($body:expr, $($arg:tt)*) => { + if let Ok(_) = ::std::panic::catch_unwind(|| $body) { + panic!($($arg)*); + } + }; +} + #[test] fn test_matmul_arcarray() { let mut A = ArcArray::::zeros((2, 3)); @@ -328,6 +342,57 @@ fn test_slice_collapse_with_indices() { assert_eq!(vi, Array3::from_elem((1, 1, 1), elem)); } +#[test] +#[allow(clippy::cognitive_complexity)] +fn test_multislice() { + defmac!(test_multislice arr, s1, s2 => { + let copy = arr.clone(); + assert_eq!( + arr.multi_slice_mut((s1, s2)), + (copy.clone().slice_mut(s1), copy.clone().slice_mut(s2)) + ); + }); + let mut arr = Array1::from_iter(0..48).into_shape((8, 6)).unwrap(); + + assert_eq!(arr.clone().view(), arr.multi_slice_mut(s![.., ..])); + test_multislice!(&mut arr, s![0, ..], s![1, ..]); + test_multislice!(&mut arr, s![0, ..], s![-1, ..]); + test_multislice!(&mut arr, s![0, ..], s![1.., ..]); + test_multislice!(&mut arr, s![1, ..], s![..;2, ..]); + test_multislice!(&mut arr, s![..2, ..], s![2.., ..]); + test_multislice!(&mut arr, s![1..;2, ..], s![..;2, ..]); + test_multislice!(&mut arr, s![..;-2, ..], s![..;2, ..]); + test_multislice!(&mut arr, s![..;12, ..], s![3..;3, ..]); +} + +#[test] +fn test_multislice_intersecting() { + assert_panics!({ + let mut arr = Array2::::zeros((8, 6)); + arr.multi_slice_mut((s![3, ..], s![3, ..])); + }); + assert_panics!({ + let mut arr = Array2::::zeros((8, 6)); + arr.multi_slice_mut((s![3, ..], s![3.., ..])); + }); + assert_panics!({ + let mut arr = Array2::::zeros((8, 6)); + arr.multi_slice_mut((s![3, ..], s![..;3, ..])); + }); + assert_panics!({ + let mut arr = Array2::::zeros((8, 6)); + arr.multi_slice_mut((s![..;6, ..], s![3..;3, ..])); + }); + assert_panics!({ + let mut arr = Array2::::zeros((8, 6)); + arr.multi_slice_mut((s![2, ..], s![..-1;-2, ..])); + }); + { + let mut arr = Array2::::zeros((8, 6)); + arr.multi_slice_mut((s![3, ..], s![-1..;-2, ..])); + } +} + #[should_panic] #[test] fn index_out_of_bounds() { From 39302f42f13194e1bf483404fa0f7907c3ffcf87 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Wed, 18 Sep 2019 20:16:20 -0400 Subject: [PATCH 02/10] Limit MultiSlice impls to flat tuples --- src/impl_methods.rs | 2 +- src/impl_views/splitting.rs | 4 +- src/slice.rs | 206 +++++++++++++----------------------- tests/array.rs | 2 +- 4 files changed, 80 insertions(+), 134 deletions(-) diff --git a/src/impl_methods.rs b/src/impl_methods.rs index e68c5c42f..027f5a8af 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -381,7 +381,7 @@ where M: MultiSlice<'a, A, D>, S: DataMut, { - unsafe { info.slice_and_deref(self.raw_view_mut()) } + info.multi_slice_move(self.view_mut()) } /// Slice the array, possibly changing the number of dimensions. diff --git a/src/impl_views/splitting.rs b/src/impl_views/splitting.rs index e72a8d44f..dcfb04b86 100644 --- a/src/impl_views/splitting.rs +++ b/src/impl_views/splitting.rs @@ -129,10 +129,10 @@ where /// * if any of the views would intersect (i.e. if any element would appear in multiple slices) /// * if an index is out of bounds or step size is zero /// * if `D` is `IxDyn` and `info` does not match the number of array axes - pub fn multi_slice_move(mut self, info: M) -> M::Output + pub fn multi_slice_move(self, info: M) -> M::Output where M: MultiSlice<'a, A, D>, { - unsafe { info.slice_and_deref(self.raw_view_mut()) } + info.multi_slice_move(self) } } diff --git a/src/slice.rs b/src/slice.rs index b2731e1cf..0b79f9340 100644 --- a/src/slice.rs +++ b/src/slice.rs @@ -7,7 +7,7 @@ // except according to those terms. use crate::dimension::slices_intersect; use crate::error::{ErrorKind, ShapeError}; -use crate::{ArrayViewMut, Dimension, RawArrayViewMut}; +use crate::{ArrayViewMut, Dimension}; use std::fmt; use std::marker::PhantomData; use std::ops::{Deref, Range, RangeFrom, RangeFull, RangeInclusive, RangeTo, RangeToInclusive}; @@ -633,186 +633,132 @@ macro_rules! s( /// Slicing information describing multiple mutable, disjoint slices. /// -/// It's unfortunate that we need `'out` and `A` to be parameters of the trait, +/// It's unfortunate that we need `'a` and `A` to be parameters of the trait, /// but they're necessary until Rust supports generic associated types. -/// -/// # Safety -/// -/// Implementers of this trait must ensure that: -/// -/// * `.slice_and_deref()` panics or aborts if the slices would intersect, and -/// -/// * the `.intersects_self()`, `.intersects_indices()`, and -/// `.intersects_other()` implementations are correct. -pub unsafe trait MultiSlice<'out, A, D> +pub trait MultiSlice<'a, A, D> where - A: 'out, + A: 'a, D: Dimension, { - /// The type of the slices created by `.slice_and_deref()`. + /// The type of the slices created by `.multi_slice_move()`. type Output; /// Slice the raw view into multiple raw views, and dereference them. /// /// **Panics** if performing any individual slice panics or if the slices /// are not disjoint (i.e. if they intersect). - /// - /// # Safety - /// - /// The caller must ensure that it is safe to mutably dereference the view - /// using the lifetime `'out`. - unsafe fn slice_and_deref(&self, view: RawArrayViewMut) -> Self::Output; - - /// Returns `true` if slicing an array of the specified `shape` with `self` - /// would result in intersecting slices. - /// - /// If `self.intersects_self(&view.raw_dim())` is `true`, then - /// `self.slice_and_deref(view)` must panic. - fn intersects_self(&self, shape: &D) -> bool; - - /// Returns `true` if any slices created by slicing an array of the - /// specified `shape` with `self` would intersect with the specified - /// indices. - /// - /// Note that even if this returns `false`, `self.intersects_self(shape)` - /// may still return `true`. (`.intersects_indices()` doesn't check for - /// intersections within `self`; it only checks for intersections between - /// `self` and `indices`.) - fn intersects_indices(&self, shape: &D, indices: &D::SliceArg) -> bool; - - /// Returns `true` if any slices created by slicing an array of the - /// specified `shape` with `self` would intersect any slices created by - /// slicing the array with `other`. - /// - /// Note that even if this returns `false`, `self.intersects_self(shape)` - /// or `other.intersects_self(shape)` may still return `true`. - /// (`.intersects_other()` doesn't check for intersections within `self` or - /// within `other`; it only checks for intersections between `self` and - /// `other`.) - fn intersects_other(&self, shape: &D, other: impl MultiSlice<'out, A, D>) -> bool; + fn multi_slice_move(&self, view: ArrayViewMut<'a, A, D>) -> Self::Output; } -unsafe impl<'out, A, D, Do> MultiSlice<'out, A, D> for SliceInfo +impl<'a, A, D> MultiSlice<'a, A, D> for () where - A: 'out, + A: 'a, D: Dimension, - Do: Dimension, { - type Output = ArrayViewMut<'out, A, Do>; - - unsafe fn slice_and_deref(&self, view: RawArrayViewMut) -> Self::Output { - view.slice_move(self).deref_into_view_mut() - } - - fn intersects_self(&self, _shape: &D) -> bool { - false - } - - fn intersects_indices(&self, shape: &D, indices: &D::SliceArg) -> bool { - slices_intersect(shape, &*self, indices) - } + type Output = (); - fn intersects_other(&self, shape: &D, other: impl MultiSlice<'out, A, D>) -> bool { - other.intersects_indices(shape, &*self) - } + fn multi_slice_move(&self, _view: ArrayViewMut<'a, A, D>) -> Self::Output {} } -unsafe impl<'out, A, D> MultiSlice<'out, A, D> for () +impl<'a, A, D, Do0> MultiSlice<'a, A, D> for (SliceInfo,) where - A: 'out, + A: 'a, D: Dimension, + D::SliceArg: Sized, + Do0: Dimension, { - type Output = (); + type Output = (ArrayViewMut<'a, A, Do0>,); - unsafe fn slice_and_deref(&self, _view: RawArrayViewMut) -> Self::Output {} - - fn intersects_self(&self, _shape: &D) -> bool { - false + fn multi_slice_move(&self, view: ArrayViewMut<'a, A, D>) -> Self::Output { + (view.slice_move(&self.0),) } +} - fn intersects_indices(&self, _shape: &D, _indices: &D::SliceArg) -> bool { - false - } +impl<'a, A, D, Do0> MultiSlice<'a, A, D> for (&SliceInfo,) +where + A: 'a, + D: Dimension, + Do0: Dimension, +{ + type Output = (ArrayViewMut<'a, A, Do0>,); - fn intersects_other(&self, _shape: &D, _other: impl MultiSlice<'out, A, D>) -> bool { - false + fn multi_slice_move(&self, view: ArrayViewMut<'a, A, D>) -> Self::Output { + (view.slice_move(self.0),) } } macro_rules! impl_multislice_tuple { - ($($T:ident,)*) => { - unsafe impl<'out, A, D, $($T,)*> MultiSlice<'out, A, D> for ($($T,)*) + ($($Do:ident,)*) => { + impl<'a, A, D, $($Do,)*> MultiSlice<'a, A, D> for ($(SliceInfo,)*) where - A: 'out, + A: 'a, D: Dimension, - $($T: MultiSlice<'out, A, D>,)* + D::SliceArg: Sized, + $($Do: Dimension,)* { - type Output = ($($T::Output,)*); - - unsafe fn slice_and_deref(&self, view: RawArrayViewMut) -> Self::Output { - assert!(!self.intersects_self(&view.raw_dim())); + type Output = ($(ArrayViewMut<'a, A, $Do>,)*); + fn multi_slice_move(&self, view: ArrayViewMut<'a, A, D>) -> Self::Output { #[allow(non_snake_case)] - let ($($T,)*) = self; - ($($T.slice_and_deref(view.clone()),)*) - } + let ($($Do,)*) = self; - fn intersects_self(&self, shape: &D) -> bool { - #[allow(non_snake_case)] - let ($($T,)*) = self; - impl_multislice_tuple!(@intersects_self shape, ($($T,)*)) - } + let shape = view.raw_dim(); + assert!(!impl_multislice_tuple!(@intersects_self &shape, ($(&$Do,)*))); - fn intersects_indices(&self, shape: &D, indices: &D::SliceArg) -> bool { - #[allow(non_snake_case)] - let ($($T,)*) = self; - $($T.intersects_indices(shape, indices)) ||* + let raw_view = view.into_raw_view_mut(); + unsafe { + ($(raw_view.clone().slice_move(&$Do).deref_into_view_mut(),)*) + } } + } + + impl<'a, A, D, $($Do,)*> MultiSlice<'a, A, D> for ($(&SliceInfo,)*) + where + A: 'a, + D: Dimension, + $($Do: Dimension,)* + { + type Output = ($(ArrayViewMut<'a, A, $Do>,)*); - fn intersects_other(&self, shape: &D, other: impl MultiSlice<'out, A, D>) -> bool { + fn multi_slice_move(&self, view: ArrayViewMut<'a, A, D>) -> Self::Output { #[allow(non_snake_case)] - let ($($T,)*) = self; - $($T.intersects_other(shape, &other)) ||* + let ($($Do,)*) = self; + + let shape = view.raw_dim(); + assert!(!impl_multislice_tuple!(@intersects_self &shape, ($($Do,)*))); + + let raw_view = view.into_raw_view_mut(); + unsafe { + ($(raw_view.clone().slice_move($Do).deref_into_view_mut(),)*) + } } } }; + (@intersects_self $shape:expr, ($head:expr,)) => { - $head.intersects_self($shape) + false }; (@intersects_self $shape:expr, ($head:expr, $($tail:expr,)*)) => { - $head.intersects_self($shape) || - $($head.intersects_other($shape, &$tail)) ||* || - impl_multislice_tuple!(@intersects_self $shape, ($($tail,)*)) + $(slices_intersect($shape, $head, $tail)) ||* + || impl_multislice_tuple!(@intersects_self $shape, ($($tail,)*)) }; } -impl_multislice_tuple!(T0,); -impl_multislice_tuple!(T0, T1,); -impl_multislice_tuple!(T0, T1, T2,); -impl_multislice_tuple!(T0, T1, T2, T3,); -impl_multislice_tuple!(T0, T1, T2, T3, T4,); -impl_multislice_tuple!(T0, T1, T2, T3, T4, T5,); - -unsafe impl<'out, A, D, T> MultiSlice<'out, A, D> for &'_ T + +impl_multislice_tuple!(Do0, Do1,); +impl_multislice_tuple!(Do0, Do1, Do2,); +impl_multislice_tuple!(Do0, Do1, Do2, Do3,); +impl_multislice_tuple!(Do0, Do1, Do2, Do3, Do4,); +impl_multislice_tuple!(Do0, Do1, Do2, Do3, Do4, Do5,); + +impl<'a, A, D, T> MultiSlice<'a, A, D> for &T where - A: 'out, + A: 'a, D: Dimension, - T: MultiSlice<'out, A, D>, + T: MultiSlice<'a, A, D>, { type Output = T::Output; - unsafe fn slice_and_deref(&self, view: RawArrayViewMut) -> Self::Output { - T::slice_and_deref(self, view) - } - - fn intersects_self(&self, shape: &D) -> bool { - T::intersects_self(self, shape) - } - - fn intersects_indices(&self, shape: &D, indices: &D::SliceArg) -> bool { - T::intersects_indices(self, shape, indices) - } - - fn intersects_other(&self, shape: &D, other: impl MultiSlice<'out, A, D>) -> bool { - T::intersects_other(self, shape, other) + fn multi_slice_move(&self, view: ArrayViewMut<'a, A, D>) -> Self::Output { + T::multi_slice_move(self, view) } } diff --git a/tests/array.rs b/tests/array.rs index 8aaa697c2..3596d32a8 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -354,7 +354,7 @@ fn test_multislice() { }); let mut arr = Array1::from_iter(0..48).into_shape((8, 6)).unwrap(); - assert_eq!(arr.clone().view(), arr.multi_slice_mut(s![.., ..])); + assert_eq!((arr.clone().view_mut(),), arr.multi_slice_mut((s![.., ..],))); test_multislice!(&mut arr, s![0, ..], s![1, ..]); test_multislice!(&mut arr, s![0, ..], s![-1, ..]); test_multislice!(&mut arr, s![0, ..], s![1.., ..]); From 3b60ec614e9ff7bfd0d63046c4cb53a191d95541 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Thu, 19 Sep 2019 17:46:36 -0400 Subject: [PATCH 03/10] Fix formatting --- tests/array.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/array.rs b/tests/array.rs index 3596d32a8..f25914ff9 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -354,7 +354,10 @@ fn test_multislice() { }); let mut arr = Array1::from_iter(0..48).into_shape((8, 6)).unwrap(); - assert_eq!((arr.clone().view_mut(),), arr.multi_slice_mut((s![.., ..],))); + assert_eq!( + (arr.clone().view_mut(),), + arr.multi_slice_mut((s![.., ..],)), + ); test_multislice!(&mut arr, s![0, ..], s![1, ..]); test_multislice!(&mut arr, s![0, ..], s![-1, ..]); test_multislice!(&mut arr, s![0, ..], s![1.., ..]); From 1e94cf67918b360aea90f8c4cd8c4b4ffdaefa9a Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Thu, 19 Sep 2019 18:37:11 -0400 Subject: [PATCH 04/10] Remove unnecessary dead_code annotation --- src/dimension/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/dimension/mod.rs b/src/dimension/mod.rs index 2a0ec4b0d..28d2e9b2c 100644 --- a/src/dimension/mod.rs +++ b/src/dimension/mod.rs @@ -544,7 +544,6 @@ fn slice_min_max(axis_len: usize, slice: Slice) -> Option<(usize, usize)> { } /// Returns `true` iff the slices intersect. -#[allow(dead_code)] pub fn slices_intersect( dim: &D, indices1: &D::SliceArg, From 692a7e7875c574d275440fe7a520257d55a4e96c Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Wed, 25 Sep 2019 16:59:27 -0400 Subject: [PATCH 05/10] Fix typo in docs --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index badb9dcb0..680dc8557 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -475,7 +475,7 @@ pub type Ixs = isize; /// [`.slice_move()`]: #method.slice_move /// [`.slice_collapse()`]: #method.slice_collapse /// -/// It's possible to take multiple simultaneous *mutable* slices with the +/// It's possible to take multiple simultaneous *mutable* slices with /// [`.multi_slice_mut()`] or (for [`ArrayViewMut`] only) /// [`.multi_slice_move()`]. /// From 963e54183b41a61f26ce2fff1a70b8fb4fa9af4b Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Wed, 25 Sep 2019 17:17:38 -0400 Subject: [PATCH 06/10] Forward multi_slice_move impl for owned tuples --- src/slice.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/slice.rs b/src/slice.rs index 0b79f9340..bb540deb5 100644 --- a/src/slice.rs +++ b/src/slice.rs @@ -701,14 +701,7 @@ macro_rules! impl_multislice_tuple { fn multi_slice_move(&self, view: ArrayViewMut<'a, A, D>) -> Self::Output { #[allow(non_snake_case)] let ($($Do,)*) = self; - - let shape = view.raw_dim(); - assert!(!impl_multislice_tuple!(@intersects_self &shape, ($(&$Do,)*))); - - let raw_view = view.into_raw_view_mut(); - unsafe { - ($(raw_view.clone().slice_move(&$Do).deref_into_view_mut(),)*) - } + ($($Do,)*).multi_slice_move(view) } } From bd51ce27b1496283ec1292b70ceac1cf0dabf82b Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Wed, 25 Sep 2019 17:43:40 -0400 Subject: [PATCH 07/10] Avoid final clone in multi_slice_move impl --- src/slice.rs | 45 ++++++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/src/slice.rs b/src/slice.rs index bb540deb5..081c268ea 100644 --- a/src/slice.rs +++ b/src/slice.rs @@ -688,46 +688,53 @@ where } macro_rules! impl_multislice_tuple { - ($($Do:ident,)*) => { - impl<'a, A, D, $($Do,)*> MultiSlice<'a, A, D> for ($(SliceInfo,)*) + ([$($but_last:ident)*] $last:ident) => { + impl_multislice_tuple!(@impl_owned ($($but_last,)* $last,)); + impl_multislice_tuple!(@impl_ref ($($but_last,)* $last,), [$($but_last)*] $last); + }; + (@impl_owned ($($all:ident,)*)) => { + impl<'a, A, D, $($all,)*> MultiSlice<'a, A, D> for ($(SliceInfo,)*) where A: 'a, D: Dimension, D::SliceArg: Sized, - $($Do: Dimension,)* + $($all: Dimension,)* { - type Output = ($(ArrayViewMut<'a, A, $Do>,)*); + type Output = ($(ArrayViewMut<'a, A, $all>,)*); fn multi_slice_move(&self, view: ArrayViewMut<'a, A, D>) -> Self::Output { #[allow(non_snake_case)] - let ($($Do,)*) = self; - ($($Do,)*).multi_slice_move(view) + let ($($all,)*) = self; + ($($all,)*).multi_slice_move(view) } } - - impl<'a, A, D, $($Do,)*> MultiSlice<'a, A, D> for ($(&SliceInfo,)*) + }; + (@impl_ref ($($all:ident,)*), [$($but_last:ident)*] $last:ident) => { + impl<'a, A, D, $($all,)*> MultiSlice<'a, A, D> for ($(&SliceInfo,)*) where A: 'a, D: Dimension, - $($Do: Dimension,)* + $($all: Dimension,)* { - type Output = ($(ArrayViewMut<'a, A, $Do>,)*); + type Output = ($(ArrayViewMut<'a, A, $all>,)*); fn multi_slice_move(&self, view: ArrayViewMut<'a, A, D>) -> Self::Output { #[allow(non_snake_case)] - let ($($Do,)*) = self; + let ($($all,)*) = self; let shape = view.raw_dim(); - assert!(!impl_multislice_tuple!(@intersects_self &shape, ($($Do,)*))); + assert!(!impl_multislice_tuple!(@intersects_self &shape, ($($all,)*))); let raw_view = view.into_raw_view_mut(); unsafe { - ($(raw_view.clone().slice_move($Do).deref_into_view_mut(),)*) + ( + $(raw_view.clone().slice_move($but_last).deref_into_view_mut(),)* + raw_view.slice_move($last).deref_into_view_mut(), + ) } } } }; - (@intersects_self $shape:expr, ($head:expr,)) => { false }; @@ -737,11 +744,11 @@ macro_rules! impl_multislice_tuple { }; } -impl_multislice_tuple!(Do0, Do1,); -impl_multislice_tuple!(Do0, Do1, Do2,); -impl_multislice_tuple!(Do0, Do1, Do2, Do3,); -impl_multislice_tuple!(Do0, Do1, Do2, Do3, Do4,); -impl_multislice_tuple!(Do0, Do1, Do2, Do3, Do4, Do5,); +impl_multislice_tuple!([Do0] Do1); +impl_multislice_tuple!([Do0 Do1] Do2); +impl_multislice_tuple!([Do0 Do1 Do2] Do3); +impl_multislice_tuple!([Do0 Do1 Do2 Do3] Do4); +impl_multislice_tuple!([Do0 Do1 Do2 Do3 Do4] Do5); impl<'a, A, D, T> MultiSlice<'a, A, D> for &T where From 92db6c31c53377d3de3eb97cbf4e3facfd3a3e96 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Thu, 26 Sep 2019 18:39:59 -0400 Subject: [PATCH 08/10] Remove impl of MultiSlice for tuples of SliceInfo --- src/slice.rs | 36 ++---------------------------------- 1 file changed, 2 insertions(+), 34 deletions(-) diff --git a/src/slice.rs b/src/slice.rs index 081c268ea..6cd80d1d4 100644 --- a/src/slice.rs +++ b/src/slice.rs @@ -660,20 +660,6 @@ where fn multi_slice_move(&self, _view: ArrayViewMut<'a, A, D>) -> Self::Output {} } -impl<'a, A, D, Do0> MultiSlice<'a, A, D> for (SliceInfo,) -where - A: 'a, - D: Dimension, - D::SliceArg: Sized, - Do0: Dimension, -{ - type Output = (ArrayViewMut<'a, A, Do0>,); - - fn multi_slice_move(&self, view: ArrayViewMut<'a, A, D>) -> Self::Output { - (view.slice_move(&self.0),) - } -} - impl<'a, A, D, Do0> MultiSlice<'a, A, D> for (&SliceInfo,) where A: 'a, @@ -689,27 +675,9 @@ where macro_rules! impl_multislice_tuple { ([$($but_last:ident)*] $last:ident) => { - impl_multislice_tuple!(@impl_owned ($($but_last,)* $last,)); - impl_multislice_tuple!(@impl_ref ($($but_last,)* $last,), [$($but_last)*] $last); - }; - (@impl_owned ($($all:ident,)*)) => { - impl<'a, A, D, $($all,)*> MultiSlice<'a, A, D> for ($(SliceInfo,)*) - where - A: 'a, - D: Dimension, - D::SliceArg: Sized, - $($all: Dimension,)* - { - type Output = ($(ArrayViewMut<'a, A, $all>,)*); - - fn multi_slice_move(&self, view: ArrayViewMut<'a, A, D>) -> Self::Output { - #[allow(non_snake_case)] - let ($($all,)*) = self; - ($($all,)*).multi_slice_move(view) - } - } + impl_multislice_tuple!(@def_impl ($($but_last,)* $last,), [$($but_last)*] $last); }; - (@impl_ref ($($all:ident,)*), [$($but_last:ident)*] $last:ident) => { + (@def_impl ($($all:ident,)*), [$($but_last:ident)*] $last:ident) => { impl<'a, A, D, $($all,)*> MultiSlice<'a, A, D> for ($(&SliceInfo,)*) where A: 'a, From 7152ada50c496718ba4834a84c1aeb1429783374 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 14 Oct 2019 17:55:40 -0400 Subject: [PATCH 09/10] Fix docs for MultiSlice::multi_slice_move --- src/slice.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/slice.rs b/src/slice.rs index 6cd80d1d4..58bff48b5 100644 --- a/src/slice.rs +++ b/src/slice.rs @@ -643,7 +643,7 @@ where /// The type of the slices created by `.multi_slice_move()`. type Output; - /// Slice the raw view into multiple raw views, and dereference them. + /// Split the view into multiple disjoint slices. /// /// **Panics** if performing any individual slice panics or if the slices /// are not disjoint (i.e. if they intersect). From ef93420864c4247847f0d12d3e7c37e025dad895 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Mon, 14 Oct 2019 18:10:30 -0400 Subject: [PATCH 10/10] Add more multi_slice_mut tests --- tests/array.rs | 60 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/tests/array.rs b/tests/array.rs index f25914ff9..db54b7e5f 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -343,29 +343,39 @@ fn test_slice_collapse_with_indices() { } #[test] -#[allow(clippy::cognitive_complexity)] fn test_multislice() { - defmac!(test_multislice arr, s1, s2 => { - let copy = arr.clone(); - assert_eq!( - arr.multi_slice_mut((s1, s2)), - (copy.clone().slice_mut(s1), copy.clone().slice_mut(s2)) - ); - }); + macro_rules! do_test { + ($arr:expr, $($s:expr),*) => { + { + let arr = $arr; + let copy = arr.clone(); + assert_eq!( + arr.multi_slice_mut(($($s,)*)), + ($(copy.clone().slice_mut($s),)*) + ); + } + }; + } + let mut arr = Array1::from_iter(0..48).into_shape((8, 6)).unwrap(); assert_eq!( (arr.clone().view_mut(),), arr.multi_slice_mut((s![.., ..],)), ); - test_multislice!(&mut arr, s![0, ..], s![1, ..]); - test_multislice!(&mut arr, s![0, ..], s![-1, ..]); - test_multislice!(&mut arr, s![0, ..], s![1.., ..]); - test_multislice!(&mut arr, s![1, ..], s![..;2, ..]); - test_multislice!(&mut arr, s![..2, ..], s![2.., ..]); - test_multislice!(&mut arr, s![1..;2, ..], s![..;2, ..]); - test_multislice!(&mut arr, s![..;-2, ..], s![..;2, ..]); - test_multislice!(&mut arr, s![..;12, ..], s![3..;3, ..]); + assert_eq!(arr.multi_slice_mut(()), ()); + do_test!(&mut arr, s![0, ..]); + do_test!(&mut arr, s![0, ..], s![1, ..]); + do_test!(&mut arr, s![0, ..], s![-1, ..]); + do_test!(&mut arr, s![0, ..], s![1.., ..]); + do_test!(&mut arr, s![1, ..], s![..;2, ..]); + do_test!(&mut arr, s![..2, ..], s![2.., ..]); + do_test!(&mut arr, s![1..;2, ..], s![..;2, ..]); + do_test!(&mut arr, s![..;-2, ..], s![..;2, ..]); + do_test!(&mut arr, s![..;12, ..], s![3..;3, ..]); + do_test!(&mut arr, s![3, ..], s![..-1;-2, ..]); + do_test!(&mut arr, s![0, ..], s![1, ..], s![2, ..]); + do_test!(&mut arr, s![0, ..], s![1, ..], s![2, ..], s![3, ..]); } #[test] @@ -390,10 +400,22 @@ fn test_multislice_intersecting() { let mut arr = Array2::::zeros((8, 6)); arr.multi_slice_mut((s![2, ..], s![..-1;-2, ..])); }); - { + assert_panics!({ let mut arr = Array2::::zeros((8, 6)); - arr.multi_slice_mut((s![3, ..], s![-1..;-2, ..])); - } + arr.multi_slice_mut((s![4, ..], s![3, ..], s![3, ..])); + }); + assert_panics!({ + let mut arr = Array2::::zeros((8, 6)); + arr.multi_slice_mut((s![3, ..], s![4, ..], s![3, ..])); + }); + assert_panics!({ + let mut arr = Array2::::zeros((8, 6)); + arr.multi_slice_mut((s![3, ..], s![3, ..], s![4, ..])); + }); + assert_panics!({ + let mut arr = Array2::::zeros((8, 6)); + arr.multi_slice_mut((s![3, ..], s![3, ..], s![4, ..], s![3, ..])); + }); } #[should_panic]