Skip to content

Commit dd5ef0a

Browse files
committed
Avoid re-borrowing in multislice! macro
The old implementation always called `.view_mut()` on the input array, which meant that it was not possible to use `multislice!` to split an `ArrayViewMut` instance into pieces with the same lifetime as the original view. Now, `multislice!` uses `ArrayViewMut::from()` instead so that this works. The primary disadvantage is that an explicit borrow is necessary in many cases, which is less concise. Closes #687.
1 parent ce80d38 commit dd5ef0a

File tree

3 files changed

+85
-63
lines changed

3 files changed

+85
-63
lines changed

src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ pub type Ixs = isize;
524524
/// // - One containing all the odd-index columns in the matrix
525525
/// let mut h = arr2(&[[0, 1, 2, 3],
526526
/// [4, 5, 6, 7]]);
527-
/// let (s0, s1) = multislice!(h, mut [.., ..;2], mut [.., 1..;2]);
527+
/// let (s0, s1) = multislice!(&mut h, mut [.., ..;2], mut [.., 1..;2]);
528528
/// let i = arr2(&[[0, 2],
529529
/// [4, 6]]);
530530
/// let j = arr2(&[[1, 3],

src/slice.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -657,8 +657,8 @@ pub unsafe fn deref_raw_view_mut_into_view_mut_with_life<'a, A, D: Dimension>(
657657
/// disjoint).
658658
///
659659
/// The syntax is `multislice!(` *expression, pattern [, pattern [, …]]* `)`,
660-
/// where *expression* evaluates to a mutable array, and each *pattern* is
661-
/// either
660+
/// where *expression* is any valid input to `ArrayViewMut::from()`, and each
661+
/// *pattern* is either
662662
///
663663
/// * `mut` *s-args-or-expr*: creates an `ArrayViewMut` or
664664
/// * *s-args-or-expr*: creates an `ArrayView`
@@ -667,9 +667,9 @@ pub unsafe fn deref_raw_view_mut_into_view_mut_with_life<'a, A, D: Dimension>(
667667
/// the [`s!`] macro to create a `&SliceInfo` instance or (2) an expression
668668
/// that evaluates to a `&SliceInfo` instance.
669669
///
670-
/// **Note** that this macro always mutably borrows the array even if there are
671-
/// no `mut` patterns. If all you want to do is take read-only slices, you
672-
/// don't need `multislice!()`; just call
670+
/// **Note** that *expression* is converted to an `ArrayViewMut` using
671+
/// `ArrayViewMut::from()` even if there are no `mut` patterns. If all you want
672+
/// to do is take read-only slices, you don't need `multislice!()`; just call
673673
/// [`.slice()`](struct.ArrayBase.html#method.slice) multiple times instead.
674674
///
675675
/// `multislice!()` evaluates to a tuple of `ArrayView` and/or `ArrayViewMut`
@@ -697,7 +697,7 @@ pub unsafe fn deref_raw_view_mut_into_view_mut_with_life<'a, A, D: Dimension>(
697697
///
698698
/// # fn main() {
699699
/// let mut arr: Array1<_> = (0..12).collect();
700-
/// let (a, b, c, d) = multislice!(arr, [0..5], mut [6..;2], [1..6], mut [7..;2]);
700+
/// let (a, b, c, d) = multislice!(&mut arr, [0..5], mut [6..;2], [1..6], mut [7..;2]);
701701
/// assert_eq!(a, array![0, 1, 2, 3, 4]);
702702
/// assert_eq!(b, array![6, 8, 10]);
703703
/// assert_eq!(c, array![1, 2, 3, 4, 5]);
@@ -715,7 +715,7 @@ pub unsafe fn deref_raw_view_mut_into_view_mut_with_life<'a, A, D: Dimension>(
715715
/// # use ndarray::prelude::*;
716716
/// # fn main() {
717717
/// let mut arr: Array1<_> = (0..12).collect();
718-
/// multislice!(arr, [0..5], mut [1..;2]); // panic!
718+
/// multislice!(&mut arr, [0..5], mut [1..;2]); // panic!
719719
/// # }
720720
/// ```
721721
///
@@ -727,7 +727,7 @@ pub unsafe fn deref_raw_view_mut_into_view_mut_with_life<'a, A, D: Dimension>(
727727
/// # use ndarray::prelude::*;
728728
/// # fn main() {
729729
/// let mut arr: Array1<_> = (0..12).collect();
730-
/// multislice!(arr, mut [0..5], mut [1..;2]); // panic!
730+
/// multislice!(&mut arr, mut [0..5], mut [1..;2]); // panic!
731731
/// # }
732732
/// ```
733733
#[macro_export]
@@ -985,7 +985,7 @@ macro_rules! multislice(
985985
($arr:expr, $($t:tt)*) => {
986986
{
987987
let (life, raw_view) = {
988-
let mut view = $crate::ArrayBase::view_mut(&mut $arr);
988+
let mut view: $crate::ArrayViewMut<_, _> = ::core::convert::From::from($arr);
989989
($crate::life_of_view_mut(&view), view.raw_view_mut())
990990
};
991991
$crate::multislice!(@parse raw_view, life, (), (), (), ($($t)*))

tests/array.rs

+75-53
Original file line numberDiff line numberDiff line change
@@ -336,76 +336,76 @@ fn test_slice_collapse_with_indices() {
336336
}
337337

338338
#[test]
339-
#[allow(clippy::cognitive_complexity)]
340339
fn test_multislice() {
341-
defmac!(test_multislice mut arr, s1, s2 => {
342-
{
343-
let copy = arr.clone();
344-
assert_eq!(
345-
multislice!(arr, mut s1, mut s2,),
346-
(copy.clone().slice_mut(s1), copy.clone().slice_mut(s2))
347-
);
348-
}
349-
{
350-
let copy = arr.clone();
351-
assert_eq!(
352-
multislice!(arr, mut s1, s2,),
353-
(copy.clone().slice_mut(s1), copy.clone().slice(s2))
354-
);
355-
}
356-
{
357-
let copy = arr.clone();
358-
assert_eq!(
359-
multislice!(arr, s1, mut s2),
360-
(copy.clone().slice(s1), copy.clone().slice_mut(s2))
361-
);
362-
}
363-
{
364-
let copy = arr.clone();
365-
assert_eq!(
366-
multislice!(arr, s1, s2),
367-
(copy.clone().slice(s1), copy.clone().slice(s2))
368-
);
369-
}
370-
});
340+
macro_rules! check_multislice {
341+
($arr:expr, $s1:expr, $s2:expr) => {
342+
{
343+
let copy = $arr.clone();
344+
assert_eq!(
345+
multislice!(&mut $arr, mut $s1, mut $s2,),
346+
(copy.clone().slice_mut($s1), copy.clone().slice_mut($s2))
347+
);
348+
}
349+
{
350+
let copy = $arr.clone();
351+
assert_eq!(
352+
multislice!(&mut $arr, mut $s1, $s2,),
353+
(copy.clone().slice_mut($s1), copy.clone().slice($s2))
354+
);
355+
}
356+
{
357+
let copy = $arr.clone();
358+
assert_eq!(
359+
multislice!(&mut $arr, $s1, mut $s2),
360+
(copy.clone().slice($s1), copy.clone().slice_mut($s2))
361+
);
362+
}
363+
{
364+
let copy = $arr.clone();
365+
assert_eq!(
366+
multislice!(&mut $arr, $s1, $s2),
367+
(copy.clone().slice($s1), copy.clone().slice($s2))
368+
);
369+
}
370+
};
371+
};
371372
let mut arr = Array1::from_iter(0..48).into_shape((8, 6)).unwrap();
372-
373-
assert_eq!((arr.clone().view(),), multislice!(arr, [.., ..]));
374-
test_multislice!(&mut arr, s![0, ..], s![1, ..]);
375-
test_multislice!(&mut arr, s![0, ..], s![-1, ..]);
376-
test_multislice!(&mut arr, s![0, ..], s![1.., ..]);
377-
test_multislice!(&mut arr, s![1, ..], s![..;2, ..]);
378-
test_multislice!(&mut arr, s![..2, ..], s![2.., ..]);
379-
test_multislice!(&mut arr, s![1..;2, ..], s![..;2, ..]);
380-
test_multislice!(&mut arr, s![..;-2, ..], s![..;2, ..]);
381-
test_multislice!(&mut arr, s![..;12, ..], s![3..;3, ..]);
373+
assert_eq!((arr.clone().view(),), multislice!(&mut arr, [.., ..]));
374+
check_multislice!(arr, s![0, ..], s![1, ..]);
375+
check_multislice!(arr, s![0, ..], s![-1, ..]);
376+
check_multislice!(arr, s![0, ..], s![1.., ..]);
377+
check_multislice!(arr, s![1, ..], s![..;2, ..]);
378+
check_multislice!(arr, s![..2, ..], s![2.., ..]);
379+
check_multislice!(arr, s![1..;2, ..], s![..;2, ..]);
380+
check_multislice!(arr, s![..;-2, ..], s![..;2, ..]);
381+
check_multislice!(arr, s![..;12, ..], s![3..;3, ..]);
382382
}
383383

384384
#[test]
385385
fn test_multislice_intersecting() {
386386
assert_panics!({
387387
let mut arr = Array2::<u8>::zeros((8, 6));
388-
multislice!(arr, mut [3, ..], [3, ..]);
388+
multislice!(&mut arr, mut [3, ..], [3, ..]);
389389
});
390390
assert_panics!({
391391
let mut arr = Array2::<u8>::zeros((8, 6));
392-
multislice!(arr, mut [3, ..], [3.., ..]);
392+
multislice!(&mut arr, mut [3, ..], [3.., ..]);
393393
});
394394
assert_panics!({
395395
let mut arr = Array2::<u8>::zeros((8, 6));
396-
multislice!(arr, mut [3, ..], [..;3, ..]);
396+
multislice!(&mut arr, mut [3, ..], [..;3, ..]);
397397
});
398398
assert_panics!({
399399
let mut arr = Array2::<u8>::zeros((8, 6));
400-
multislice!(arr, mut [..;6, ..], [3..;3, ..]);
400+
multislice!(&mut arr, mut [..;6, ..], [3..;3, ..]);
401401
});
402402
assert_panics!({
403403
let mut arr = Array2::<u8>::zeros((8, 6));
404-
multislice!(arr, mut [2, ..], mut [..-1;-2, ..]);
404+
multislice!(&mut arr, mut [2, ..], mut [..-1;-2, ..]);
405405
});
406406
{
407407
let mut arr = Array2::<u8>::zeros((8, 6));
408-
multislice!(arr, [3, ..], [-1..;-2, ..]);
408+
multislice!(&mut arr, [3, ..], [-1..;-2, ..]);
409409
}
410410
}
411411

@@ -418,7 +418,7 @@ fn test_multislice_eval_args_only_once() {
418418
eval_count += 1;
419419
*s![1..2]
420420
};
421-
multislice!(arr, mut &slice(), [3..4], [5..6]);
421+
multislice!(&mut arr, mut &slice(), [3..4], [5..6]);
422422
}
423423
assert_eq!(eval_count, 1);
424424
let mut eval_count = 0;
@@ -427,7 +427,7 @@ fn test_multislice_eval_args_only_once() {
427427
eval_count += 1;
428428
*s![1..2]
429429
};
430-
multislice!(arr, [3..4], mut &slice(), [5..6]);
430+
multislice!(&mut arr, [3..4], mut &slice(), [5..6]);
431431
}
432432
assert_eq!(eval_count, 1);
433433
let mut eval_count = 0;
@@ -436,7 +436,7 @@ fn test_multislice_eval_args_only_once() {
436436
eval_count += 1;
437437
*s![1..2]
438438
};
439-
multislice!(arr, [3..4], [5..6], mut &slice());
439+
multislice!(&mut arr, [3..4], [5..6], mut &slice());
440440
}
441441
assert_eq!(eval_count, 1);
442442
let mut eval_count = 0;
@@ -445,7 +445,7 @@ fn test_multislice_eval_args_only_once() {
445445
eval_count += 1;
446446
*s![1..2]
447447
};
448-
multislice!(arr, &slice(), mut [3..4], [5..6]);
448+
multislice!(&mut arr, &slice(), mut [3..4], [5..6]);
449449
}
450450
assert_eq!(eval_count, 1);
451451
let mut eval_count = 0;
@@ -454,7 +454,7 @@ fn test_multislice_eval_args_only_once() {
454454
eval_count += 1;
455455
*s![1..2]
456456
};
457-
multislice!(arr, mut [3..4], &slice(), [5..6]);
457+
multislice!(&mut arr, mut [3..4], &slice(), [5..6]);
458458
}
459459
assert_eq!(eval_count, 1);
460460
let mut eval_count = 0;
@@ -463,11 +463,33 @@ fn test_multislice_eval_args_only_once() {
463463
eval_count += 1;
464464
*s![1..2]
465465
};
466-
multislice!(arr, mut [3..4], [5..6], &slice());
466+
multislice!(&mut arr, mut [3..4], [5..6], &slice());
467467
}
468468
assert_eq!(eval_count, 1);
469469
}
470470

471+
#[test]
472+
fn test_multislice_arrayviewmut_same_life() {
473+
// This test makes sure that it's possible for the borrowed elements
474+
// returned from `get_mut2` to have the same life as the `arr` view.
475+
fn get_mut2<'a, A>(
476+
arr: ArrayViewMut<'a, A, Ix2>,
477+
[i1, j1]: [usize; 2],
478+
[i2, j2]: [usize; 2],
479+
) -> (&'a mut A, &'a mut A) {
480+
use ndarray::IndexLonger;
481+
let (x1, x2) = multislice!(arr, mut [i1, j1], mut [i2, j2]);
482+
(x1.index([]), x2.index([]))
483+
}
484+
let mut arr = array![[1, 2], [3, 4]];
485+
{
486+
let (x1, x2) = get_mut2(arr.view_mut(), [0, 0], [1, 0]);
487+
*x1 += 1;
488+
*x2 += 2;
489+
}
490+
assert_eq!(arr, array![[2, 2], [5, 4]]);
491+
}
492+
471493
#[should_panic]
472494
#[test]
473495
fn index_out_of_bounds() {

0 commit comments

Comments
 (0)