about summary refs log tree commit diff
diff options
context:
space:
mode:
authorChayim Refael Friedman <chayimfr@gmail.com>2024-11-28 23:34:47 +0200
committerChayim Refael Friedman <chayimfr@gmail.com>2024-11-28 23:34:47 +0200
commitfa87a3e88942e407a2214728bb466620f538d13d (patch)
tree281053032580490caede6c4b2dbe9a8ee6000992
parent7e565cce6a03340edb4b9f56228cf5e480e24806 (diff)
downloadrust-fa87a3e88942e407a2214728bb466620f538d13d.tar.gz
rust-fa87a3e88942e407a2214728bb466620f538d13d.zip
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()`.
-rw-r--r--library/alloc/src/slice.rs2
-rw-r--r--library/core/src/error.rs2
-rw-r--r--library/core/src/slice/mod.rs58
3 files changed, 34 insertions, 28 deletions
diff --git a/library/alloc/src/slice.rs b/library/alloc/src/slice.rs
index e3c7835f1d1..edc8d99f2f9 100644
--- a/library/alloc/src/slice.rs
+++ b/library/alloc/src/slice.rs
@@ -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))]
diff --git a/library/core/src/error.rs b/library/core/src/error.rs
index 95a39cc3aed..91549f49f9f 100644
--- a/library/core/src/error.rs
+++ b/library/core/src/error.rs
@@ -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 {}
diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs
index ee91479bb1a..40b197f8f18 100644
--- a/library/core/src/slice/mod.rs
+++ b/library/core/src/slice/mod.rs
@@ -4627,13 +4627,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)) }
@@ -4976,53 +4974,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)
     }
 }