Skip to content

Commit 8a51ff9

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 8a51ff9

File tree

3 files changed

+69
-49
lines changed

3 files changed

+69
-49
lines changed

src/lib.rs

Lines changed: 1 addition & 1 deletion
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

Lines changed: 9 additions & 9 deletions
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

Lines changed: 59 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -336,76 +336,74 @@ 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();
340+
macro_rules! check_multislice {
341+
($arr:expr, $s1:expr, $s2:expr) => {{
342+
let copy = $arr.clone();
344343
assert_eq!(
345-
multislice!(arr, mut s1, mut s2,),
346-
(copy.clone().slice_mut(s1), copy.clone().slice_mut(s2))
344+
multislice!(&mut $arr, mut $s1, mut $s2,),
345+
(copy.clone().slice_mut($s1), copy.clone().slice_mut($s2))
347346
);
348347
}
349348
{
350-
let copy = arr.clone();
349+
let copy = $arr.clone();
351350
assert_eq!(
352-
multislice!(arr, mut s1, s2,),
353-
(copy.clone().slice_mut(s1), copy.clone().slice(s2))
351+
multislice!(&mut $arr, mut $s1, $s2,),
352+
(copy.clone().slice_mut($s1), copy.clone().slice($s2))
354353
);
355354
}
356355
{
357-
let copy = arr.clone();
356+
let copy = $arr.clone();
358357
assert_eq!(
359-
multislice!(arr, s1, mut s2),
360-
(copy.clone().slice(s1), copy.clone().slice_mut(s2))
358+
multislice!(&mut $arr, $s1, mut $s2),
359+
(copy.clone().slice($s1), copy.clone().slice_mut($s2))
361360
);
362361
}
363362
{
364-
let copy = arr.clone();
363+
let copy = $arr.clone();
365364
assert_eq!(
366-
multislice!(arr, s1, s2),
367-
(copy.clone().slice(s1), copy.clone().slice(s2))
365+
multislice!(&mut $arr, $s1, $s2),
366+
(copy.clone().slice($s1), copy.clone().slice($s2))
368367
);
369-
}
370-
});
368+
}};
369+
};
371370
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, ..]);
371+
assert_eq!((arr.clone().view(),), multislice!(&mut arr, [.., ..]));
372+
check_multislice!(arr, s![0, ..], s![1, ..]);
373+
check_multislice!(arr, s![0, ..], s![-1, ..]);
374+
check_multislice!(arr, s![0, ..], s![1.., ..]);
375+
check_multislice!(arr, s![1, ..], s![..;2, ..]);
376+
check_multislice!(arr, s![..2, ..], s![2.., ..]);
377+
check_multislice!(arr, s![1..;2, ..], s![..;2, ..]);
378+
check_multislice!(arr, s![..;-2, ..], s![..;2, ..]);
379+
check_multislice!(arr, s![..;12, ..], s![3..;3, ..]);
382380
}
383381

384382
#[test]
385383
fn test_multislice_intersecting() {
386384
assert_panics!({
387385
let mut arr = Array2::<u8>::zeros((8, 6));
388-
multislice!(arr, mut [3, ..], [3, ..]);
386+
multislice!(&mut arr, mut [3, ..], [3, ..]);
389387
});
390388
assert_panics!({
391389
let mut arr = Array2::<u8>::zeros((8, 6));
392-
multislice!(arr, mut [3, ..], [3.., ..]);
390+
multislice!(&mut arr, mut [3, ..], [3.., ..]);
393391
});
394392
assert_panics!({
395393
let mut arr = Array2::<u8>::zeros((8, 6));
396-
multislice!(arr, mut [3, ..], [..;3, ..]);
394+
multislice!(&mut arr, mut [3, ..], [..;3, ..]);
397395
});
398396
assert_panics!({
399397
let mut arr = Array2::<u8>::zeros((8, 6));
400-
multislice!(arr, mut [..;6, ..], [3..;3, ..]);
398+
multislice!(&mut arr, mut [..;6, ..], [3..;3, ..]);
401399
});
402400
assert_panics!({
403401
let mut arr = Array2::<u8>::zeros((8, 6));
404-
multislice!(arr, mut [2, ..], mut [..-1;-2, ..]);
402+
multislice!(&mut arr, mut [2, ..], mut [..-1;-2, ..]);
405403
});
406404
{
407405
let mut arr = Array2::<u8>::zeros((8, 6));
408-
multislice!(arr, [3, ..], [-1..;-2, ..]);
406+
multislice!(&mut arr, [3, ..], [-1..;-2, ..]);
409407
}
410408
}
411409

@@ -418,7 +416,7 @@ fn test_multislice_eval_args_only_once() {
418416
eval_count += 1;
419417
*s![1..2]
420418
};
421-
multislice!(arr, mut &slice(), [3..4], [5..6]);
419+
multislice!(&mut arr, mut &slice(), [3..4], [5..6]);
422420
}
423421
assert_eq!(eval_count, 1);
424422
let mut eval_count = 0;
@@ -427,7 +425,7 @@ fn test_multislice_eval_args_only_once() {
427425
eval_count += 1;
428426
*s![1..2]
429427
};
430-
multislice!(arr, [3..4], mut &slice(), [5..6]);
428+
multislice!(&mut arr, [3..4], mut &slice(), [5..6]);
431429
}
432430
assert_eq!(eval_count, 1);
433431
let mut eval_count = 0;
@@ -436,7 +434,7 @@ fn test_multislice_eval_args_only_once() {
436434
eval_count += 1;
437435
*s![1..2]
438436
};
439-
multislice!(arr, [3..4], [5..6], mut &slice());
437+
multislice!(&mut arr, [3..4], [5..6], mut &slice());
440438
}
441439
assert_eq!(eval_count, 1);
442440
let mut eval_count = 0;
@@ -445,7 +443,7 @@ fn test_multislice_eval_args_only_once() {
445443
eval_count += 1;
446444
*s![1..2]
447445
};
448-
multislice!(arr, &slice(), mut [3..4], [5..6]);
446+
multislice!(&mut arr, &slice(), mut [3..4], [5..6]);
449447
}
450448
assert_eq!(eval_count, 1);
451449
let mut eval_count = 0;
@@ -454,7 +452,7 @@ fn test_multislice_eval_args_only_once() {
454452
eval_count += 1;
455453
*s![1..2]
456454
};
457-
multislice!(arr, mut [3..4], &slice(), [5..6]);
455+
multislice!(&mut arr, mut [3..4], &slice(), [5..6]);
458456
}
459457
assert_eq!(eval_count, 1);
460458
let mut eval_count = 0;
@@ -463,11 +461,33 @@ fn test_multislice_eval_args_only_once() {
463461
eval_count += 1;
464462
*s![1..2]
465463
};
466-
multislice!(arr, mut [3..4], [5..6], &slice());
464+
multislice!(&mut arr, mut [3..4], [5..6], &slice());
467465
}
468466
assert_eq!(eval_count, 1);
469467
}
470468

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

0 commit comments

Comments
 (0)