Skip to content

Commit fa87a3e

Browse files
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()`.
1 parent 7e565cc commit fa87a3e

File tree

3 files changed

+34
-28
lines changed

3 files changed

+34
-28
lines changed

library/alloc/src/slice.rs

+2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ pub use core::slice::ArrayChunksMut;
2727
pub use core::slice::ArrayWindows;
2828
#[stable(feature = "inherent_ascii_escape", since = "1.60.0")]
2929
pub use core::slice::EscapeAscii;
30+
#[unstable(feature = "get_many_mut", issue = "104642")]
31+
pub use core::slice::GetManyMutError;
3032
#[stable(feature = "slice_get_slice", since = "1.28.0")]
3133
pub use core::slice::SliceIndex;
3234
#[cfg(not(no_global_oom_handling))]

library/core/src/error.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1076,4 +1076,4 @@ impl Error for crate::time::TryFromFloatSecsError {}
10761076
impl Error for crate::ffi::FromBytesUntilNulError {}
10771077

10781078
#[unstable(feature = "get_many_mut", issue = "104642")]
1079-
impl<const N: usize> Error for crate::slice::GetManyMutError<N> {}
1079+
impl Error for crate::slice::GetManyMutError {}

library/core/src/slice/mod.rs

+31-27
Original file line numberDiff line numberDiff line change
@@ -4627,13 +4627,11 @@ impl<T> [T] {
46274627
pub fn get_many_mut<I, const N: usize>(
46284628
&mut self,
46294629
indices: [I; N],
4630-
) -> Result<[&mut I::Output; N], GetManyMutError<N>>
4630+
) -> Result<[&mut I::Output; N], GetManyMutError>
46314631
where
46324632
I: GetManyMutIndex + SliceIndex<Self>,
46334633
{
4634-
if !get_many_check_valid(&indices, self.len()) {
4635-
return Err(GetManyMutError { _private: () });
4636-
}
4634+
get_many_check_valid(&indices, self.len())?;
46374635
// SAFETY: The `get_many_check_valid()` call checked that all indices
46384636
// are disjunct and in bounds.
46394637
unsafe { Ok(self.get_many_unchecked_mut(indices)) }
@@ -4976,53 +4974,59 @@ impl<T, const N: usize> SlicePattern for [T; N] {
49764974
/// This will do `binomial(N + 1, 2) = N * (N + 1) / 2 = 0, 1, 3, 6, 10, ..`
49774975
/// comparison operations.
49784976
#[inline]
4979-
fn get_many_check_valid<I: GetManyMutIndex, const N: usize>(indices: &[I; N], len: usize) -> bool {
4977+
fn get_many_check_valid<I: GetManyMutIndex, const N: usize>(
4978+
indices: &[I; N],
4979+
len: usize,
4980+
) -> Result<(), GetManyMutError> {
49804981
// NB: The optimizer should inline the loops into a sequence
49814982
// of instructions without additional branching.
4982-
let mut valid = true;
49834983
for (i, idx) in indices.iter().enumerate() {
4984-
valid &= idx.is_in_bounds(len);
4984+
if !idx.is_in_bounds(len) {
4985+
return Err(GetManyMutError::IndexOutOfBounds);
4986+
}
49854987
for idx2 in &indices[..i] {
4986-
valid &= !idx.is_overlapping(idx2);
4988+
if idx.is_overlapping(idx2) {
4989+
return Err(GetManyMutError::OverlappingIndices);
4990+
}
49874991
}
49884992
}
4989-
valid
4993+
Ok(())
49904994
}
49914995

4992-
/// The error type returned by [`get_many_mut<N>`][`slice::get_many_mut`].
4996+
/// The error type returned by [`get_many_mut`][`slice::get_many_mut`].
49934997
///
49944998
/// It indicates one of two possible errors:
49954999
/// - An index is out-of-bounds.
4996-
/// - The same index appeared multiple times in the array.
5000+
/// - The same index appeared multiple times in the array
5001+
/// (or different but overlapping indices when ranges are provided).
49975002
///
49985003
/// # Examples
49995004
///
50005005
/// ```
50015006
/// #![feature(get_many_mut)]
5007+
/// use std::slice::GetManyMutError;
50025008
///
50035009
/// let v = &mut [1, 2, 3];
5004-
/// assert!(v.get_many_mut([0, 999]).is_err());
5005-
/// assert!(v.get_many_mut([1, 1]).is_err());
5010+
/// assert_eq!(v.get_many_mut([0, 999]), Err(GetManyMutError::IndexOutOfBounds));
5011+
/// assert_eq!(v.get_many_mut([1, 1]), Err(GetManyMutError::OverlappingIndices));
50065012
/// ```
50075013
#[unstable(feature = "get_many_mut", issue = "104642")]
5008-
// NB: The N here is there to be forward-compatible with adding more details
5009-
// to the error type at a later point
5010-
#[derive(Clone, PartialEq, Eq)]
5011-
pub struct GetManyMutError<const N: usize> {
5012-
_private: (),
5014+
#[derive(Debug, Clone, PartialEq, Eq)]
5015+
pub enum GetManyMutError {
5016+
/// An index provided was out-of-bounds for the slice.
5017+
IndexOutOfBounds,
5018+
/// Two indices provided were overlapping.
5019+
OverlappingIndices,
50135020
}
50145021

50155022
#[unstable(feature = "get_many_mut", issue = "104642")]
5016-
impl<const N: usize> fmt::Debug for GetManyMutError<N> {
5023+
impl fmt::Display for GetManyMutError {
50175024
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
5018-
f.debug_struct("GetManyMutError").finish_non_exhaustive()
5019-
}
5020-
}
5021-
5022-
#[unstable(feature = "get_many_mut", issue = "104642")]
5023-
impl<const N: usize> fmt::Display for GetManyMutError<N> {
5024-
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
5025-
fmt::Display::fmt("an index is out of bounds or appeared multiple times in the array", f)
5025+
let msg = match self {
5026+
GetManyMutError::IndexOutOfBounds => "an index is out of bounds",
5027+
GetManyMutError::OverlappingIndices => "there were overlapping indices",
5028+
};
5029+
fmt::Display::fmt(msg, f)
50265030
}
50275031
}
50285032

0 commit comments

Comments
 (0)