From 9c072bee749ba3dfff25f2e697d98f5b001031c1 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 14 Mar 2021 19:49:58 -0400 Subject: [PATCH 1/4] Implement SliceArg for &T --- src/slice.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/slice.rs b/src/slice.rs index 43785555d..d4f70d5e1 100644 --- a/src/slice.rs +++ b/src/slice.rs @@ -324,6 +324,24 @@ pub unsafe trait SliceArg: AsRef<[AxisSliceInfo]> { private_decl! {} } +unsafe impl SliceArg for &T +where + T: SliceArg + ?Sized, + D: Dimension, +{ + type OutDim = T::OutDim; + + fn in_ndim(&self) -> usize { + T::in_ndim(self) + } + + fn out_ndim(&self) -> usize { + T::out_ndim(self) + } + + private_impl! {} +} + macro_rules! impl_slicearg_samedim { ($in_dim:ty) => { unsafe impl SliceArg<$in_dim> for SliceInfo From 24c6786ceebdaf953001bf3513331c26e7a51e48 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 14 Mar 2021 19:52:03 -0400 Subject: [PATCH 2/4] Allow passing owned slice arg to slicing methods This allows the `s![]` macro to return an owned type, without requiring an explicit `&` when calling slicing methods. --- src/dimension/mod.rs | 4 ++-- src/impl_methods.rs | 16 ++++++++-------- src/lib.rs | 7 +++---- src/slice.rs | 18 +++++++----------- 4 files changed, 20 insertions(+), 25 deletions(-) diff --git a/src/dimension/mod.rs b/src/dimension/mod.rs index 243d5eeea..f94a7135c 100644 --- a/src/dimension/mod.rs +++ b/src/dimension/mod.rs @@ -599,8 +599,8 @@ fn slice_min_max(axis_len: usize, slice: Slice) -> Option<(usize, usize)> { /// Returns `true` iff the slices intersect. pub fn slices_intersect( dim: &D, - indices1: &impl SliceArg, - indices2: &impl SliceArg, + indices1: impl SliceArg, + indices2: impl SliceArg, ) -> bool { debug_assert_eq!(indices1.in_ndim(), indices2.in_ndim()); for (&axis_len, &si1, &si2) in izip!( diff --git a/src/impl_methods.rs b/src/impl_methods.rs index d039d68a6..a0833048e 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -338,9 +338,9 @@ where /// /// **Panics** if an index is out of bounds or step size is zero.
/// (**Panics** if `D` is `IxDyn` and `info` does not match the number of array axes.) - pub fn slice(&self, info: &I) -> ArrayView<'_, A, I::OutDim> + pub fn slice(&self, info: I) -> ArrayView<'_, A, I::OutDim> where - I: SliceArg + ?Sized, + I: SliceArg, S: Data, { self.view().slice_move(info) @@ -353,9 +353,9 @@ where /// /// **Panics** if an index is out of bounds or step size is zero.
/// (**Panics** if `D` is `IxDyn` and `info` does not match the number of array axes.) - pub fn slice_mut(&mut self, info: &I) -> ArrayViewMut<'_, A, I::OutDim> + pub fn slice_mut(&mut self, info: I) -> ArrayViewMut<'_, A, I::OutDim> where - I: SliceArg + ?Sized, + I: SliceArg, S: DataMut, { self.view_mut().slice_move(info) @@ -399,9 +399,9 @@ where /// /// **Panics** if an index is out of bounds or step size is zero.
/// (**Panics** if `D` is `IxDyn` and `info` does not match the number of array axes.) - pub fn slice_move(mut self, info: &I) -> ArrayBase + pub fn slice_move(mut self, info: I) -> ArrayBase where - I: SliceArg + ?Sized, + I: SliceArg, { assert_eq!( info.in_ndim(), @@ -468,9 +468,9 @@ where /// - if [`AxisSliceInfo::NewAxis`] is in `info`, e.g. if [`NewAxis`] was /// used in the [`s!`] macro /// - if `D` is `IxDyn` and `info` does not match the number of array axes - pub fn slice_collapse(&mut self, info: &I) + pub fn slice_collapse(&mut self, info: I) where - I: SliceArg + ?Sized, + I: SliceArg, { assert_eq!( info.in_ndim(), diff --git a/src/lib.rs b/src/lib.rs index f48da4b32..172289efe 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -492,9 +492,8 @@ pub type Ixs = isize; /// /// The slicing argument can be passed using the macro [`s![]`](macro.s!.html), /// which will be used in all examples. (The explicit form is an instance of -/// [`&SliceInfo`]; see its docs for more information.) -/// -/// [`&SliceInfo`]: struct.SliceInfo.html +/// [`SliceInfo`] or another type which implements [`SliceArg`]; see their docs +/// for more information.) /// /// If a range is used, the axis is preserved. If an index is used, that index /// is selected and the axis is removed; this selects a subview. See @@ -510,7 +509,7 @@ pub type Ixs = isize; /// [`NewAxis`]: struct.NewAxis.html /// /// When slicing arrays with generic dimensionality, creating an instance of -/// [`&SliceInfo`] to pass to the multi-axis slicing methods like [`.slice()`] +/// [`SliceInfo`] to pass to the multi-axis slicing methods like [`.slice()`] /// is awkward. In these cases, it's usually more convenient to use /// [`.slice_each_axis()`]/[`.slice_each_axis_mut()`]/[`.slice_each_axis_inplace()`] /// or to create a view and then slice individual axes of the view using diff --git a/src/slice.rs b/src/slice.rs index d4f70d5e1..dbb3873e1 100644 --- a/src/slice.rs +++ b/src/slice.rs @@ -78,7 +78,7 @@ pub struct NewAxis; /// A slice (range with step), an index, or a new axis token. /// /// See also the [`s![]`](macro.s!.html) macro for a convenient way to create a -/// `&SliceInfo<[AxisSliceInfo; n], Din, Dout>`. +/// `SliceInfo<[AxisSliceInfo; n], Din, Dout>`. /// /// ## Examples /// @@ -721,9 +721,7 @@ impl_slicenextdim!((), NewAxis, Ix0, Ix1); /// /// `s![]` takes a list of ranges/slices/indices/new-axes, separated by comma, /// with optional step sizes that are separated from the range by a semicolon. -/// It is converted into a [`&SliceInfo`] instance. -/// -/// [`&SliceInfo`]: struct.SliceInfo.html +/// It is converted into a [`SliceInfo`] instance. /// /// Each range/slice/index uses signed indices, where a negative value is /// counted from the end of the axis. Step sizes are also signed and may be @@ -907,9 +905,7 @@ macro_rules! s( <$crate::AxisSliceInfo as ::std::convert::From<_>>::from($r).step_by($s as isize) }; ($($t:tt)*) => { - // The extra `*&` is a workaround for this compiler bug: - // https://github.com/rust-lang/rust/issues/23014 - &*&$crate::s![@parse + $crate::s![@parse ::std::marker::PhantomData::<$crate::Ix0>, ::std::marker::PhantomData::<$crate::Ix0>, [] @@ -951,7 +947,7 @@ where private_impl! {} } -impl<'a, A, D, I0> MultiSliceArg<'a, A, D> for (&I0,) +impl<'a, A, D, I0> MultiSliceArg<'a, A, D> for (I0,) where A: 'a, D: Dimension, @@ -960,7 +956,7 @@ where type Output = (ArrayViewMut<'a, A, I0::OutDim>,); fn multi_slice_move(&self, view: ArrayViewMut<'a, A, D>) -> Self::Output { - (view.slice_move(self.0),) + (view.slice_move(&self.0),) } private_impl! {} @@ -971,7 +967,7 @@ macro_rules! impl_multislice_tuple { impl_multislice_tuple!(@def_impl ($($but_last,)* $last,), [$($but_last)*] $last); }; (@def_impl ($($all:ident,)*), [$($but_last:ident)*] $last:ident) => { - impl<'a, A, D, $($all,)*> MultiSliceArg<'a, A, D> for ($(&$all,)*) + impl<'a, A, D, $($all,)*> MultiSliceArg<'a, A, D> for ($($all,)*) where A: 'a, D: Dimension, @@ -981,7 +977,7 @@ macro_rules! impl_multislice_tuple { fn multi_slice_move(&self, view: ArrayViewMut<'a, A, D>) -> Self::Output { #[allow(non_snake_case)] - let &($($all,)*) = self; + let ($($all,)*) = self; let shape = view.raw_dim(); assert!(!impl_multislice_tuple!(@intersects_self &shape, ($($all,)*))); From 4326b2523e7b222c0ac224d0d09eecb2a240ce00 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 14 Mar 2021 21:34:51 -0400 Subject: [PATCH 3/4] Make SliceInfo be Sized, and remove pointer casts The original reason for `SliceInfo` being `?Sized` and these conversions was to work around the limitations of the slicing methods taking a type involving `Dimension::SliceArg`. Now that the slicing methods take `I: SliceArg` as the parameter type, these conversions are no longer necessary, and `SliceInfo` doesn't need to be `?Sized`. --- src/slice.rs | 52 +++++++++++++++------------------------------------- 1 file changed, 15 insertions(+), 37 deletions(-) diff --git a/src/slice.rs b/src/slice.rs index dbb3873e1..03544bb5e 100644 --- a/src/slice.rs +++ b/src/slice.rs @@ -406,7 +406,7 @@ unsafe impl SliceArg for [AxisSliceInfo] { /// Represents all of the necessary information to perform a slice. /// -/// The type `T` is typically `[AxisSliceInfo; n]`, `[AxisSliceInfo]`, or +/// The type `T` is typically `[AxisSliceInfo; n]`, `&[AxisSliceInfo]`, or /// `Vec`. The type `Din` is the dimension of the array to be /// sliced, and `Dout` is the output dimension after calling [`.slice()`]. Note /// that if `Din` is a fixed dimension type (`Ix0`, `Ix1`, `Ix2`, etc.), the @@ -415,14 +415,13 @@ unsafe impl SliceArg for [AxisSliceInfo] { /// /// [`.slice()`]: struct.ArrayBase.html#method.slice #[derive(Debug)] -#[repr(transparent)] -pub struct SliceInfo { +pub struct SliceInfo { in_dim: PhantomData, out_dim: PhantomData, indices: T, } -impl Deref for SliceInfo +impl Deref for SliceInfo where Din: Dimension, Dout: Dimension, @@ -482,14 +481,7 @@ where indices, } } -} -impl SliceInfo -where - T: AsRef<[AxisSliceInfo]>, - Din: Dimension, - Dout: Dimension, -{ /// Returns a new `SliceInfo` instance. /// /// Errors if `Din` or `Dout` is not consistent with `indices`. @@ -508,14 +500,7 @@ where indices, }) } -} -impl SliceInfo -where - T: AsRef<[AxisSliceInfo]>, - Din: Dimension, - Dout: Dimension, -{ /// Returns the number of dimensions of the input array for /// [`.slice()`](struct.ArrayBase.html#method.slice). /// @@ -546,7 +531,7 @@ where } } -impl<'a, Din, Dout> TryFrom<&'a [AxisSliceInfo]> for &'a SliceInfo<[AxisSliceInfo], Din, Dout> +impl<'a, Din, Dout> TryFrom<&'a [AxisSliceInfo]> for SliceInfo<&'a [AxisSliceInfo], Din, Dout> where Din: Dimension, Dout: Dimension, @@ -555,16 +540,11 @@ where fn try_from( indices: &'a [AxisSliceInfo], - ) -> Result<&'a SliceInfo<[AxisSliceInfo], Din, Dout>, ShapeError> { - check_dims_for_sliceinfo::(indices)?; + ) -> Result, ShapeError> { unsafe { - // This is okay because we've already checked the correctness of - // `Din` and `Dout`, and the only non-zero-sized member of - // `SliceInfo` is `indices`, so `&SliceInfo<[AxisSliceInfo], Din, - // Dout>` should have the same bitwise representation as - // `&[AxisSliceInfo]`. - Ok(&*(indices as *const [AxisSliceInfo] - as *const SliceInfo<[AxisSliceInfo], Din, Dout>)) + // This is okay because `&[AxisSliceInfo]` always returns the same + // value for `.as_ref()`. + Self::new(indices) } } } @@ -630,20 +610,18 @@ where } } -impl AsRef> for SliceInfo +impl<'a, T, Din, Dout> From<&'a SliceInfo> + for SliceInfo<&'a [AxisSliceInfo], Din, Dout> where T: AsRef<[AxisSliceInfo]>, Din: Dimension, Dout: Dimension, { - fn as_ref(&self) -> &SliceInfo<[AxisSliceInfo], Din, Dout> { - unsafe { - // This is okay because the only non-zero-sized member of - // `SliceInfo` is `indices`, so `&SliceInfo<[AxisSliceInfo], Din, Dout>` - // should have the same bitwise representation as - // `&[AxisSliceInfo]`. - &*(self.indices.as_ref() as *const [AxisSliceInfo] - as *const SliceInfo<[AxisSliceInfo], Din, Dout>) + fn from(info: &'a SliceInfo) -> SliceInfo<&'a [AxisSliceInfo], Din, Dout> { + SliceInfo { + in_dim: info.in_dim, + out_dim: info.out_dim, + indices: info.indices.as_ref(), } } } From fb7e94dd6954bc92a02938fdeb6250820b9afde6 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 14 Mar 2021 22:10:44 -0400 Subject: [PATCH 4/4] Remove unnecessary `&` in tests --- tests/array.rs | 12 ++++++------ tests/oper.rs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/array.rs b/tests/array.rs index fa5da4419..1cbd34e34 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -219,7 +219,7 @@ fn test_slice_dyninput_array_fixed() { #[test] fn test_slice_array_dyn() { let mut arr = Array3::::zeros((5, 2, 5)); - let info = &SliceInfo::<_, Ix3, IxDyn>::try_from([ + let info = SliceInfo::<_, Ix3, IxDyn>::try_from([ AxisSliceInfo::from(1..), AxisSliceInfo::from(1), AxisSliceInfo::from(NewAxis), @@ -229,7 +229,7 @@ fn test_slice_array_dyn() { arr.slice(info); arr.slice_mut(info); arr.view().slice_move(info); - let info2 = &SliceInfo::<_, Ix3, IxDyn>::try_from([ + let info2 = SliceInfo::<_, Ix3, IxDyn>::try_from([ AxisSliceInfo::from(1..), AxisSliceInfo::from(1), AxisSliceInfo::from(..).step_by(2), @@ -241,7 +241,7 @@ fn test_slice_array_dyn() { #[test] fn test_slice_dyninput_array_dyn() { let mut arr = Array3::::zeros((5, 2, 5)).into_dyn(); - let info = &SliceInfo::<_, Ix3, IxDyn>::try_from([ + let info = SliceInfo::<_, Ix3, IxDyn>::try_from([ AxisSliceInfo::from(1..), AxisSliceInfo::from(1), AxisSliceInfo::from(NewAxis), @@ -251,7 +251,7 @@ fn test_slice_dyninput_array_dyn() { arr.slice(info); arr.slice_mut(info); arr.view().slice_move(info); - let info2 = &SliceInfo::<_, Ix3, IxDyn>::try_from([ + let info2 = SliceInfo::<_, Ix3, IxDyn>::try_from([ AxisSliceInfo::from(1..), AxisSliceInfo::from(1), AxisSliceInfo::from(..).step_by(2), @@ -273,7 +273,7 @@ fn test_slice_dyninput_vec_fixed() { arr.slice(info); arr.slice_mut(info); arr.view().slice_move(info); - let info2 = &SliceInfo::<_, Ix3, Ix2>::try_from(vec![ + let info2 = SliceInfo::<_, Ix3, Ix2>::try_from(vec![ AxisSliceInfo::from(1..), AxisSliceInfo::from(1), AxisSliceInfo::from(..).step_by(2), @@ -295,7 +295,7 @@ fn test_slice_dyninput_vec_dyn() { arr.slice(info); arr.slice_mut(info); arr.view().slice_move(info); - let info2 = &SliceInfo::<_, Ix3, IxDyn>::try_from(vec![ + let info2 = SliceInfo::<_, Ix3, IxDyn>::try_from(vec![ AxisSliceInfo::from(1..), AxisSliceInfo::from(1), AxisSliceInfo::from(..).step_by(2), diff --git a/tests/oper.rs b/tests/oper.rs index b91a9bd85..3533faa8e 100644 --- a/tests/oper.rs +++ b/tests/oper.rs @@ -596,7 +596,7 @@ fn scaled_add_3() { { let mut av = a.slice_mut(s![..;s1, ..;s2]); - let c = c.slice(&SliceInfo::<_, IxDyn, IxDyn>::try_from(cslice).unwrap()); + let c = c.slice(SliceInfo::<_, IxDyn, IxDyn>::try_from(cslice).unwrap()); let mut answerv = answer.slice_mut(s![..;s1, ..;s2]); answerv += &(beta * &c);