Rollup merge of #133598 - ChayimFriedman2:get-many-mut-detailed-err, r=scottmcm

Change `GetManyMutError` to match T-libs-api decision

That is, differentiate between out-of-bounds and overlapping indices, and remove the generic parameter `N`.

I also exported `GetManyMutError` from `alloc` (and `std`), which was apparently forgotten.

Changing the error to carry additional details means LLVM no longer generates separate short-circuiting branches for the checks, instead it generates one branch at the end. I therefore changed the  code to use early returns to make LLVM generate jumps. Benchmark results between the approaches are somewhat mixed, but I chose this approach because it is significantly faster with ranges and also faster with `unwrap()`.

Benchmark (`jumps` refer to short-circuiting, `acc` is not short-circuiting):
```rust
use criterion::{black_box, criterion_group, criterion_main, Criterion};
use my_crate::{get_many_check_valid_acc, get_many_check_valid_jumps, GetManyMutError};

mod externs {
    #[unsafe(no_mangle)]
    fn foo() {}
    #[unsafe(no_mangle)]
    fn bar() {}
    #[unsafe(no_mangle)]
    fn baz() {}
}

unsafe extern "C" {
    safe fn foo();
    safe fn bar();
    safe fn baz();
}

fn bench_method(c: &mut Criterion) {
    c.bench_function("jumps two usize", |b| {
        b.iter(|| get_many_check_valid_jumps(&[black_box(1), black_box(5)], black_box(10)))
    });
    c.bench_function("jumps two usize unwrap", |b| {
        b.iter(|| get_many_check_valid_jumps(&[black_box(1), black_box(5)], black_box(10)).unwrap())
    });
    c.bench_function("jumps two usize ok", |b| {
        b.iter(|| get_many_check_valid_jumps(&[black_box(1), black_box(5)], black_box(10)).ok())
    });
    c.bench_function("jumps three usize", |b| {
        b.iter(|| {
            get_many_check_valid_jumps(&[black_box(1), black_box(5), black_box(7)], black_box(10))
        })
    });
    c.bench_function("jumps three usize match", |b| {
        b.iter(|| {
            match get_many_check_valid_jumps(
                &[black_box(1), black_box(5), black_box(7)],
                black_box(10),
            ) {
                Err(GetManyMutError::IndexOutOfBounds) => foo(),
                Err(GetManyMutError::OverlappingIndices) => bar(),
                Ok(()) => baz(),
            }
        })
    });
    c.bench_function("jumps two Range", |b| {
        b.iter(|| {
            get_many_check_valid_jumps(
                &[black_box(1)..black_box(5), black_box(7)..black_box(8)],
                black_box(10),
            )
        })
    });
    c.bench_function("jumps two RangeInclusive", |b| {
        b.iter(|| {
            get_many_check_valid_jumps(
                &[black_box(1)..=black_box(5), black_box(7)..=black_box(8)],
                black_box(10),
            )
        })
    });

    c.bench_function("acc two usize", |b| {
        b.iter(|| get_many_check_valid_acc(&[black_box(1), black_box(5)], black_box(10)))
    });
    c.bench_function("acc two usize unwrap", |b| {
        b.iter(|| get_many_check_valid_acc(&[black_box(1), black_box(5)], black_box(10)).unwrap())
    });
    c.bench_function("acc two usize ok", |b| {
        b.iter(|| get_many_check_valid_acc(&[black_box(1), black_box(5)], black_box(10)).ok())
    });
    c.bench_function("acc three usize", |b| {
        b.iter(|| {
            get_many_check_valid_acc(&[black_box(1), black_box(5), black_box(7)], black_box(10))
        })
    });
    c.bench_function("acc three usize match", |b| {
        b.iter(|| {
            match get_many_check_valid_jumps(
                &[black_box(1), black_box(5), black_box(7)],
                black_box(10),
            ) {
                Err(GetManyMutError::IndexOutOfBounds) => foo(),
                Err(GetManyMutError::OverlappingIndices) => bar(),
                Ok(()) => baz(),
            }
        })
    });
    c.bench_function("acc two Range", |b| {
        b.iter(|| {
            get_many_check_valid_acc(
                &[black_box(1)..black_box(5), black_box(7)..black_box(8)],
                black_box(10),
            )
        })
    });
    c.bench_function("acc two RangeInclusive", |b| {
        b.iter(|| {
            get_many_check_valid_acc(
                &[black_box(1)..=black_box(5), black_box(7)..=black_box(8)],
                black_box(10),
            )
        })
    });
}

criterion_group!(benches, bench_method);
criterion_main!(benches);
```
Benchmark results:
```none
jumps two usize          time:   [586.44 ps 590.20 ps 594.50 ps]
jumps two usize unwrap   time:   [390.44 ps 393.63 ps 397.44 ps]
jumps two usize ok       time:   [585.52 ps 591.74 ps 599.38 ps]
jumps three usize        time:   [976.51 ps 983.79 ps 991.51 ps]
jumps three usize match  time:   [390.82 ps 393.80 ps 397.07 ps]
jumps two Range          time:   [1.2583 ns 1.2640 ns 1.2695 ns]
jumps two RangeInclusive time:   [1.2673 ns 1.2770 ns 1.2877 ns]
acc two usize            time:   [592.63 ps 596.44 ps 600.52 ps]
acc two usize unwrap     time:   [582.65 ps 587.07 ps 591.90 ps]
acc two usize ok         time:   [581.59 ps 587.82 ps 595.71 ps]
acc three usize          time:   [894.69 ps 901.23 ps 908.24 ps]
acc three usize match    time:   [392.68 ps 395.73 ps 399.17 ps]
acc two Range            time:   [1.5531 ns 1.5617 ns 1.5711 ns]
acc two RangeInclusive   time:   [1.5746 ns 1.5840 ns 1.5939 ns]
```
This commit is contained in:
Matthias Krüger 2024-12-11 20:00:14 +01:00 committed by GitHub
commit 2e60288ce0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 34 additions and 28 deletions

