Skip to content

Commit 118bfc5

Browse files
ChayimFriedman2gitbot
authored and
gitbot
committed
Support ranges in <[T]>::get_many_mut()
I implemented that with a separate trait and not within `SliceIndex`, because doing that via `SliceIndex` requires adding support for range types that are (almost) always overlapping e.g. `RangeFrom`, and also adding fake support code for `impl SliceIndex<str>`. An inconvenience that I ran into was that slice indexing takes the index by value, but I only have it by reference. I could change slice indexing to take by ref, but this is pretty much the hottest code ever so I'm afraid to touch it. Instead I added a requirement for `Clone` (which all index types implement anyway) and cloned. This is an internal requirement the user won't see and the clone should always be optimized away. I also implemented `Clone`, `PartialEq` and `Eq` for the error type, since I noticed it does not do that when writing the tests and other errors in std seem to implement them. I didn't implement `Copy` because maybe we will want to put something non-`Copy` there.
1 parent d17849f commit 118bfc5

File tree

2 files changed

+249
-19
lines changed

2 files changed

+249
-19
lines changed

core/src/slice/mod.rs

+180-18
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ use crate::cmp::Ordering::{self, Equal, Greater, Less};
1010
use crate::intrinsics::{exact_div, select_unpredictable, unchecked_sub};
1111
use crate::mem::{self, SizedTypeProperties};
1212
use crate::num::NonZero;
13-
use crate::ops::{Bound, OneSidedRange, Range, RangeBounds};
13+
use crate::ops::{Bound, OneSidedRange, Range, RangeBounds, RangeInclusive};
1414
use crate::simd::{self, Simd};
1515
use crate::ub_checks::assert_unsafe_precondition;
16-
use crate::{fmt, hint, ptr, slice};
16+
use crate::{fmt, hint, ptr, range, slice};
1717

