Skip to content

Commit

Permalink
Change GetManyMutError to match T-libs-api decision
Browse files Browse the repository at this point in the history
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()`.
  • Loading branch information
ChayimFriedman2 authored and gitbot committed Feb 20, 2025
1 parent 0652319 commit a0662fc
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 28 deletions.
2 changes: 2 additions & 0 deletions alloc/src/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ pub use core::slice::ArrayChunksMut;
pub use core::slice::ArrayWindows;
#[stable(feature = "inherent_ascii_escape", since = "1.60.0")]
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")]
pub use core::slice::SliceIndex;
#[cfg(not(no_global_oom_handling))]
Expand Down
2 changes: 1 addition & 1 deletion core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1076,4 +1076,4 @@ impl Error for crate::time::TryFromFloatSecsError {}
impl Error for crate::ffi::FromBytesUntilNulError {}

#[unstable(feature = "get_many_mut", issue = "104642")]
impl<const N: usize> Error for crate::slice::GetManyMutError<N> {}
impl Error for crate::slice::GetManyMutError {}
58 changes: 31 additions & 27 deletions core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4629,13 +4629,11 @@ impl<T> [T] {
pub fn get_many_mut<I, const N: usize>(
&mut self,
indices: [I; N],
) -> Result<[&mut I::Output; N], GetManyMutError<N>>
) -> Result<[&mut I::Output; N], GetManyMutError>
where
I: GetManyMutIndex + SliceIndex<Self>,
{
if !get_many_check_valid(&indices, self.len()) {
return Err(GetManyMutError { _private: () });
}
get_many_check_valid(&indices, self.len())?;
// SAFETY: The `get_many_check_valid()` call checked that all indices
// are disjunct and in bounds.
unsafe { Ok(self.get_many_unchecked_mut(indices)) }
Expand Down Expand Up @@ -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, ..`
/// comparison operations.
#[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
// of instructions without additional branching.
let mut valid = true;
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] {
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:
/// - 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
///
/// ```
/// #![feature(get_many_mut)]
/// use std::slice::GetManyMutError;
///
/// let v = &mut [1, 2, 3];
/// assert!(v.get_many_mut([0, 999]).is_err());
/// assert!(v.get_many_mut([1, 1]).is_err());
/// assert_eq!(v.get_many_mut([0, 999]), Err(GetManyMutError::IndexOutOfBounds));
/// assert_eq!(v.get_many_mut([1, 1]), Err(GetManyMutError::OverlappingIndices));
/// ```
#[unstable(feature = "get_many_mut", issue = "104642")]
// NB: The N here is there to be forward-compatible with adding more details
// to the error type at a later point
#[derive(Clone, PartialEq, Eq)]
pub struct GetManyMutError<const N: usize> {
_private: (),
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum GetManyMutError {
/// An index provided was out-of-bounds for the slice.
IndexOutOfBounds,
/// Two indices provided were overlapping.
OverlappingIndices,
}

#[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 {
f.debug_struct("GetManyMutError").finish_non_exhaustive()
}
}

#[unstable(feature = "get_many_mut", issue = "104642")]
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)
let msg = match self {
GetManyMutError::IndexOutOfBounds => "an index is out of bounds",
GetManyMutError::OverlappingIndices => "there were overlapping indices",
};
fmt::Display::fmt(msg, f)
}
}

Expand Down

0 comments on commit a0662fc

Please sign in to comment.