View File

@ -27,6 +27,8 @@ pub use core::slice::ArrayChunksMut;
pub use core::slice::ArrayWindows; pub use core::slice::ArrayWindows;
#[stable(feature = "inherent_ascii_escape", since = "1.60.0")] #[stable(feature = "inherent_ascii_escape", since = "1.60.0")]
pub use core::slice::EscapeAscii; pub use core::slice::EscapeAscii;
#[unstable(feature = "get_many_mut", issue = "104642")]
pub use core::slice::GetManyMutError;
#[stable(feature = "slice_get_slice", since = "1.28.0")] #[stable(feature = "slice_get_slice", since = "1.28.0")]
pub use core::slice::SliceIndex; pub use core::slice::SliceIndex;
#[cfg(not(no_global_oom_handling))] #[cfg(not(no_global_oom_handling))]

View File

@ -1076,4 +1076,4 @@ impl Error for crate::time::TryFromFloatSecsError {}
impl Error for crate::ffi::FromBytesUntilNulError {} impl Error for crate::ffi::FromBytesUntilNulError {}
#[unstable(feature = "get_many_mut", issue = "104642")] #[unstable(feature = "get_many_mut", issue = "104642")]
impl<const N: usize> Error for crate::slice::GetManyMutError<N> {} impl Error for crate::slice::GetManyMutError {}

View File