1818
#[unstable(
1919
feature = "slice_internals",
@@ -4469,6 +4469,12 @@ impl<T> [T] {
44694469

44704470
/// Returns mutable references to many indices at once, without doing any checks.
44714471
///
4472+
/// An index can be either a `usize`, a [`Range`] or a [`RangeInclusive`]. Note
4473+
/// that this method takes an array, so all indices must be of the same type.
4474+
/// If passed an array of `usize`s this method gives back an array of mutable references
4475+
/// to single elements, while if passed an array of ranges it gives back an array of
4476+
/// mutable references to slices.
4477+
///
44724478
/// For a safe alternative see [`get_many_mut`].
44734479
///
44744480
/// # Safety
@@ -4489,39 +4495,68 @@ impl<T> [T] {
44894495
/// *b *= 100;
44904496
/// }
44914497
/// assert_eq!(x, &[10, 2, 400]);
4498+
///
4499+
/// unsafe {
4500+
/// let [a, b] = x.get_many_unchecked_mut([0..1, 1..3]);
4501+
/// a[0] = 8;
4502+
/// b[0] = 88;
4503+
/// b[1] = 888;
4504+
/// }
4505+
/// assert_eq!(x, &[8, 88, 888]);
4506+
///
4507+
/// unsafe {
4508+
/// let [a, b] = x.get_many_unchecked_mut([1..=2, 0..=0]);
4509+
/// a[0] = 11;
4510+
/// a[1] = 111;
4511+
/// b[0] = 1;
4512+
/// }
4513+
/// assert_eq!(x, &[1, 11, 111]);
44924514
/// ```
44934515
///
44944516
/// [`get_many_mut`]: slice::get_many_mut
44954517
/// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
44964518
#[unstable(feature = "get_many_mut", issue = "104642")]
44974519
#[inline]
4498-
pub unsafe fn get_many_unchecked_mut<const N: usize>(
4520+
pub unsafe fn get_many_unchecked_mut<I, const N: usize>(
44994521
&mut self,
4500-
indices: [usize; N],
4501-
) -> [&mut T; N] {
4522+
indices: [I; N],
4523+
) -> [&mut I::Output; N]
4524+
where
4525+
I: GetManyMutIndex + SliceIndex<Self>,
4526+
{
45024527
// NB: This implementation is written as it is because any variation of
45034528
// `indices.map(|i| self.get_unchecked_mut(i))` would make miri unhappy,
45044529
// or generate worse code otherwise. This is also why we need to go
45054530
// through a raw pointer here.
45064531
let slice: *mut [T] = self;
4507-
let mut arr: mem::MaybeUninit<[&mut T; N]> = mem::MaybeUninit::uninit();
4532+
let mut arr: mem::MaybeUninit<[&mut I::Output; N]> = mem::MaybeUninit::uninit();
45084533
let arr_ptr = arr.as_mut_ptr();
45094534

45104535
// SAFETY: We expect `indices` to contain disjunct values that are
45114536
// in bounds of `self`.
45124537
unsafe {
45134538
for i in 0..N {
4514-
let idx = *indices.get_unchecked(i);
4515-
*(*arr_ptr).get_unchecked_mut(i) = &mut *slice.get_unchecked_mut(idx);
4539+
let idx = indices.get_unchecked(i).clone();
4540+
arr_ptr.cast::<&mut I::Output>().add(i).write(&mut *slice.get_unchecked_mut(idx));
45164541
}
45174542
arr.assume_init()
45184543
}
45194544
}
45204545

45214546
/// Returns mutable references to many indices at once.
45224547
///
4523-
/// Returns an error if any index is out-of-bounds, or if the same index was
4524-
/// passed more than once.
4548+
/// An index can be either a `usize`, a [`Range`] or a [`RangeInclusive`]. Note
4549+
/// that this method takes an array, so all indices must be of the same type.
4550+
/// If passed an array of `usize`s this method gives back an array of mutable references
4551+
/// to single elements, while if passed an array of ranges it gives back an array of
4552+
/// mutable references to slices.
4553+
///
4554+
/// Returns an error if any index is out-of-bounds, or if there are overlapping indices.
4555+
/// An empty range is not considered to overlap if it is located at the beginning or at
4556+
/// the end of another range, but is considered to overlap if it is located in the middle.
4557+
///
4558+
/// This method does a O(n^2) check to check that there are no overlapping indices, so be careful
4559+
/// when passing many indices.
45254560
///
45264561
/// # Examples
45274562
///
@@ -4534,13 +4569,30 @@ impl<T> [T] {
45344569
/// *b = 612;
45354570
/// }
45364571
/// assert_eq!(v, &[413, 2, 612]);
4572+
///
4573+
/// if let Ok([a, b]) = v.get_many_mut([0..1, 1..3]) {
4574+
/// a[0] = 8;
4575+
/// b[0] = 88;
4576+
/// b[1] = 888;
4577+
/// }
4578+
/// assert_eq!(v, &[8, 88, 888]);
4579+
///
4580+
/// if let Ok([a, b]) = v.get_many_mut([1..=2, 0..=0]) {
4581+
/// a[0] = 11;
4582+
/// a[1] = 111;
4583+
/// b[0] = 1;
4584+
/// }
4585+
/// assert_eq!(v, &[1, 11, 111]);
45374586
/// ```
45384587
#[unstable(feature = "get_many_mut", issue = "104642")]
45394588
#[inline]
4540-
pub fn get_many_mut<const N: usize>(
4589+
pub fn get_many_mut<I, const N: usize>(
45414590
&mut self,
4542-
indices: [usize; N],
4543-
) -> Result<[&mut T; N], GetManyMutError<N>> {
4591+
indices: [I; N],
4592+
) -> Result<[&mut I::Output; N], GetManyMutError<N>>
4593+
where
4594+
I: GetManyMutIndex + SliceIndex<Self>,
4595+
{
45444596
if !get_many_check_valid(&indices, self.len()) {
45454597
return Err(GetManyMutError { _private: () });
45464598
}
@@ -4885,14 +4937,15 @@ impl<T, const N: usize> SlicePattern for [T; N] {
48854937
///
48864938
/// This will do `binomial(N + 1, 2) = N * (N + 1) / 2 = 0, 1, 3, 6, 10, ..`
48874939
/// comparison operations.
4888-
fn get_many_check_valid<const N: usize>(indices: &[usize; N], len: usize) -> bool {
4940+
#[inline]
4941+
fn get_many_check_valid<I: GetManyMutIndex, const N: usize>(indices: &[I; N], len: usize) -> bool {
48894942
// NB: The optimizer should inline the loops into a sequence
48904943
// of instructions without additional branching.
48914944
let mut valid = true;
4892-
for (i, &idx) in indices.iter().enumerate() {
4893-
valid &= idx < len;
4894-
for &idx2 in &indices[..i] {
4895-
valid &= idx != idx2;
4945+
for (i, idx) in indices.iter().enumerate() {
4946+
valid &= idx.is_in_bounds(len);
4947+
for idx2 in &indices[..i] {
4948+
valid &= !idx.is_overlapping(idx2);
48964949
}
48974950
}
48984951
valid
@@ -4916,6 +4969,7 @@ fn get_many_check_valid<const N: usize>(indices: &[usize; N], len: usize) -> boo
49164969
#[unstable(feature = "get_many_mut", issue = "104642")]
49174970
// NB: The N here is there to be forward-compatible with adding more details
49184971
// to the error type at a later point
4972+
#[derive(Clone, PartialEq, Eq)]
49194973
pub struct GetManyMutError<const N: usize> {
49204974
_private: (),
49214975
}
@@ -4933,3 +4987,111 @@ impl<const N: usize> fmt::Display for GetManyMutError<N> {
49334987
fmt::Display::fmt("an index is out of bounds or appeared multiple times in the array", f)
49344988
}
49354989
}
4990+
4991+
mod private_get_many_mut_index {
4992+
use super::{Range, RangeInclusive, range};
4993+
4994+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
4995+
pub trait Sealed {}
4996+
4997+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
4998+
impl Sealed for usize {}
4999+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
5000+
impl Sealed for Range<usize> {}
5001+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
5002+
impl Sealed for RangeInclusive<usize> {}
5003+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
5004+
impl Sealed for range::Range<usize> {}
5005+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
5006+
impl Sealed for range::RangeInclusive<usize> {}
5007+
}
5008+
5009+
/// A helper trait for `<[T]>::get_many_mut()`.
5010+
///
5011+
/// # Safety
5012+
///
5013+
/// If `is_in_bounds()` returns `true` and `is_overlapping()` returns `false`,
5014+
/// it must be safe to index the slice with the indices.
5015+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
5016+
pub unsafe trait GetManyMutIndex: Clone + private_get_many_mut_index::Sealed {
5017+
/// Returns `true` if `self` is in bounds for `len` slice elements.
5018+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
5019+
fn is_in_bounds(&self, len: usize) -> bool;
5020+
5021+
/// Returns `true` if `self` overlaps with `other`.
5022+
///
5023+
/// Note that we don't consider zero-length ranges to overlap at the beginning or the end,
5024+
/// but do consider them to overlap in the middle.
5025+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
5026+
fn is_overlapping(&self, other: &Self) -> bool;
5027+
}
5028+
5029+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
5030+
// SAFETY: We implement `is_in_bounds()` and `is_overlapping()` correctly.
5031+
unsafe impl GetManyMutIndex for usize {
5032+
#[inline]
5033+
fn is_in_bounds(&self, len: usize) -> bool {
5034+
*self < len
5035+
}
5036+
5037+
#[inline]
5038+
fn is_overlapping(&self, other: &Self) -> bool {
5039+
*self == *other
5040+
}
5041+
}
5042+
5043+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
5044+
// SAFETY: We implement `is_in_bounds()` and `is_overlapping()` correctly.
5045+
unsafe impl GetManyMutIndex for Range<usize> {
5046+
#[inline]
5047+
fn is_in_bounds(&self, len: usize) -> bool {
5048+
(self.start <= self.end) & (self.end <= len)
5049+
}
5050+
5051+
#[inline]
5052+
fn is_overlapping(&self, other: &Self) -> bool {
5053+
(self.start < other.end) & (other.start < self.end)
5054+
}
5055+
}
5056+
5057+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
5058+
// SAFETY: We implement `is_in_bounds()` and `is_overlapping()` correctly.
5059+
unsafe impl GetManyMutIndex for RangeInclusive<usize> {
5060+
#[inline]
5061+
fn is_in_bounds(&self, len: usize) -> bool {
5062+
(self.start <= self.end) & (self.end < len)
5063+
}
5064+
5065+
#[inline]
5066+
fn is_overlapping(&self, other: &Self) -> bool {
5067+
(self.start <= other.end) & (other.start <= self.end)
5068+
}
5069+
}
5070+
5071+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
5072+
// SAFETY: We implement `is_in_bounds()` and `is_overlapping()` correctly.
5073+
unsafe impl GetManyMutIndex for range::Range<usize> {
5074+
#[inline]
5075+
fn is_in_bounds(&self, len: usize) -> bool {
5076+
Range::from(*self).is_in_bounds(len)
5077+
}
5078+
5079+
#[inline]
5080+
fn is_overlapping(&self, other: &Self) -> bool {
5081+
Range::from(*self).is_overlapping(&Range::from(*other))
5082+
}
5083+
}
5084+
5085+
#[unstable(feature = "get_many_mut_helpers", issue = "none")]
5086+
// SAFETY: We implement `is_in_bounds()` and `is_overlapping()` correctly.
5087+
unsafe impl GetManyMutIndex for range::RangeInclusive<usize> {
5088+
#[inline]
5089+
fn is_in_bounds(&self, len: usize) -> bool {
5090+
RangeInclusive::from(*self).is_in_bounds(len)
5091+
}
5092+
5093+
#[inline]
5094+
fn is_overlapping(&self, other: &Self) -> bool {
5095+
RangeInclusive::from(*self).is_overlapping(&RangeInclusive::from(*other))
5096+
}
5097+
}

core/tests/slice.rs

+69-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use core::cell::Cell;
22
use core::cmp::Ordering;
33
use core::mem::MaybeUninit;
44
use core::num::NonZero;
5+
use core::ops::{Range, RangeInclusive};
56
use core::slice;
67

78
#[test]
@@ -2553,6 +2554,14 @@ fn test_get_many_mut_normal_2() {
25532554
*a += 10;
25542555
*b += 100;
25552556
assert_eq!(v, vec![101, 2, 3, 14, 5]);
2557+
2558+
let [a, b] = v.get_many_mut([0..=1, 2..=2]).unwrap();
2559+
assert_eq!(a, &mut [101, 2][..]);
2560+
assert_eq!(b, &mut [3][..]);
2561+
a[0] += 10;
2562+
a[1] += 20;
2563+
b[0] += 100;
2564+
assert_eq!(v, vec![111, 22, 103, 14, 5]);
25562565
}
25572566

25582567
#[test]
@@ -2563,12 +2572,23 @@ fn test_get_many_mut_normal_3() {
25632572
*b += 100;
25642573
*c += 1000;
25652574
assert_eq!(v, vec![11, 2, 1003, 4, 105]);
2575+
2576+
let [a, b, c] = v.get_many_mut([0..1, 4..5, 1..4]).unwrap();
2577+
assert_eq!(a, &mut [11][..]);
2578+
assert_eq!(b, &mut [105][..]);
2579+
assert_eq!(c, &mut [2, 1003, 4][..]);
2580+
a[0] += 10;
2581+
b[0] += 100;
2582+
c[0] += 1000;
2583+
assert_eq!(v, vec![21, 1002, 1003, 4, 205]);
25662584
}
25672585

25682586
#[test]
25692587
fn test_get_many_mut_empty() {
25702588
let mut v = vec![1, 2, 3, 4, 5];
2571-
let [] = v.get_many_mut([]).unwrap();
2589+
let [] = v.get_many_mut::<usize, 0>([]).unwrap();
2590+
let [] = v.get_many_mut::<RangeInclusive<usize>, 0>([]).unwrap();
2591+
let [] = v.get_many_mut::<Range<usize>, 0>([]).unwrap();
25722592
assert_eq!(v, vec![1, 2, 3, 4, 5]);
25732593
}
25742594

@@ -2606,6 +2626,54 @@ fn test_get_many_mut_duplicate() {
26062626
assert!(v.get_many_mut([1, 3, 3, 4]).is_err());
26072627
}
26082628

2629+
#[test]
2630+
fn test_get_many_mut_range_oob() {
2631+
let mut v = vec![1, 2, 3, 4, 5];
2632+
assert!(v.get_many_mut([0..6]).is_err());
2633+
assert!(v.get_many_mut([5..6]).is_err());
2634+
assert!(v.get_many_mut([6..6]).is_err());
2635+
assert!(v.get_many_mut([0..=5]).is_err());
2636+
assert!(v.get_many_mut([0..=6]).is_err());
2637+
assert!(v.get_many_mut([5..=5]).is_err());
2638+
}
2639+
2640+
#[test]
2641+
fn test_get_many_mut_range_overlapping() {
2642+
let mut v = vec![1, 2, 3, 4, 5];
2643+
assert!(v.get_many_mut([0..1, 0..2]).is_err());
2644+
assert!(v.get_many_mut([0..1, 1..2, 0..1]).is_err());
2645+
assert!(v.get_many_mut([0..3, 1..1]).is_err());
2646+
assert!(v.get_many_mut([0..3, 1..2]).is_err());
2647+
assert!(v.get_many_mut([0..=0, 2..=2, 0..=1]).is_err());
2648+
assert!(v.get_many_mut([0..=4, 0..=0]).is_err());
2649+
assert!(v.get_many_mut([4..=4, 0..=0, 3..=4]).is_err());
2650+
}
2651+
2652+
#[test]
2653+
fn test_get_many_mut_range_empty_at_edge() {
2654+
let mut v = vec![1, 2, 3, 4, 5];
2655+
assert_eq!(
2656+
v.get_many_mut([0..0, 0..5, 5..5]),
2657+
Ok([&mut [][..], &mut [1, 2, 3, 4, 5], &mut []]),
2658+
);
2659+
assert_eq!(
2660+
v.get_many_mut([0..0, 0..1, 1..1, 1..2, 2..2, 2..3, 3..3, 3..4, 4..4, 4..5, 5..5]),
2661+
Ok([
2662+
&mut [][..],
2663+
&mut [1],
2664+
&mut [],
2665+
&mut [2],
2666+
&mut [],
2667+
&mut [3],
2668+
&mut [],
2669+
&mut [4],
2670+
&mut [],
2671+
&mut [5],
2672+
&mut [],
2673+
]),
2674+
);
2675+
}
2676+
26092677
#[test]
26102678
fn test_slice_from_raw_parts_in_const() {
26112679
static FANCY: i32 = 4;

0 commit comments

Comments
 (0)