Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid re-borrowing in multislice! macro #692

Closed
wants to merge 1 commit into from
Closed

Avoid re-borrowing in multislice! macro #692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ pub type Ixs = isize;
/// // - 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) = multislice!(h, mut [.., ..;2], mut [.., 1..;2]);
/// let (s0, s1) = multislice!(&mut h, mut [.., ..;2], mut [.., 1..;2]);
/// let i = arr2(&[[0, 2],
/// [4, 6]]);
/// let j = arr2(&[[1, 3],
Expand Down
18 changes: 9 additions & 9 deletions src/slice.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,8 +657,8 @@ pub unsafe fn deref_raw_view_mut_into_view_mut_with_life<'a, A, D: Dimension>(
/// disjoint).
///
/// The syntax is `multislice!(` *expression, pattern [, pattern [, …]]* `)`,
/// where *expression* evaluates to a mutable array, and each *pattern* is
/// either
/// where *expression* is any valid input to `ArrayViewMut::from()`, and each
/// *pattern* is either
///
/// * `mut` *s-args-or-expr*: creates an `ArrayViewMut` or
/// * *s-args-or-expr*: creates an `ArrayView`
Expand All @@ -667,9 +667,9 @@ pub unsafe fn deref_raw_view_mut_into_view_mut_with_life<'a, A, D: Dimension>(
/// the [`s!`] macro to create a `&SliceInfo` instance or (2) an expression
/// that evaluates to a `&SliceInfo` instance.
///
/// **Note** that this macro always mutably borrows the array even if there are
/// no `mut` patterns. If all you want to do is take read-only slices, you
/// don't need `multislice!()`; just call
/// **Note** that *expression* is converted to an `ArrayViewMut` using
/// `ArrayViewMut::from()` even if there are no `mut` patterns. If all you want
/// to do is take read-only slices, you don't need `multislice!()`; just call
/// [`.slice()`](struct.ArrayBase.html#method.slice) multiple times instead.
///
/// `multislice!()` evaluates to a tuple of `ArrayView` and/or `ArrayViewMut`
Expand Down Expand Up @@ -697,7 +697,7 @@ pub unsafe fn deref_raw_view_mut_into_view_mut_with_life<'a, A, D: Dimension>(
///
/// # fn main() {
/// let mut arr: Array1<_> = (0..12).collect();
/// let (a, b, c, d) = multislice!(arr, [0..5], mut [6..;2], [1..6], mut [7..;2]);
/// let (a, b, c, d) = multislice!(&mut arr, [0..5], mut [6..;2], [1..6], mut [7..;2]);
/// assert_eq!(a, array![0, 1, 2, 3, 4]);
/// assert_eq!(b, array![6, 8, 10]);
/// assert_eq!(c, array![1, 2, 3, 4, 5]);
Expand All @@ -715,7 +715,7 @@ pub unsafe fn deref_raw_view_mut_into_view_mut_with_life<'a, A, D: Dimension>(
/// # use ndarray::prelude::*;
/// # fn main() {
/// let mut arr: Array1<_> = (0..12).collect();
/// multislice!(arr, [0..5], mut [1..;2]); // panic!
/// multislice!(&mut arr, [0..5], mut [1..;2]); // panic!
/// # }
/// ```
///
Expand All @@ -727,7 +727,7 @@ pub unsafe fn deref_raw_view_mut_into_view_mut_with_life<'a, A, D: Dimension>(
/// # use ndarray::prelude::*;
/// # fn main() {
/// let mut arr: Array1<_> = (0..12).collect();
/// multislice!(arr, mut [0..5], mut [1..;2]); // panic!
/// multislice!(&mut arr, mut [0..5], mut [1..;2]); // panic!
/// # }
/// ```
#[macro_export]
Expand Down Expand Up @@ -985,7 +985,7 @@ macro_rules! multislice(
($arr:expr, $($t:tt)*) => {
{
let (life, raw_view) = {
let mut view = $crate::ArrayBase::view_mut(&mut $arr);
let mut view: $crate::ArrayViewMut<_, _> = ::core::convert::From::from($arr);
($crate::life_of_view_mut(&view), view.raw_view_mut())
};
$crate::multislice!(@parse raw_view, life, (), (), (), ($($t)*))
Expand Down
98 changes: 59 additions & 39 deletions tests/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,76 +336,74 @@ fn test_slice_collapse_with_indices() {
}

#[test]
#[allow(clippy::cognitive_complexity)]
fn test_multislice() {
defmac!(test_multislice mut arr, s1, s2 => {
{
let copy = arr.clone();
macro_rules! check_multislice {
($arr:expr, $s1:expr, $s2:expr) => {{
let copy = $arr.clone();
assert_eq!(
multislice!(arr, mut s1, mut s2,),
(copy.clone().slice_mut(s1), copy.clone().slice_mut(s2))
multislice!(&mut $arr, mut $s1, mut $s2,),
(copy.clone().slice_mut($s1), copy.clone().slice_mut($s2))
);
}
{
let copy = arr.clone();
let copy = $arr.clone();
assert_eq!(
multislice!(arr, mut s1, s2,),
(copy.clone().slice_mut(s1), copy.clone().slice(s2))
multislice!(&mut $arr, mut $s1, $s2,),
(copy.clone().slice_mut($s1), copy.clone().slice($s2))
);
}
{
let copy = arr.clone();
let copy = $arr.clone();
assert_eq!(
multislice!(arr, s1, mut s2),
(copy.clone().slice(s1), copy.clone().slice_mut(s2))
multislice!(&mut $arr, $s1, mut $s2),
(copy.clone().slice($s1), copy.clone().slice_mut($s2))
);
}
{
let copy = arr.clone();
let copy = $arr.clone();
assert_eq!(
multislice!(arr, s1, s2),
(copy.clone().slice(s1), copy.clone().slice(s2))
multislice!(&mut $arr, $s1, $s2),
(copy.clone().slice($s1), copy.clone().slice($s2))
);
}
});
}};
};
let mut arr = Array1::from_iter(0..48).into_shape((8, 6)).unwrap();

assert_eq!((arr.clone().view(),), multislice!(arr, [.., ..]));
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.clone().view(),), multislice!(&mut arr, [.., ..]));
check_multislice!(arr, s![0, ..], s![1, ..]);
check_multislice!(arr, s![0, ..], s![-1, ..]);
check_multislice!(arr, s![0, ..], s![1.., ..]);
check_multislice!(arr, s![1, ..], s![..;2, ..]);
check_multislice!(arr, s![..2, ..], s![2.., ..]);
check_multislice!(arr, s![1..;2, ..], s![..;2, ..]);
check_multislice!(arr, s![..;-2, ..], s![..;2, ..]);
check_multislice!(arr, s![..;12, ..], s![3..;3, ..]);
}

#[test]
fn test_multislice_intersecting() {
assert_panics!({
let mut arr = Array2::<u8>::zeros((8, 6));
multislice!(arr, mut [3, ..], [3, ..]);
multislice!(&mut arr, mut [3, ..], [3, ..]);
});
assert_panics!({
let mut arr = Array2::<u8>::zeros((8, 6));
multislice!(arr, mut [3, ..], [3.., ..]);
multislice!(&mut arr, mut [3, ..], [3.., ..]);
});
assert_panics!({
let mut arr = Array2::<u8>::zeros((8, 6));
multislice!(arr, mut [3, ..], [..;3, ..]);
multislice!(&mut arr, mut [3, ..], [..;3, ..]);
});
assert_panics!({
let mut arr = Array2::<u8>::zeros((8, 6));
multislice!(arr, mut [..;6, ..], [3..;3, ..]);
multislice!(&mut arr, mut [..;6, ..], [3..;3, ..]);
});
assert_panics!({
let mut arr = Array2::<u8>::zeros((8, 6));
multislice!(arr, mut [2, ..], mut [..-1;-2, ..]);
multislice!(&mut arr, mut [2, ..], mut [..-1;-2, ..]);
});
{
let mut arr = Array2::<u8>::zeros((8, 6));
multislice!(arr, [3, ..], [-1..;-2, ..]);
multislice!(&mut arr, [3, ..], [-1..;-2, ..]);
}
}

Expand All @@ -418,7 +416,7 @@ fn test_multislice_eval_args_only_once() {
eval_count += 1;
*s![1..2]
};
multislice!(arr, mut &slice(), [3..4], [5..6]);
multislice!(&mut arr, mut &slice(), [3..4], [5..6]);
}
assert_eq!(eval_count, 1);
let mut eval_count = 0;
Expand All @@ -427,7 +425,7 @@ fn test_multislice_eval_args_only_once() {
eval_count += 1;
*s![1..2]
};
multislice!(arr, [3..4], mut &slice(), [5..6]);
multislice!(&mut arr, [3..4], mut &slice(), [5..6]);
}
assert_eq!(eval_count, 1);
let mut eval_count = 0;
Expand All @@ -436,7 +434,7 @@ fn test_multislice_eval_args_only_once() {
eval_count += 1;
*s![1..2]
};
multislice!(arr, [3..4], [5..6], mut &slice());
multislice!(&mut arr, [3..4], [5..6], mut &slice());
}
assert_eq!(eval_count, 1);
let mut eval_count = 0;
Expand All @@ -445,7 +443,7 @@ fn test_multislice_eval_args_only_once() {
eval_count += 1;
*s![1..2]
};
multislice!(arr, &slice(), mut [3..4], [5..6]);
multislice!(&mut arr, &slice(), mut [3..4], [5..6]);
}
assert_eq!(eval_count, 1);
let mut eval_count = 0;
Expand All @@ -454,7 +452,7 @@ fn test_multislice_eval_args_only_once() {
eval_count += 1;
*s![1..2]
};
multislice!(arr, mut [3..4], &slice(), [5..6]);
multislice!(&mut arr, mut [3..4], &slice(), [5..6]);
}
assert_eq!(eval_count, 1);
let mut eval_count = 0;
Expand All @@ -463,11 +461,33 @@ fn test_multislice_eval_args_only_once() {
eval_count += 1;
*s![1..2]
};
multislice!(arr, mut [3..4], [5..6], &slice());
multislice!(&mut arr, mut [3..4], [5..6], &slice());
}
assert_eq!(eval_count, 1);
}

#[test]
fn test_multislice_arrayviewmut_same_life() {
// This test makes sure that it's possible for the borrowed elements
// returned from `get_mut2` to have the same life as the `arr` view.
fn get_mut2<'a, A>(
arr: ArrayViewMut<'a, A, Ix2>,
[i1, j1]: [usize; 2],
[i2, j2]: [usize; 2],
) -> (&'a mut A, &'a mut A) {
use ndarray::IndexLonger;
let (x1, x2) = multislice!(arr, mut [i1, j1], mut [i2, j2]);
(x1.index([]), x2.index([]))
}
let mut arr = array![[1, 2], [3, 4]];
{
let (x1, x2) = get_mut2(arr.view_mut(), [0, 0], [1, 0]);
*x1 += 1;
*x2 += 2;
}
assert_eq!(arr, array![[2, 2], [5, 4]]);
}

#[should_panic]
#[test]
fn index_out_of_bounds() {
Expand Down