From 4c3bd81856bb86e9a04835b61098622bd9251028 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sun, 28 Jul 2024 20:25:57 +0300 Subject: [PATCH 1/4] In `<[T]>::get_many_mut()`, where N>=9, do a O(N*lg(N)) check instead of O(N^2) by sorting first --- library/core/src/slice/mod.rs | 36 ++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 8b502624176b6..c9bafa8318b0b 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -4851,20 +4851,30 @@ impl SlicePattern for [T; N] { } /// This checks every index against each other, and against `len`. -/// -/// This will do `binomial(N + 1, 2) = N * (N + 1) / 2 = 0, 1, 3, 6, 10, ..` -/// comparison operations. fn get_many_check_valid(indices: &[usize; N], len: usize) -> bool { - // 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 < len; - for &idx2 in &indices[..i] { - valid &= idx != idx2; + // Based on benchmarks, it is faster to sort starting with 9 indices. + if N >= 9 { + let mut sorted_indices = *indices; + sorted_indices.sort_unstable(); + for &[i, j] in sorted_indices.array_windows() { + if i == j { + return false; + } + } + let biggest_index = *sorted_indices.last().expect("indices array should not be empty"); + biggest_index < len + } else { + // 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 < len; + for &idx2 in &indices[..i] { + valid &= idx != idx2; + } } + valid } - valid } /// The error type returned by [`get_many_mut`][`slice::get_many_mut`]. @@ -4883,8 +4893,8 @@ fn get_many_check_valid(indices: &[usize; N], len: usize) -> boo /// assert!(v.get_many_mut([1, 1]).is_err()); /// ``` #[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 +// NB: The N and the private field here is there to be forward-compatible with +// adding more details to the error type at a later point pub struct GetManyMutError { _private: (), } From 42bb6774b94f8ac948162a9bf61c8f8089b5e681 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sun, 28 Jul 2024 21:26:54 +0300 Subject: [PATCH 2/4] Take `&[usize; N]` instead of `[usize; N]` in `<[T]>::get_many_mut()` We only need an immutable access to the indices array, and it we take it owned we may force a copy for big arrays. For small arrays, LLVM will fully inline and unroll the body anyway, so this doesn't matter. In simple cases, LLVM is able to avoid the copy, but it has troubles in more complex cases (https://godbolt.org/z/s371G4r9e). Given that I expect `get_many_mut()` to be used most of the times with few, probably only two, indices, this seems unlikely to matter and adds one character to type, but it isn't that bad and avoiding a copy seems nice. --- library/core/src/slice/mod.rs | 14 +++++++------- library/core/tests/slice.rs | 16 ++++++++-------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index c9bafa8318b0b..4582b0107ee39 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -4453,7 +4453,7 @@ impl [T] { /// let x = &mut [1, 2, 4]; /// /// unsafe { - /// let [a, b] = x.get_many_unchecked_mut([0, 2]); + /// let [a, b] = x.get_many_unchecked_mut(&[0, 2]); /// *a *= 10; /// *b *= 100; /// } @@ -4466,7 +4466,7 @@ impl [T] { #[inline] pub unsafe fn get_many_unchecked_mut( &mut self, - indices: [usize; N], + indices: &[usize; N], ) -> [&mut T; N] { // NB: This implementation is written as it is because any variation of // `indices.map(|i| self.get_unchecked_mut(i))` would make miri unhappy, @@ -4498,7 +4498,7 @@ impl [T] { /// #![feature(get_many_mut)] /// /// let v = &mut [1, 2, 3]; - /// if let Ok([a, b]) = v.get_many_mut([0, 2]) { + /// if let Ok([a, b]) = v.get_many_mut(&[0, 2]) { /// *a = 413; /// *b = 612; /// } @@ -4508,9 +4508,9 @@ impl [T] { #[inline] pub fn get_many_mut( &mut self, - indices: [usize; N], + indices: &[usize; N], ) -> Result<[&mut T; N], GetManyMutError> { - if !get_many_check_valid(&indices, self.len()) { + if !get_many_check_valid(indices, self.len()) { return Err(GetManyMutError { _private: () }); } // SAFETY: The `get_many_check_valid()` call checked that all indices @@ -4889,8 +4889,8 @@ fn get_many_check_valid(indices: &[usize; N], len: usize) -> boo /// #![feature(get_many_mut)] /// /// 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!(v.get_many_mut(&[0, 999]).is_err()); +/// assert!(v.get_many_mut(&[1, 1]).is_err()); /// ``` #[unstable(feature = "get_many_mut", issue = "104642")] // NB: The N and the private field here is there to be forward-compatible with diff --git a/library/core/tests/slice.rs b/library/core/tests/slice.rs index 4cbbabb672ba0..f3e7d27dc77af 100644 --- a/library/core/tests/slice.rs +++ b/library/core/tests/slice.rs @@ -2599,7 +2599,7 @@ fn test_flatten_mut_size_overflow() { #[test] fn test_get_many_mut_normal_2() { let mut v = vec![1, 2, 3, 4, 5]; - let [a, b] = v.get_many_mut([3, 0]).unwrap(); + let [a, b] = v.get_many_mut(&[3, 0]).unwrap(); *a += 10; *b += 100; assert_eq!(v, vec![101, 2, 3, 14, 5]); @@ -2608,7 +2608,7 @@ fn test_get_many_mut_normal_2() { #[test] fn test_get_many_mut_normal_3() { let mut v = vec![1, 2, 3, 4, 5]; - let [a, b, c] = v.get_many_mut([0, 4, 2]).unwrap(); + let [a, b, c] = v.get_many_mut(&[0, 4, 2]).unwrap(); *a += 10; *b += 100; *c += 1000; @@ -2618,14 +2618,14 @@ fn test_get_many_mut_normal_3() { #[test] fn test_get_many_mut_empty() { let mut v = vec![1, 2, 3, 4, 5]; - let [] = v.get_many_mut([]).unwrap(); + let [] = v.get_many_mut(&[]).unwrap(); assert_eq!(v, vec![1, 2, 3, 4, 5]); } #[test] fn test_get_many_mut_single_first() { let mut v = vec![1, 2, 3, 4, 5]; - let [a] = v.get_many_mut([0]).unwrap(); + let [a] = v.get_many_mut(&[0]).unwrap(); *a += 10; assert_eq!(v, vec![11, 2, 3, 4, 5]); } @@ -2633,7 +2633,7 @@ fn test_get_many_mut_single_first() { #[test] fn test_get_many_mut_single_last() { let mut v = vec![1, 2, 3, 4, 5]; - let [a] = v.get_many_mut([4]).unwrap(); + let [a] = v.get_many_mut(&[4]).unwrap(); *a += 10; assert_eq!(v, vec![1, 2, 3, 4, 15]); } @@ -2641,19 +2641,19 @@ fn test_get_many_mut_single_last() { #[test] fn test_get_many_mut_oob_nonempty() { let mut v = vec![1, 2, 3, 4, 5]; - assert!(v.get_many_mut([5]).is_err()); + assert!(v.get_many_mut(&[5]).is_err()); } #[test] fn test_get_many_mut_oob_empty() { let mut v: Vec = vec![]; - assert!(v.get_many_mut([0]).is_err()); + assert!(v.get_many_mut(&[0]).is_err()); } #[test] fn test_get_many_mut_duplicate() { let mut v = vec![1, 2, 3, 4, 5]; - assert!(v.get_many_mut([1, 3, 3, 4]).is_err()); + assert!(v.get_many_mut(&[1, 3, 3, 4]).is_err()); } #[test] From d618ebcce380b422a0d04841e131b360d417c29f Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sun, 28 Jul 2024 21:41:58 +0300 Subject: [PATCH 3/4] Re-export `GetManyMutError` from `alloc::slice` and `std::slice` --- library/alloc/src/slice.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/alloc/src/slice.rs b/library/alloc/src/slice.rs index c7960b3fb49c3..c229f448b0774 100644 --- a/library/alloc/src/slice.rs +++ b/library/alloc/src/slice.rs @@ -41,6 +41,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; #[stable(feature = "from_ref", since = "1.28.0")] From 3b9517df220fad90757dceea36a96ef09d6005e5 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sun, 28 Jul 2024 22:01:22 +0300 Subject: [PATCH 4/4] Stabilize `<[T]>::get_many_mut()` --- library/alloc/src/slice.rs | 2 +- library/core/src/error.rs | 2 +- library/core/src/slice/mod.rs | 16 +++++----------- library/core/tests/lib.rs | 1 - library/std/src/lib.rs | 1 - 5 files changed, 7 insertions(+), 15 deletions(-) diff --git a/library/alloc/src/slice.rs b/library/alloc/src/slice.rs index c229f448b0774..b404bd6f2f1bb 100644 --- a/library/alloc/src/slice.rs +++ b/library/alloc/src/slice.rs @@ -41,7 +41,7 @@ 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")] +#[stable(feature = "get_many_mut", since = "CURRENT_RUSTC_VERSION")] pub use core::slice::GetManyMutError; #[stable(feature = "slice_get_slice", since = "1.28.0")] pub use core::slice::SliceIndex; diff --git a/library/core/src/error.rs b/library/core/src/error.rs index 19b7bb44f855a..e252df6d5441e 100644 --- a/library/core/src/error.rs +++ b/library/core/src/error.rs @@ -1076,5 +1076,5 @@ impl Error for crate::time::TryFromFloatSecsError {} #[stable(feature = "cstr_from_bytes_until_nul", since = "1.69.0")] impl Error for crate::ffi::FromBytesUntilNulError {} -#[unstable(feature = "get_many_mut", issue = "104642")] +#[stable(feature = "get_many_mut", since = "CURRENT_RUSTC_VERSION")] impl Error for crate::slice::GetManyMutError {} diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 4582b0107ee39..abaa8ce28a21c 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -4448,8 +4448,6 @@ impl [T] { /// # Examples /// /// ``` - /// #![feature(get_many_mut)] - /// /// let x = &mut [1, 2, 4]; /// /// unsafe { @@ -4462,7 +4460,7 @@ impl [T] { /// /// [`get_many_mut`]: slice::get_many_mut /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html - #[unstable(feature = "get_many_mut", issue = "104642")] + #[stable(feature = "get_many_mut", since = "CURRENT_RUSTC_VERSION")] #[inline] pub unsafe fn get_many_unchecked_mut( &mut self, @@ -4495,8 +4493,6 @@ impl [T] { /// # Examples /// /// ``` - /// #![feature(get_many_mut)] - /// /// let v = &mut [1, 2, 3]; /// if let Ok([a, b]) = v.get_many_mut(&[0, 2]) { /// *a = 413; @@ -4504,7 +4500,7 @@ impl [T] { /// } /// assert_eq!(v, &[413, 2, 612]); /// ``` - #[unstable(feature = "get_many_mut", issue = "104642")] + #[stable(feature = "get_many_mut", since = "CURRENT_RUSTC_VERSION")] #[inline] pub fn get_many_mut( &mut self, @@ -4886,27 +4882,25 @@ fn get_many_check_valid(indices: &[usize; N], len: usize) -> boo /// # Examples /// /// ``` -/// #![feature(get_many_mut)] -/// /// let v = &mut [1, 2, 3]; /// assert!(v.get_many_mut(&[0, 999]).is_err()); /// assert!(v.get_many_mut(&[1, 1]).is_err()); /// ``` -#[unstable(feature = "get_many_mut", issue = "104642")] +#[stable(feature = "get_many_mut", since = "CURRENT_RUSTC_VERSION")] // NB: The N and the private field here is there to be forward-compatible with // adding more details to the error type at a later point pub struct GetManyMutError { _private: (), } -#[unstable(feature = "get_many_mut", issue = "104642")] +#[stable(feature = "get_many_mut", since = "CURRENT_RUSTC_VERSION")] impl fmt::Debug 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")] +#[stable(feature = "get_many_mut", since = "CURRENT_RUSTC_VERSION")] impl fmt::Display for GetManyMutError { 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) diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index 51d57c9e37d7c..e16c7740e38b1 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -110,7 +110,6 @@ #![feature(error_generic_member_access)] #![feature(trait_upcasting)] #![feature(is_ascii_octdigit)] -#![feature(get_many_mut)] #![feature(iter_map_windows)] #![allow(internal_features)] #![deny(unsafe_op_in_unsafe_fn)] diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 9fba657d116de..0e6443c676ac8 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -392,7 +392,6 @@ #![feature(custom_test_frameworks)] #![feature(edition_panic)] #![feature(format_args_nl)] -#![feature(get_many_mut)] #![feature(log_syntax)] #![feature(test)] #![feature(trace_macros)]