@ -4629,13 +4629,11 @@ impl<T> [T] {
pub fn get_many_mut<I, const N: usize>( pub fn get_many_mut<I, const N: usize>(
&mut self, &mut self,
indices: [I; N], indices: [I; N],
) -> Result<[&mut I::Output; N], GetManyMutError<N>> ) -> Result<[&mut I::Output; N], GetManyMutError>
where where
I: GetManyMutIndex + SliceIndex<Self>, I: GetManyMutIndex + SliceIndex<Self>,
{ {
if !get_many_check_valid(&indices, self.len()) { get_many_check_valid(&indices, self.len())?;
return Err(GetManyMutError { _private: () });
}
// SAFETY: The `get_many_check_valid()` call checked that all indices // SAFETY: The `get_many_check_valid()` call checked that all indices
// are disjunct and in bounds. // are disjunct and in bounds.
unsafe { Ok(self.get_many_unchecked_mut(indices)) } unsafe { Ok(self.get_many_unchecked_mut(indices)) }
@ -4978,53 +4976,59 @@ impl<T, const N: usize> SlicePattern for [T; N] {
/// This will do `binomial(N + 1, 2) = N * (N + 1) / 2 = 0, 1, 3, 6, 10, ..` /// This will do `binomial(N + 1, 2) = N * (N + 1) / 2 = 0, 1, 3, 6, 10, ..`
/// comparison operations. /// comparison operations.
#[inline] #[inline]
fn get_many_check_valid<I: GetManyMutIndex, const N: usize>(indices: &[I; N], len: usize) -> bool { fn get_many_check_valid<I: GetManyMutIndex, const N: usize>(
indices: &[I; N],
len: usize,
) -> Result<(), GetManyMutError> {
// NB: The optimizer should inline the loops into a sequence // NB: The optimizer should inline the loops into a sequence
// of instructions without additional branching. // of instructions without additional branching.
let mut valid = true;
for (i, idx) in indices.iter().enumerate() { for (i, idx) in indices.iter().enumerate() {
valid &= idx.is_in_bounds(len); if !idx.is_in_bounds(len) {
return Err(GetManyMutError::IndexOutOfBounds);
}
for idx2 in &indices[..i] { for idx2 in &indices[..i] {
valid &= !idx.is_overlapping(idx2); if idx.is_overlapping(idx2) {
return Err(GetManyMutError::OverlappingIndices);
}
} }
} }
valid Ok(())
} }
/// The error type returned by [`get_many_mut<N>`][`slice::get_many_mut`]. /// The error type returned by [`get_many_mut`][`slice::get_many_mut`].
/// ///
/// It indicates one of two possible errors: /// It indicates one of two possible errors:
/// - An index is out-of-bounds. /// - An index is out-of-bounds.
/// - The same index appeared multiple times in the array. /// - The same index appeared multiple times in the array
/// (or different but overlapping indices when ranges are provided).
/// ///
/// # Examples /// # Examples
/// ///
/// ``` /// ```
/// #![feature(get_many_mut)] /// #![feature(get_many_mut)]
/// use std::slice::GetManyMutError;
/// ///
/// let v = &mut [1, 2, 3]; /// let v = &mut [1, 2, 3];
/// assert!(v.get_many_mut([0, 999]).is_err()); /// assert_eq!(v.get_many_mut([0, 999]), Err(GetManyMutError::IndexOutOfBounds));
/// assert!(v.get_many_mut([1, 1]).is_err()); /// assert_eq!(v.get_many_mut([1, 1]), Err(GetManyMutError::OverlappingIndices));
/// ``` /// ```
#[unstable(feature = "get_many_mut", issue = "104642")] #[unstable(feature = "get_many_mut", issue = "104642")]
// NB: The N here is there to be forward-compatible with adding more details #[derive(Debug, Clone, PartialEq, Eq)]
// to the error type at a later point pub enum GetManyMutError {
#[derive(Clone, PartialEq, Eq)] /// An index provided was out-of-bounds for the slice.
pub struct GetManyMutError<const N: usize> { IndexOutOfBounds,
_private: (), /// Two indices provided were overlapping.
OverlappingIndices,
} }
#[unstable(feature = "get_many_mut", issue = "104642")] #[unstable(feature = "get_many_mut", issue = "104642")]
impl<const N: usize> fmt::Debug for GetManyMutError<N> { impl fmt::Display for GetManyMutError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("GetManyMutError").finish_non_exhaustive() let msg = match self {
} GetManyMutError::IndexOutOfBounds => "an index is out of bounds",
} GetManyMutError::OverlappingIndices => "there were overlapping indices",
};
#[unstable(feature = "get_many_mut", issue = "104642")] fmt::Display::fmt(msg, f)
impl<const N: usize> fmt::Display for GetManyMutError<N> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt("an index is out of bounds or appeared multiple times in the array", f)
} }
} }