Skip to content

Commit d4c857a

Browse files
ChayimFriedman2gitbot
authored and
gitbot
committed
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 face2ac commit d4c857a

File tree

3 files changed

+34
-28
lines changed

3 files changed

+34
-28
lines changed

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))]

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 {}

core/src/slice/mod.rs

+31-27
Original file line numberDiff line numberDiff line change
@@ -4629,13 +4629,11 @@ impl<T> [T] {
46294629
pub fn get_many_mut<I, const N: usize>(
46304630
&mut self,
46314631
indices: [I; N],
4632-
) -> Result<[&mut I::Output; N], GetManyMutError<N>>
4632+
) -> Result<[&mut I::Output; N], GetManyMutError>
46334633
where
46344634
I: GetManyMutIndex + SliceIndex<Self>,
46354635
{
4636-
if !get_many_check_valid(&indices, self.len()) {
4637-
return Err(GetManyMutError { _private: () });
4638-
}
4636+
get_many_check_valid(&indices, self.len())?;
46394637
// SAFETY: The `get_many_check_valid()` call checked that all indices
46404638
// are disjunct and in bounds.
46414639
unsafe { Ok(self.get_many_unchecked_mut(indices)) }
@@ -4978,53 +4976,59 @@ impl<T, const N: usize> SlicePattern for [T; N] {
49784976
/// This will do `binomial(N + 1, 2) = N * (N + 1) / 2 = 0, 1, 3, 6, 10, ..`
49794977
/// comparison operations.
49804978
#[inline]
4981-
fn get_many_check_valid<I: GetManyMutIndex, const N: usize>(indices: &[I; N], len: usize) -> bool {
4979+
fn get_many_check_valid<I: GetManyMutIndex, const N: usize>(
4980+
indices: &[I; N],
4981+
len: usize,
4982+
) -> Result<(), GetManyMutError> {
49824983
// NB: The optimizer should inline the loops into a sequence
49834984
// of instructions without additional branching.
4984-
let mut valid = true;
49854985
for (i, idx) in indices.iter().enumerate() {
4986-
valid &= idx.is_in_bounds(len);
4986+
if !idx.is_in_bounds(len) {
4987+
return Err(GetManyMutError::IndexOutOfBounds);
4988+
}
49874989
for idx2 in &indices[..i] {
4988-
valid &= !idx.is_overlapping(idx2);
4990+
if idx.is_overlapping(idx2) {
4991+
return Err(GetManyMutError::OverlappingIndices);
4992+
}
49894993
}
49904994
}
4991-
valid
4995+
Ok(())
49924996
}
49934997

4994-
/// The error type returned by [`get_many_mut<N>`][`slice::get_many_mut`].
4998+
/// The error type returned by [`get_many_mut`][`slice::get_many_mut`].
49954999
///
49965000
/// It indicates one of two possible errors:
49975001
/// - An index is out-of-bounds.
4998-
/// - The same index appeared multiple times in the array.
5002+
/// - The same index appeared multiple times in the array
5003+
/// (or different but overlapping indices when ranges are provided).
49995004
///
50005005
/// # Examples
50015006
///
50025007
/// ```
50035008
/// #![feature(get_many_mut)]
5009+
/// use std::slice::GetManyMutError;
50045010
///
50055011
/// let v = &mut [1, 2, 3];
5006-
/// assert!(v.get_many_mut([0, 999]).is_err());
5007-
/// assert!(v.get_many_mut([1, 1]).is_err());
5012+
/// assert_eq!(v.get_many_mut([0, 999]), Err(GetManyMutError::IndexOutOfBounds));
5013+
/// assert_eq!(v.get_many_mut([1, 1]), Err(GetManyMutError::OverlappingIndices));
50085014
/// ```
50095015
#[unstable(feature = "get_many_mut", issue = "104642")]
5010-
// NB: The N here is there to be forward-compatible with adding more details
5011-
// to the error type at a later point
5012-
#[derive(Clone, PartialEq, Eq)]
5013-
pub struct GetManyMutError<const N: usize> {
5014-
_private: (),
5016+
#[derive(Debug, Clone, PartialEq, Eq)]
5017+
pub enum GetManyMutError {
5018+
/// An index provided was out-of-bounds for the slice.
5019+
IndexOutOfBounds,
5020+
/// Two indices provided were overlapping.
5021+
OverlappingIndices,
50155022
}
50165023

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

0 commit comments

Comments
 (0)