From c9fd0afe04be79b9e3cc4dfcdd4cc11616aa76a9 Mon Sep 17 00:00:00 2001 From: Niklas Jonsson Date: Thu, 14 Jul 2022 19:52:18 +0200 Subject: [PATCH 1/6] Implement get_disjoint_mut for arrays of keys (cherry picked from commit 5aec9ec674d40f2c2da74ef6a335353cc41092dc) --- src/map.rs | 69 ++++++++++++++++++++++++++++++++++++ src/map/tests.rs | 91 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+) diff --git a/src/map.rs b/src/map.rs index 101f2ae..360635c 100644 --- a/src/map.rs +++ b/src/map.rs @@ -825,6 +825,37 @@ where } } + /// Return the values for `N` keys. If any key is missing a value, or there + /// are duplicate keys, `None` is returned. + /// + /// # Examples + /// + /// ``` + /// let mut map = ringmap::RingMap::from([(1, 'a'), (3, 'b'), (2, 'c')]); + /// assert_eq!(map.get_disjoint_mut([&2, &1]), Some([&mut 'c', &mut 'a'])); + /// ``` + pub fn get_disjoint_mut(&mut self, keys: [&Q; N]) -> Option<[&mut V; N]> + where + Q: Hash + Equivalent + ?Sized, + { + let len = self.len(); + let indices = keys.map(|key| self.get_index_of(key)); + + // Handle out-of-bounds indices with panic as this is an internal error in get_index_of. + for idx in indices { + let idx = idx?; + debug_assert!( + idx < len, + "Index is out of range! Got '{}' but length is '{}'", + idx, + len + ); + } + let indices = indices.map(Option::unwrap); + let entries = self.get_disjoint_indices_mut(indices)?; + Some(entries.map(|(_key, value)| value)) + } + /// Remove the key-value pair equivalent to `key` and return its value. /// /// Like [`VecDeque::remove`], the pair is removed by shifting all of the @@ -1286,6 +1317,44 @@ impl RingMap { Some(IndexedEntry::new(&mut self.core, index)) } + /// Get an array of `N` key-value pairs by `N` indices + /// + /// Valid indices are *0 <= index < self.len()* and each index needs to be unique. + /// + /// Computes in **O(1)** time. + /// + /// # Examples + /// + /// ``` + /// let mut map = ringmap::RingMap::from([(1, 'a'), (3, 'b'), (2, 'c')]); + /// assert_eq!(map.get_disjoint_indices_mut([2, 0]), Some([(&2, &mut 'c'), (&1, &mut 'a')])); + /// ``` + pub fn get_disjoint_indices_mut( + &mut self, + indices: [usize; N], + ) -> Option<[(&K, &mut V); N]> { + // SAFETY: Can't allow duplicate indices as we would return several mutable refs to the same data. + let len = self.len(); + for i in 0..N { + let idx = indices[i]; + if idx >= len || indices[i + 1..N].contains(&idx) { + return None; + } + } + + let entries_ptr = self.as_entries_mut().as_mut_ptr(); + let out = indices.map(|i| { + // SAFETY: The base pointer is valid as it comes from a slice and the deref is always + // in-bounds as we've already checked the indices above. + #[allow(unsafe_code)] + unsafe { + (*(entries_ptr.add(i))).ref_mut() + } + }); + + Some(out) + } + /// Get the first key-value pair /// /// Computes in **O(1)** time. diff --git a/src/map/tests.rs b/src/map/tests.rs index b22f995..4fcf558 100644 --- a/src/map/tests.rs +++ b/src/map/tests.rs @@ -923,3 +923,94 @@ move_index_oob!(test_move_index_out_of_bounds_0_10, 0, 10); move_index_oob!(test_move_index_out_of_bounds_0_max, 0, usize::MAX); move_index_oob!(test_move_index_out_of_bounds_10_0, 10, 0); move_index_oob!(test_move_index_out_of_bounds_max_0, usize::MAX, 0); + +#[test] +fn disjoint_mut_empty_map() { + let mut map: RingMap = RingMap::default(); + assert!(map.get_disjoint_mut([&0, &1, &2, &3]).is_none()); +} + +#[test] +fn disjoint_mut_empty_param() { + let mut map: RingMap = RingMap::default(); + map.insert(1, 10); + assert!(map.get_disjoint_mut([] as [&u32; 0]).is_some()); +} + +#[test] +fn disjoint_mut_single_fail() { + let mut map: RingMap = RingMap::default(); + map.insert(1, 10); + assert!(map.get_disjoint_mut([&0]).is_none()); +} + +#[test] +fn disjoint_mut_single_success() { + let mut map: RingMap = RingMap::default(); + map.insert(1, 10); + assert_eq!(map.get_disjoint_mut([&1]), Some([&mut 10])); +} + +#[test] +fn disjoint_mut_multi_success() { + let mut map: RingMap = RingMap::default(); + map.insert(1, 100); + map.insert(2, 200); + map.insert(3, 300); + map.insert(4, 400); + assert_eq!(map.get_disjoint_mut([&1, &2]), Some([&mut 100, &mut 200])); + assert_eq!(map.get_disjoint_mut([&1, &3]), Some([&mut 100, &mut 300])); + assert_eq!( + map.get_disjoint_mut([&3, &1, &4, &2]), + Some([&mut 300, &mut 100, &mut 400, &mut 200]) + ); +} + +#[test] +fn disjoint_mut_multi_success_unsized_key() { + let mut map: RingMap<&'static str, u32> = RingMap::default(); + map.insert("1", 100); + map.insert("2", 200); + map.insert("3", 300); + map.insert("4", 400); + assert_eq!(map.get_disjoint_mut(["1", "2"]), Some([&mut 100, &mut 200])); + assert_eq!(map.get_disjoint_mut(["1", "3"]), Some([&mut 100, &mut 300])); + assert_eq!( + map.get_disjoint_mut(["3", "1", "4", "2"]), + Some([&mut 300, &mut 100, &mut 400, &mut 200]) + ); +} + +#[test] +fn disjoint_mut_multi_fail_missing() { + let mut map: RingMap = RingMap::default(); + map.insert(1, 10); + map.insert(1123, 100); + map.insert(321, 20); + map.insert(1337, 30); + assert_eq!(map.get_disjoint_mut([&121, &1123]), None); + assert_eq!(map.get_disjoint_mut([&1, &1337, &56]), None); + assert_eq!(map.get_disjoint_mut([&1337, &123, &321, &1, &1123]), None); +} + +#[test] +fn disjoint_mut_multi_fail_duplicate() { + let mut map: RingMap = RingMap::default(); + map.insert(1, 10); + map.insert(1123, 100); + map.insert(321, 20); + map.insert(1337, 30); + assert_eq!(map.get_disjoint_mut([&1, &1]), None); + assert_eq!( + map.get_disjoint_mut([&1337, &123, &321, &1337, &1, &1123]), + None + ); +} + +#[test] +fn many_index_mut_fail_oob() { + let mut map: RingMap = RingMap::default(); + map.insert(1, 10); + map.insert(321, 20); + assert_eq!(map.get_disjoint_indices_mut([1, 3]), None); +} From be0a14b962c27b2c364a5bfb3a6053437dd88811 Mon Sep 17 00:00:00 2001 From: Niklas Jonsson Date: Sat, 1 Mar 2025 21:39:06 +0100 Subject: [PATCH 2/6] Address review feedback (cherry picked from commit 4e1d8cef470b4d96380ebbb8bae8994db1d79f51) --- src/lib.rs | 30 +++++++++++ src/map.rs | 61 +++++++--------------- src/map/slice.rs | 47 +++++++++++++++++ src/map/tests.rs | 133 +++++++++++++++++++++++++++++++++++++++-------- 4 files changed, 205 insertions(+), 66 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 71be0b3..14cf513 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -266,3 +266,33 @@ impl core::fmt::Display for TryReserveError { #[cfg(feature = "std")] #[cfg_attr(docsrs, doc(cfg(feature = "std")))] impl std::error::Error for TryReserveError {} + +// NOTE: This is copied from the slice module in the std lib. +/// The error type returned by [`get_disjoint_indices_mut`][`IndexMap::get_disjoint_indices_mut`]. +/// +/// It indicates one of two possible errors: +/// - An index is out-of-bounds. +/// - The same index appeared multiple times in the array +/// (or different but overlapping indices when ranges are provided). +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum GetDisjointMutError { + /// An index provided was out-of-bounds for the slice. + IndexOutOfBounds, + /// Two indices provided were overlapping. + OverlappingIndices, +} + +impl core::fmt::Display for GetDisjointMutError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + let msg = match self { + GetDisjointMutError::IndexOutOfBounds => "an index is out of bounds", + GetDisjointMutError::OverlappingIndices => "there were overlapping indices", + }; + + core::fmt::Display::fmt(msg, f) + } +} + +#[cfg(feature = "std")] +#[cfg_attr(docsrs, doc(cfg(feature = "std")))] +impl std::error::Error for GetDisjointMutError {} diff --git a/src/map.rs b/src/map.rs index 360635c..c50ddaa 100644 --- a/src/map.rs +++ b/src/map.rs @@ -41,7 +41,7 @@ use std::collections::hash_map::RandomState; use self::core::RingMapCore; use crate::util::third; -use crate::{Bucket, Entries, Equivalent, HashValue, TryReserveError}; +use crate::{Bucket, Entries, Equivalent, GetDisjointMutError, HashValue, TryReserveError}; /// A hash table where the iteration order of the key-value pairs is independent /// of the hash values of the keys. @@ -825,35 +825,31 @@ where } } - /// Return the values for `N` keys. If any key is missing a value, or there - /// are duplicate keys, `None` is returned. + /// Return the values for `N` keys. If any key is duplicated, this function will panic. /// /// # Examples /// /// ``` /// let mut map = ringmap::RingMap::from([(1, 'a'), (3, 'b'), (2, 'c')]); - /// assert_eq!(map.get_disjoint_mut([&2, &1]), Some([&mut 'c', &mut 'a'])); + /// assert_eq!(map.get_disjoint_mut([&2, &1]), [Some(&mut 'c'), Some(&mut 'a')]); /// ``` - pub fn get_disjoint_mut(&mut self, keys: [&Q; N]) -> Option<[&mut V; N]> + #[allow(unsafe_code)] + pub fn get_disjoint_mut(&mut self, keys: [&Q; N]) -> [Option<&mut V>; N] where Q: Hash + Equivalent + ?Sized, { - let len = self.len(); let indices = keys.map(|key| self.get_index_of(key)); - - // Handle out-of-bounds indices with panic as this is an internal error in get_index_of. - for idx in indices { - let idx = idx?; - debug_assert!( - idx < len, - "Index is out of range! Got '{}' but length is '{}'", - idx, - len - ); + match self.as_mut_slice().get_disjoint_opt_mut(indices) { + Err(GetDisjointMutError::IndexOutOfBounds) => { + unreachable!( + "Internal error: indices should never be OOB as we got them from get_index_of" + ); + } + Err(GetDisjointMutError::OverlappingIndices) => { + panic!("duplicate keys found"); + } + Ok(key_values) => key_values.map(|kv_opt| kv_opt.map(|kv| kv.1)), } - let indices = indices.map(Option::unwrap); - let entries = self.get_disjoint_indices_mut(indices)?; - Some(entries.map(|(_key, value)| value)) } /// Remove the key-value pair equivalent to `key` and return its value. @@ -1321,38 +1317,17 @@ impl RingMap { /// /// Valid indices are *0 <= index < self.len()* and each index needs to be unique. /// - /// Computes in **O(1)** time. - /// /// # Examples /// /// ``` /// let mut map = ringmap::RingMap::from([(1, 'a'), (3, 'b'), (2, 'c')]); - /// assert_eq!(map.get_disjoint_indices_mut([2, 0]), Some([(&2, &mut 'c'), (&1, &mut 'a')])); + /// assert_eq!(map.get_disjoint_indices_mut([2, 0]), Ok([(&2, &mut 'c'), (&1, &mut 'a')])); /// ``` pub fn get_disjoint_indices_mut( &mut self, indices: [usize; N], - ) -> Option<[(&K, &mut V); N]> { - // SAFETY: Can't allow duplicate indices as we would return several mutable refs to the same data. - let len = self.len(); - for i in 0..N { - let idx = indices[i]; - if idx >= len || indices[i + 1..N].contains(&idx) { - return None; - } - } - - let entries_ptr = self.as_entries_mut().as_mut_ptr(); - let out = indices.map(|i| { - // SAFETY: The base pointer is valid as it comes from a slice and the deref is always - // in-bounds as we've already checked the indices above. - #[allow(unsafe_code)] - unsafe { - (*(entries_ptr.add(i))).ref_mut() - } - }); - - Some(out) + ) -> Result<[(&K, &mut V); N], GetDisjointMutError> { + self.as_mut_slice().get_disjoint_mut(indices) } /// Get the first key-value pair diff --git a/src/map/slice.rs b/src/map/slice.rs index 488c559..949bb50 100644 --- a/src/map/slice.rs +++ b/src/map/slice.rs @@ -1,5 +1,6 @@ use super::{Bucket, IntoIter, IntoKeys, IntoValues, Iter, IterMut, Keys, Values, ValuesMut}; use crate::util::{slice_eq, try_simplify_range}; +use crate::GetDisjointMutError; use alloc::boxed::Box; use alloc::collections::VecDeque; @@ -264,6 +265,52 @@ impl Slice { self.entries .partition_point(move |a| pred(&a.key, &a.value)) } + + /// Get an array of `N` key-value pairs by `N` indices + /// + /// Valid indices are *0 <= index < self.len()* and each index needs to be unique. + pub fn get_disjoint_mut( + &mut self, + indices: [usize; N], + ) -> Result<[(&K, &mut V); N], GetDisjointMutError> { + let indices = indices.map(Some); + let key_values = self.get_disjoint_opt_mut(indices)?; + Ok(key_values.map(Option::unwrap)) + } + + #[allow(unsafe_code)] + pub(crate) fn get_disjoint_opt_mut( + &mut self, + indices: [Option; N], + ) -> Result<[Option<(&K, &mut V)>; N], GetDisjointMutError> { + // SAFETY: Can't allow duplicate indices as we would return several mutable refs to the same data. + let len = self.len(); + for i in 0..N { + let Some(idx) = indices[i] else { + continue; + }; + if idx >= len { + return Err(GetDisjointMutError::IndexOutOfBounds); + } else if indices[i + 1..N].contains(&Some(idx)) { + return Err(GetDisjointMutError::OverlappingIndices); + } + } + + let entries_ptr = self.entries.as_mut_ptr(); + let out = indices.map(|idx_opt| { + match idx_opt { + Some(idx) => { + // SAFETY: The base pointer is valid as it comes from a slice and the reference is always + // in-bounds & unique as we've already checked the indices above. + let kv = unsafe { (*(entries_ptr.add(idx))).ref_mut() }; + Some(kv) + } + None => None, + } + }); + + Ok(out) + } } impl<'a, K, V> IntoIterator for &'a Slice { diff --git a/src/map/tests.rs b/src/map/tests.rs index 4fcf558..5fe4e5b 100644 --- a/src/map/tests.rs +++ b/src/map/tests.rs @@ -927,28 +927,31 @@ move_index_oob!(test_move_index_out_of_bounds_max_0, usize::MAX, 0); #[test] fn disjoint_mut_empty_map() { let mut map: RingMap = RingMap::default(); - assert!(map.get_disjoint_mut([&0, &1, &2, &3]).is_none()); + assert_eq!( + map.get_disjoint_mut([&0, &1, &2, &3]), + [None, None, None, None] + ); } #[test] fn disjoint_mut_empty_param() { let mut map: RingMap = RingMap::default(); map.insert(1, 10); - assert!(map.get_disjoint_mut([] as [&u32; 0]).is_some()); + assert_eq!(map.get_disjoint_mut([] as [&u32; 0]), []); } #[test] fn disjoint_mut_single_fail() { let mut map: RingMap = RingMap::default(); map.insert(1, 10); - assert!(map.get_disjoint_mut([&0]).is_none()); + assert_eq!(map.get_disjoint_mut([&0]), [None]); } #[test] fn disjoint_mut_single_success() { let mut map: RingMap = RingMap::default(); map.insert(1, 10); - assert_eq!(map.get_disjoint_mut([&1]), Some([&mut 10])); + assert_eq!(map.get_disjoint_mut([&1]), [Some(&mut 10)]); } #[test] @@ -958,11 +961,22 @@ fn disjoint_mut_multi_success() { map.insert(2, 200); map.insert(3, 300); map.insert(4, 400); - assert_eq!(map.get_disjoint_mut([&1, &2]), Some([&mut 100, &mut 200])); - assert_eq!(map.get_disjoint_mut([&1, &3]), Some([&mut 100, &mut 300])); + assert_eq!( + map.get_disjoint_mut([&1, &2]), + [Some(&mut 100), Some(&mut 200)] + ); + assert_eq!( + map.get_disjoint_mut([&1, &3]), + [Some(&mut 100), Some(&mut 300)] + ); assert_eq!( map.get_disjoint_mut([&3, &1, &4, &2]), - Some([&mut 300, &mut 100, &mut 400, &mut 200]) + [ + Some(&mut 300), + Some(&mut 100), + Some(&mut 400), + Some(&mut 200) + ] ); } @@ -973,44 +987,117 @@ fn disjoint_mut_multi_success_unsized_key() { map.insert("2", 200); map.insert("3", 300); map.insert("4", 400); - assert_eq!(map.get_disjoint_mut(["1", "2"]), Some([&mut 100, &mut 200])); - assert_eq!(map.get_disjoint_mut(["1", "3"]), Some([&mut 100, &mut 300])); + + assert_eq!( + map.get_disjoint_mut(["1", "2"]), + [Some(&mut 100), Some(&mut 200)] + ); + assert_eq!( + map.get_disjoint_mut(["1", "3"]), + [Some(&mut 100), Some(&mut 300)] + ); assert_eq!( map.get_disjoint_mut(["3", "1", "4", "2"]), - Some([&mut 300, &mut 100, &mut 400, &mut 200]) + [ + Some(&mut 300), + Some(&mut 100), + Some(&mut 400), + Some(&mut 200) + ] + ); +} + +#[test] +fn disjoint_mut_multi_success_borrow_key() { + let mut map: RingMap = RingMap::default(); + map.insert("1".into(), 100); + map.insert("2".into(), 200); + map.insert("3".into(), 300); + map.insert("4".into(), 400); + + assert_eq!( + map.get_disjoint_mut(["1", "2"]), + [Some(&mut 100), Some(&mut 200)] + ); + assert_eq!( + map.get_disjoint_mut(["1", "3"]), + [Some(&mut 100), Some(&mut 300)] + ); + assert_eq!( + map.get_disjoint_mut(["3", "1", "4", "2"]), + [ + Some(&mut 300), + Some(&mut 100), + Some(&mut 400), + Some(&mut 200) + ] ); } #[test] fn disjoint_mut_multi_fail_missing() { + let mut map: RingMap = RingMap::default(); + map.insert(1, 100); + map.insert(2, 200); + map.insert(3, 300); + map.insert(4, 400); + + assert_eq!(map.get_disjoint_mut([&1, &5]), [Some(&mut 100), None]); + assert_eq!(map.get_disjoint_mut([&5, &6]), [None, None]); + assert_eq!( + map.get_disjoint_mut([&1, &5, &4]), + [Some(&mut 100), None, Some(&mut 400)] + ); +} + +#[test] +#[should_panic] +fn disjoint_mut_multi_fail_duplicate_panic() { + let mut map: RingMap = RingMap::default(); + map.insert(1, 100); + map.get_disjoint_mut([&1, &2, &1]); +} + +#[test] +fn disjoint_indices_mut_fail_oob() { + let mut map: RingMap = RingMap::default(); + map.insert(1, 10); + map.insert(321, 20); + assert_eq!( + map.get_disjoint_indices_mut([1, 3]), + Err(crate::GetDisjointMutError::IndexOutOfBounds) + ); +} + +#[test] +fn disjoint_indices_mut_empty() { let mut map: RingMap = RingMap::default(); map.insert(1, 10); - map.insert(1123, 100); map.insert(321, 20); - map.insert(1337, 30); - assert_eq!(map.get_disjoint_mut([&121, &1123]), None); - assert_eq!(map.get_disjoint_mut([&1, &1337, &56]), None); - assert_eq!(map.get_disjoint_mut([&1337, &123, &321, &1, &1123]), None); + assert_eq!(map.get_disjoint_indices_mut([]), Ok([])); } #[test] -fn disjoint_mut_multi_fail_duplicate() { +fn disjoint_indices_mut_success() { let mut map: RingMap = RingMap::default(); map.insert(1, 10); - map.insert(1123, 100); map.insert(321, 20); - map.insert(1337, 30); - assert_eq!(map.get_disjoint_mut([&1, &1]), None); + assert_eq!(map.get_disjoint_indices_mut([0]), Ok([(&1, &mut 10)])); + + assert_eq!(map.get_disjoint_indices_mut([1]), Ok([(&321, &mut 20)])); assert_eq!( - map.get_disjoint_mut([&1337, &123, &321, &1337, &1, &1123]), - None + map.get_disjoint_indices_mut([0, 1]), + Ok([(&1, &mut 10), (&321, &mut 20)]) ); } #[test] -fn many_index_mut_fail_oob() { +fn disjoint_indices_mut_fail_duplicate() { let mut map: RingMap = RingMap::default(); map.insert(1, 10); map.insert(321, 20); - assert_eq!(map.get_disjoint_indices_mut([1, 3]), None); + assert_eq!( + map.get_disjoint_indices_mut([1, 2, 1]), + Err(crate::GetDisjointMutError::OverlappingIndices) + ); } From b0abe311b1a5533725dc5961c3e3a5b854c40f0c Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 4 Apr 2025 14:42:19 -0700 Subject: [PATCH 3/6] Implement additional suggestions from review (cherry picked from commit 5be552d557765a8ccc919185838067b3c77eab95) --- src/lib.rs | 6 +++--- src/map.rs | 3 +-- src/map/slice.rs | 2 +- src/map/tests.rs | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 14cf513..3bf005a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -268,12 +268,12 @@ impl core::fmt::Display for TryReserveError { impl std::error::Error for TryReserveError {} // NOTE: This is copied from the slice module in the std lib. -/// The error type returned by [`get_disjoint_indices_mut`][`IndexMap::get_disjoint_indices_mut`]. +/// The error type returned by [`get_disjoint_indices_mut`][`RingMap::get_disjoint_indices_mut`]. /// /// It indicates one of two possible errors: /// - An index is out-of-bounds. -/// - The same index appeared multiple times in the array -/// (or different but overlapping indices when ranges are provided). +/// - The same index appeared multiple times in the array. +// (or different but overlapping indices when ranges are provided) #[derive(Debug, Clone, PartialEq, Eq)] pub enum GetDisjointMutError { /// An index provided was out-of-bounds for the slice. diff --git a/src/map.rs b/src/map.rs index c50ddaa..7a812e0 100644 --- a/src/map.rs +++ b/src/map.rs @@ -833,10 +833,9 @@ where /// let mut map = ringmap::RingMap::from([(1, 'a'), (3, 'b'), (2, 'c')]); /// assert_eq!(map.get_disjoint_mut([&2, &1]), [Some(&mut 'c'), Some(&mut 'a')]); /// ``` - #[allow(unsafe_code)] pub fn get_disjoint_mut(&mut self, keys: [&Q; N]) -> [Option<&mut V>; N] where - Q: Hash + Equivalent + ?Sized, + Q: ?Sized + Hash + Equivalent, { let indices = keys.map(|key| self.get_index_of(key)); match self.as_mut_slice().get_disjoint_opt_mut(indices) { diff --git a/src/map/slice.rs b/src/map/slice.rs index 949bb50..68017a7 100644 --- a/src/map/slice.rs +++ b/src/map/slice.rs @@ -291,7 +291,7 @@ impl Slice { }; if idx >= len { return Err(GetDisjointMutError::IndexOutOfBounds); - } else if indices[i + 1..N].contains(&Some(idx)) { + } else if indices[..i].contains(&Some(idx)) { return Err(GetDisjointMutError::OverlappingIndices); } } diff --git a/src/map/tests.rs b/src/map/tests.rs index 5fe4e5b..956c407 100644 --- a/src/map/tests.rs +++ b/src/map/tests.rs @@ -1097,7 +1097,7 @@ fn disjoint_indices_mut_fail_duplicate() { map.insert(1, 10); map.insert(321, 20); assert_eq!( - map.get_disjoint_indices_mut([1, 2, 1]), + map.get_disjoint_indices_mut([1, 0, 1]), Err(crate::GetDisjointMutError::OverlappingIndices) ); } From a8fc73e36269f06ff9b7742a8611eeac0c350e88 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 4 Apr 2025 14:52:20 -0700 Subject: [PATCH 4/6] Avoid let-else for MSRV's sake (cherry picked from commit 434d7ac6d122cf27dce979eb3888e362853dfb2d) --- src/map/slice.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/map/slice.rs b/src/map/slice.rs index 68017a7..6026d42 100644 --- a/src/map/slice.rs +++ b/src/map/slice.rs @@ -286,13 +286,12 @@ impl Slice { // SAFETY: Can't allow duplicate indices as we would return several mutable refs to the same data. let len = self.len(); for i in 0..N { - let Some(idx) = indices[i] else { - continue; - }; - if idx >= len { - return Err(GetDisjointMutError::IndexOutOfBounds); - } else if indices[..i].contains(&Some(idx)) { - return Err(GetDisjointMutError::OverlappingIndices); + if let Some(idx) = indices[i] { + if idx >= len { + return Err(GetDisjointMutError::IndexOutOfBounds); + } else if indices[..i].contains(&Some(idx)) { + return Err(GetDisjointMutError::OverlappingIndices); + } } } From 293b85f1a02f3a6620a33b7436e7f3a0c0c1378d Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 4 Apr 2025 19:20:33 -0700 Subject: [PATCH 5/6] Port `get_disjoint_mut` to ringmap --- src/map.rs | 8 ++++++-- src/map/slice.rs | 28 +++++++++++++++++++--------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/map.rs b/src/map.rs index 7a812e0..651efb9 100644 --- a/src/map.rs +++ b/src/map.rs @@ -838,7 +838,8 @@ where Q: ?Sized + Hash + Equivalent, { let indices = keys.map(|key| self.get_index_of(key)); - match self.as_mut_slice().get_disjoint_opt_mut(indices) { + let (head, tail) = self.as_mut_slices(); + match Slice::get_disjoint_opt_mut(head, tail, indices) { Err(GetDisjointMutError::IndexOutOfBounds) => { unreachable!( "Internal error: indices should never be OOB as we got them from get_index_of" @@ -1326,7 +1327,10 @@ impl RingMap { &mut self, indices: [usize; N], ) -> Result<[(&K, &mut V); N], GetDisjointMutError> { - self.as_mut_slice().get_disjoint_mut(indices) + let indices = indices.map(Some); + let (head, tail) = self.as_mut_slices(); + let key_values = Slice::get_disjoint_opt_mut(head, tail, indices)?; + Ok(key_values.map(Option::unwrap)) } /// Get the first key-value pair diff --git a/src/map/slice.rs b/src/map/slice.rs index 6026d42..1654ce2 100644 --- a/src/map/slice.rs +++ b/src/map/slice.rs @@ -274,17 +274,21 @@ impl Slice { indices: [usize; N], ) -> Result<[(&K, &mut V); N], GetDisjointMutError> { let indices = indices.map(Some); - let key_values = self.get_disjoint_opt_mut(indices)?; + let empty_tail = Self::new_mut(); + let key_values = Self::get_disjoint_opt_mut(self, empty_tail, indices)?; Ok(key_values.map(Option::unwrap)) } #[allow(unsafe_code)] - pub(crate) fn get_disjoint_opt_mut( - &mut self, + pub(crate) fn get_disjoint_opt_mut<'a, const N: usize>( + head: &mut Self, + tail: &mut Self, indices: [Option; N], - ) -> Result<[Option<(&K, &mut V)>; N], GetDisjointMutError> { + ) -> Result<[Option<(&'a K, &'a mut V)>; N], GetDisjointMutError> { + let mid = head.len(); + let len = mid + tail.len(); + // SAFETY: Can't allow duplicate indices as we would return several mutable refs to the same data. - let len = self.len(); for i in 0..N { if let Some(idx) = indices[i] { if idx >= len { @@ -295,14 +299,20 @@ impl Slice { } } - let entries_ptr = self.entries.as_mut_ptr(); + let head_ptr = head.entries.as_mut_ptr(); + let tail_ptr = tail.entries.as_mut_ptr(); let out = indices.map(|idx_opt| { match idx_opt { Some(idx) => { - // SAFETY: The base pointer is valid as it comes from a slice and the reference is always + // SAFETY: The base pointers are valid as they come from slices and the reference is always // in-bounds & unique as we've already checked the indices above. - let kv = unsafe { (*(entries_ptr.add(idx))).ref_mut() }; - Some(kv) + unsafe { + let ptr = match idx.checked_sub(mid) { + None => head_ptr.add(idx), + Some(tidx) => tail_ptr.add(tidx), + }; + Some((*ptr).ref_mut()) + } } None => None, } From 3d1d290dde00396a2e49ad7ab0d8d1ed02b4b3fc Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 4 Apr 2025 19:39:53 -0700 Subject: [PATCH 6/6] Test get_disjoint_mut with both head and tail slices --- src/map/tests.rs | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/map/tests.rs b/src/map/tests.rs index 956c407..352bedc 100644 --- a/src/map/tests.rs +++ b/src/map/tests.rs @@ -980,6 +980,36 @@ fn disjoint_mut_multi_success() { ); } +#[test] +fn disjoint_mut_multi_success_double_ended() { + let mut map: RingMap = RingMap::default(); + map.push_back(2, 200); + map.push_back(3, 300); + map.push_back(4, 400); + map.push_front(1, 100); + { + let (head, tail) = map.as_mut_slices(); + assert!(!head.is_empty() && !tail.is_empty()); + } + assert_eq!( + map.get_disjoint_mut([&1, &2]), + [Some(&mut 100), Some(&mut 200)] + ); + assert_eq!( + map.get_disjoint_mut([&1, &3]), + [Some(&mut 100), Some(&mut 300)] + ); + assert_eq!( + map.get_disjoint_mut([&3, &1, &4, &2]), + [ + Some(&mut 300), + Some(&mut 100), + Some(&mut 400), + Some(&mut 200) + ] + ); +} + #[test] fn disjoint_mut_multi_success_unsized_key() { let mut map: RingMap<&'static str, u32> = RingMap::default(); @@ -1091,6 +1121,23 @@ fn disjoint_indices_mut_success() { ); } +#[test] +fn disjoint_indices_mut_success_double_ended() { + let mut map: RingMap = RingMap::default(); + map.push_back(1, 10); + map.push_back(321, 20); + map.push_front(42, 0o42); + { + let (head, tail) = map.as_mut_slices(); + assert!(!head.is_empty() && !tail.is_empty()); + } + assert_eq!(map.get_disjoint_indices_mut([1]), Ok([(&1, &mut 10)])); + assert_eq!( + map.get_disjoint_indices_mut([0, 2]), + Ok([(&42, &mut 0o42), (&321, &mut 20)]) + ); +} + #[test] fn disjoint_indices_mut_fail_duplicate() { let mut map: RingMap = RingMap::default();