From 84e9c6a95089c648b8227e17c87acbd8c25eb5fb Mon Sep 17 00:00:00 2001 From: Mark Tyrkba Date: Wed, 30 Apr 2025 17:59:57 +0300 Subject: [PATCH 1/9] Add `#[track_caller]` to all functions that are marked with `***Panics***` and can potentially panic (cherry picked from commit b160489b2ea6a0f4c82d07f05a7239fc6f23d730) --- src/map/entry.rs | 3 +++ src/map/raw_entry_v1.rs | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/src/map/entry.rs b/src/map/entry.rs index e2c62d2..7bf1533 100644 --- a/src/map/entry.rs +++ b/src/map/entry.rs @@ -250,6 +250,7 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> { /// ***Panics*** if the `other` index is out of bounds. /// /// Computes in **O(1)** time (average). + #[track_caller] pub fn swap_indices(self, other: usize) { self.inner.swap_indices(other); } @@ -331,6 +332,7 @@ impl<'a, K, V> VacantEntry<'a, K, V> { /// ***Panics*** if `index` is out of bounds. /// /// Computes in **O(n)** time (average). + #[track_caller] pub fn shift_insert(self, index: usize, value: V) -> &'a mut V { self.inner.shift_insert(index, value) } @@ -462,6 +464,7 @@ impl<'a, K, V> IndexedEntry<'a, K, V> { /// ***Panics*** if the `other` index is out of bounds. /// /// Computes in **O(1)** time (average). + #[track_caller] pub fn swap_indices(self, other: usize) { self.inner.swap_indices(other) } diff --git a/src/map/raw_entry_v1.rs b/src/map/raw_entry_v1.rs index 30db048..d67e4d9 100644 --- a/src/map/raw_entry_v1.rs +++ b/src/map/raw_entry_v1.rs @@ -520,6 +520,7 @@ impl<'a, K, V, S> RawOccupiedEntryMut<'a, K, V, S> { /// ***Panics*** if `to` is out of bounds. /// /// Computes in **O(n)** time (average). + #[track_caller] pub fn move_index(self, to: usize) { self.inner.move_index(to); } @@ -532,6 +533,7 @@ impl<'a, K, V, S> RawOccupiedEntryMut<'a, K, V, S> { /// ***Panics*** if the `other` index is out of bounds. /// /// Computes in **O(1)** time (average). + #[track_caller] pub fn swap_indices(self, other: usize) { self.inner.swap_indices(other); } @@ -577,6 +579,7 @@ impl<'a, K, V, S> RawVacantEntryMut<'a, K, V, S> { /// ***Panics*** if `index` is out of bounds. /// /// Computes in **O(n)** time (average). + #[track_caller] pub fn shift_insert(self, index: usize, key: K, value: V) -> (&'a mut K, &'a mut V) where K: Hash, @@ -591,6 +594,7 @@ impl<'a, K, V, S> RawVacantEntryMut<'a, K, V, S> { /// ***Panics*** if `index` is out of bounds. /// /// Computes in **O(n)** time (average). + #[track_caller] pub fn shift_insert_hashed_nocheck( self, index: usize, From 41c699b45b6a006d69a9794d0e7ac294c07b903c Mon Sep 17 00:00:00 2001 From: Mark Tyrkba Date: Wed, 30 Apr 2025 18:05:43 +0300 Subject: [PATCH 2/9] Manage `.unwrap_or_else` closures properly to report user's call location in case of panic (cherry picked from commit 15c10c38911e06cba01b962839e5041add887603) --- src/map.rs | 26 +++++++++++++------------- src/set.rs | 6 ++++-- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/map.rs b/src/map.rs index 21c61d2..49d97a0 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1275,14 +1275,14 @@ impl Index for OrderMap { /// /// ***Panics*** if `index` is out of bounds. fn index(&self, index: usize) -> &V { - self.get_index(index) - .unwrap_or_else(|| { - panic!( - "index out of bounds: the len is {len} but the index is {index}", - len = self.len() - ); - }) - .1 + if let Some((_, value)) = self.get_index(index) { + value + } else { + panic!( + "index out of bounds: the len is {len} but the index is {index}", + len = self.len() + ); + } } } @@ -1322,11 +1322,11 @@ impl IndexMut for OrderMap { fn index_mut(&mut self, index: usize) -> &mut V { let len: usize = self.len(); - self.get_index_mut(index) - .unwrap_or_else(|| { - panic!("index out of bounds: the len is {len} but the index is {index}"); - }) - .1 + if let Some((_, value)) = self.get_index_mut(index) { + value + } else { + panic!("index out of bounds: the len is {len} but the index is {index}"); + } } } diff --git a/src/set.rs b/src/set.rs index 471a4b4..640b595 100644 --- a/src/set.rs +++ b/src/set.rs @@ -1039,12 +1039,14 @@ impl Index for OrderSet { /// /// ***Panics*** if `index` is out of bounds. fn index(&self, index: usize) -> &T { - self.get_index(index).unwrap_or_else(|| { + if let Some(value) = self.get_index(index) { + value + } else { panic!( "index out of bounds: the len is {len} but the index is {index}", len = self.len() ); - }) + } } } From d60bc534efb806cc2ba538678e331dea76359ee4 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 11 Jun 2025 22:45:36 +0000 Subject: [PATCH 3/9] Merge pull request indexmap#392 from AbeZbm/add-tests Add some missing tests Co-authored-by: AbeZbm <128758303@qq.com> (cherry picked from commit b49e95c6652d65ec68a588f79330bbdf12f9f88b) --- src/map/slice.rs | 119 ++++++++++++++++++++ src/map/tests.rs | 220 +++++++++++++++++++++++++++++++++++++ src/set/tests.rs | 276 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 615 insertions(+) diff --git a/src/map/slice.rs b/src/map/slice.rs index 2f25f33..db669ed 100644 --- a/src/map/slice.rs +++ b/src/map/slice.rs @@ -146,4 +146,123 @@ mod tests { } } } + + #[test] + fn slice_new() { + let slice: &Slice = Slice::new(); + assert!(slice.is_empty()); + assert_eq!(slice.len(), 0); + } + + #[test] + fn slice_new_mut() { + let slice: &mut Slice = Slice::new_mut(); + assert!(slice.is_empty()); + assert_eq!(slice.len(), 0); + } + + #[test] + fn slice_get_index_mut() { + let mut map: OrderMap = (0..10).map(|i| (i, i * i)).collect(); + let slice: &mut Slice = map.as_mut_slice(); + + { + let (key, value) = slice.get_index_mut(0).unwrap(); + assert_eq!(*key, 0); + assert_eq!(*value, 0); + + *value = 11; + } + + assert_eq!(slice[0], 11); + + { + let result = slice.get_index_mut(11); + assert!(result.is_none()); + } + } + + #[test] + fn slice_split_first() { + let slice: &mut Slice = Slice::new_mut(); + let result = slice.split_first(); + assert!(result.is_none()); + + let mut map: OrderMap = (0..10).map(|i| (i, i * i)).collect(); + let slice: &mut Slice = map.as_mut_slice(); + + { + let (first, rest) = slice.split_first().unwrap(); + assert_eq!(first, (&0, &0)); + assert_eq!(rest.len(), 9); + } + assert_eq!(slice.len(), 10); + } + + #[test] + fn slice_split_first_mut() { + let slice: &mut Slice = Slice::new_mut(); + let result = slice.split_first_mut(); + assert!(result.is_none()); + + let mut map: OrderMap = (0..10).map(|i| (i, i * i)).collect(); + let slice: &mut Slice = map.as_mut_slice(); + + { + let (first, rest) = slice.split_first_mut().unwrap(); + assert_eq!(first, (&0, &mut 0)); + assert_eq!(rest.len(), 9); + + *first.1 = 11; + } + assert_eq!(slice.len(), 10); + assert_eq!(slice[0], 11); + } + + #[test] + fn slice_split_last() { + let slice: &mut Slice = Slice::new_mut(); + let result = slice.split_last(); + assert!(result.is_none()); + + let mut map: OrderMap = (0..10).map(|i| (i, i * i)).collect(); + let slice: &mut Slice = map.as_mut_slice(); + + { + let (last, rest) = slice.split_last().unwrap(); + assert_eq!(last, (&9, &81)); + assert_eq!(rest.len(), 9); + } + assert_eq!(slice.len(), 10); + } + + #[test] + fn slice_split_last_mut() { + let slice: &mut Slice = Slice::new_mut(); + let result = slice.split_last_mut(); + assert!(result.is_none()); + + let mut map: OrderMap = (0..10).map(|i| (i, i * i)).collect(); + let slice: &mut Slice = map.as_mut_slice(); + + { + let (last, rest) = slice.split_last_mut().unwrap(); + assert_eq!(last, (&9, &mut 81)); + assert_eq!(rest.len(), 9); + + *last.1 = 100; + } + + assert_eq!(slice.len(), 10); + assert_eq!(slice[slice.len() - 1], 100); + } + + #[test] + fn slice_get_range() { + let mut map: OrderMap = (0..10).map(|i| (i, i * i)).collect(); + let slice: &mut Slice = map.as_mut_slice(); + let subslice = slice.get_range(3..6).unwrap(); + assert_eq!(subslice.len(), 3); + assert_eq!(subslice, &[(3, 9), (4, 16), (5, 25)]); + } } diff --git a/src/map/tests.rs b/src/map/tests.rs index 08c1c89..a1899c8 100644 --- a/src/map/tests.rs +++ b/src/map/tests.rs @@ -590,6 +590,226 @@ fn iter_default() { assert_default::>(); } +#[test] +fn get_index_mut2() { + let mut map: OrderMap = OrderMap::new(); + map.insert(1, 2); + map.insert(3, 4); + map.insert(5, 6); + + { + let (key, value) = map.get_index_mut2(0).unwrap(); + assert_eq!(*key, 1); + assert_eq!(*value, 2); + + *value = 7; + } + assert_eq!(map[0], 7); + + { + let (key, _) = map.get_index_mut2(0).unwrap(); + *key = 8; + } + assert_eq!(map.get_index(0).unwrap().0, &8); +} + +#[test] +fn shift_shift_remove_index() { + let mut map: OrderMap = OrderMap::new(); + map.insert(1, 2); + map.insert(3, 4); + map.insert(5, 6); + map.insert(7, 8); + map.insert(9, 10); + + let result = map.remove_index(1); + assert_eq!(result, Some((3, 4))); + assert_eq!(map.len(), 4); + assert_eq!(map.as_slice(), &[(1, 2), (5, 6), (7, 8), (9, 10)]); + + let result = map.remove_index(1); + assert_eq!(result, Some((5, 6))); + assert_eq!(map.len(), 3); + assert_eq!(map.as_slice(), &[(1, 2), (7, 8), (9, 10)]); + + let result = map.remove_index(2); + assert_eq!(result, Some((9, 10))); + assert_eq!(map.len(), 2); + assert_eq!(map.as_slice(), &[(1, 2), (7, 8)]); + + let result = map.remove_index(2); + assert_eq!(result, None); + assert_eq!(map.len(), 2); + assert_eq!(map.as_slice(), &[(1, 2), (7, 8)]); +} + +#[test] +fn shift_remove_entry() { + let mut map: OrderMap = OrderMap::new(); + map.insert(1, 2); + map.insert(3, 4); + map.insert(5, 6); + map.insert(7, 8); + map.insert(9, 10); + + let result = map.remove_entry(&3); + assert_eq!(result, Some((3, 4))); + assert_eq!(map.len(), 4); + assert_eq!(map.as_slice(), &[(1, 2), (5, 6), (7, 8), (9, 10)]); + + let result = map.remove_entry(&9); + assert_eq!(result, Some((9, 10))); + assert_eq!(map.len(), 3); + assert_eq!(map.as_slice(), &[(1, 2), (5, 6), (7, 8)]); + + let result = map.remove_entry(&9); + assert_eq!(result, None); + assert_eq!(map.len(), 3); + assert_eq!(map.as_slice(), &[(1, 2), (5, 6), (7, 8)]); +} + +#[test] +fn shift_remove_full() { + let mut map: OrderMap = OrderMap::new(); + map.insert(1, 2); + map.insert(3, 4); + map.insert(5, 6); + map.insert(7, 8); + map.insert(9, 10); + + let result = map.remove_full(&3); + assert_eq!(result, Some((1, 3, 4))); + assert_eq!(map.len(), 4); + assert_eq!(map.as_slice(), &[(1, 2), (5, 6), (7, 8), (9, 10)]); + + let result = map.remove_full(&9); + assert_eq!(result, Some((3, 9, 10))); + assert_eq!(map.len(), 3); + assert_eq!(map.as_slice(), &[(1, 2), (5, 6), (7, 8)]); + + let result = map.remove_full(&9); + assert_eq!(result, None); + assert_eq!(map.len(), 3); + assert_eq!(map.as_slice(), &[(1, 2), (5, 6), (7, 8)]); +} + +#[test] +fn sorted_unstable_by() { + let mut map: OrderMap = OrderMap::new(); + map.extend(vec![(1, 10), (2, 20), (3, 30), (4, 40), (5, 50)]); + let sorted = map.sorted_unstable_by(|_a, b, _c, d| d.cmp(&b)); + + assert_eq!( + sorted.as_slice(), + &[(5, 50), (4, 40), (3, 30), (2, 20), (1, 10)] + ); +} + +#[test] +fn into_boxed_slice() { + let mut map: OrderMap = OrderMap::new(); + for i in 0..5 { + map.insert(i, i * 10); + } + let boxed_slice: Box> = map.into_boxed_slice(); + assert_eq!(boxed_slice.len(), 5); + assert_eq!( + boxed_slice.as_ref(), + &[(0, 0), (1, 10), (2, 20), (3, 30), (4, 40)] + ); +} + +#[test] +fn last_mut() { + let mut map: OrderMap<&str, i32> = OrderMap::new(); + + let last_entry = map.last_mut(); + assert_eq!(last_entry, None); + + map.insert("key1", 1); + map.insert("key2", 2); + map.insert("key3", 3); + let last_entry = map.last_mut(); + assert_eq!(last_entry, Some((&"key3", &mut 3))); + + *last_entry.unwrap().1 = 4; + assert_eq!(map.get("key3"), Some(&4)); +} + +#[test] +#[should_panic = "index out of bounds"] +fn insert_before_oob() { + let mut map: OrderMap = OrderMap::new(); + let _ = map.insert_before(0, 'a', ()); + let _ = map.insert_before(1, 'b', ()); + map.insert_before(3, 'd', ()); +} + +#[test] +fn clear() { + let mut map: OrderMap = OrderMap::new(); + map.extend(vec![(1, 10), (2, 20), (3, 30), (4, 40), (5, 50)]); + map.clear(); + assert_eq!(map.len(), 0); +} + +#[test] +fn get_range() { + let mut index_map: OrderMap = OrderMap::new(); + index_map.insert(1, 10); + index_map.insert(2, 20); + index_map.insert(3, 30); + index_map.insert(4, 40); + index_map.insert(5, 50); + + let result = index_map.get_range(2..2); + assert!(result.unwrap().is_empty()); + + let result = index_map.get_range(4..2); + assert!(result.is_none()); + + let result = index_map.get_range(2..4); + let slice: &Slice = result.unwrap(); + assert_eq!(slice.len(), 2); + assert_eq!(slice, &[(3, 30), (4, 40)]); +} + +#[test] +fn get_range_mut() { + let mut index_map: OrderMap = OrderMap::new(); + index_map.insert(1, 10); + index_map.insert(2, 20); + index_map.insert(3, 30); + index_map.insert(4, 40); + index_map.insert(5, 50); + + let result = index_map.get_range_mut(2..2); + assert!(result.unwrap().is_empty()); + + let result = index_map.get_range_mut(4..2); + assert!(result.is_none()); + + let result = index_map.get_range_mut(2..4); + let slice: &mut Slice = result.unwrap(); + assert_eq!(slice.len(), 2); + assert_eq!(slice, &mut [(3, 30), (4, 40)]); + + for i in 0..slice.len() { + slice[i] += 1; + } + assert_eq!(slice, &mut [(3, 31), (4, 41)]); +} + +#[test] +#[should_panic = "index out of bounds"] +fn shift_insert_oob() { + let mut map: OrderMap = OrderMap::new(); + map.shift_insert(0, 1, 10); + map.shift_insert(1, 2, 20); + map.shift_insert(2, 3, 30); + map.shift_insert(5, 4, 40); +} + #[test] fn test_binary_search_by() { // adapted from std's test for binary_search diff --git a/src/set/tests.rs b/src/set/tests.rs index 37eacad..855058f 100644 --- a/src/set/tests.rs +++ b/src/set/tests.rs @@ -590,6 +590,282 @@ fn iter_default() { assert_default::>(); } +#[test] +#[allow(deprecated)] +fn take() { + let mut index_set: OrderSet = OrderSet::new(); + index_set.insert(10); + assert_eq!(index_set.len(), 1); + + let result = index_set.take(&10); + assert_eq!(result, Some(10)); + assert_eq!(index_set.len(), 0); + + let result = index_set.take(&20); + assert_eq!(result, None); +} + +#[test] +fn swap_take() { + let mut index_set: OrderSet = OrderSet::new(); + index_set.insert(10); + index_set.insert(20); + index_set.insert(30); + index_set.insert(40); + assert_eq!(index_set.len(), 4); + + let result = index_set.swap_take(&20); + assert_eq!(result, Some(20)); + assert_eq!(index_set.len(), 3); + assert_eq!(index_set.as_slice(), &[10, 40, 30]); + + let result = index_set.swap_take(&50); + assert_eq!(result, None); +} + +#[test] +fn sort_unstable() { + let mut index_set: OrderSet = OrderSet::new(); + index_set.insert(30); + index_set.insert(20); + index_set.insert(10); + + index_set.sort_unstable(); + assert_eq!(index_set.as_slice(), &[10, 20, 30]); +} + +#[test] +fn try_reserve_exact() { + let mut index_set: OrderSet = OrderSet::new(); + index_set.insert(10); + index_set.insert(20); + index_set.insert(30); + index_set.shrink_to_fit(); + assert_eq!(index_set.capacity(), 3); + + index_set.try_reserve_exact(2).unwrap(); + assert_eq!(index_set.capacity(), 5); +} + +#[test] +fn shift_remove_full() { + let mut set: OrderSet = OrderSet::new(); + set.insert(10); + set.insert(20); + set.insert(30); + set.insert(40); + set.insert(50); + + let result = set.remove_full(&20); + assert_eq!(result, Some((1, 20))); + assert_eq!(set.len(), 4); + assert_eq!(set.as_slice(), &[10, 30, 40, 50]); + + let result = set.remove_full(&50); + assert_eq!(result, Some((3, 50))); + assert_eq!(set.len(), 3); + assert_eq!(set.as_slice(), &[10, 30, 40]); + + let result = set.remove_full(&60); + assert_eq!(result, None); + assert_eq!(set.len(), 3); + assert_eq!(set.as_slice(), &[10, 30, 40]); +} + +#[test] +fn shift_remove_index() { + let mut set: OrderSet = OrderSet::new(); + set.insert(10); + set.insert(20); + set.insert(30); + set.insert(40); + set.insert(50); + + let result = set.remove_index(1); + assert_eq!(result, Some(20)); + assert_eq!(set.len(), 4); + assert_eq!(set.as_slice(), &[10, 30, 40, 50]); + + let result = set.remove_index(1); + assert_eq!(result, Some(30)); + assert_eq!(set.len(), 3); + assert_eq!(set.as_slice(), &[10, 40, 50]); + + let result = set.remove_index(3); + assert_eq!(result, None); + assert_eq!(set.len(), 3); + assert_eq!(set.as_slice(), &[10, 40, 50]); +} + +#[test] +fn sort_unstable_by() { + let mut set: OrderSet = OrderSet::from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + set.sort_unstable_by(|a, b| b.cmp(a)); + assert_eq!(set.as_slice(), &[10, 9, 8, 7, 6, 5, 4, 3, 2, 1]); +} + +#[test] +fn sort_by() { + let mut set: OrderSet = OrderSet::new(); + set.insert(3); + set.insert(1); + set.insert(2); + set.sort_by(|a, b| a.cmp(b)); + assert_eq!(set.as_slice(), &[1, 2, 3]); +} + +#[test] +fn drain() { + let mut set: OrderSet = OrderSet::new(); + set.insert(1); + set.insert(2); + set.insert(3); + + { + let drain = set.drain(0..2); + assert_eq!(drain.as_slice(), &[1, 2]); + } + + assert_eq!(set.len(), 1); + assert_eq!(set.as_slice(), &[3]); +} + +#[test] +fn split_off() { + let mut set: OrderSet = OrderSet::from([1, 2, 3, 4, 5]); + let split_set: OrderSet = set.split_off(3); + + assert_eq!(split_set.len(), 2); + assert_eq!(split_set.as_slice(), &[4, 5]); + + assert_eq!(set.len(), 3); + assert_eq!(set.as_slice(), &[1, 2, 3]); +} + +#[test] +fn retain() { + let mut set: OrderSet = OrderSet::from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + set.retain(|&x| x > 4); + assert_eq!(set.len(), 6); + assert_eq!(set.as_slice(), &[5, 6, 7, 8, 9, 10]); + + set.retain(|_| false); + assert_eq!(set.len(), 0); +} + +#[test] +fn first() { + let mut index_set: OrderSet = OrderSet::new(); + index_set.insert(10); + index_set.insert(20); + index_set.insert(30); + + let result = index_set.first(); + assert_eq!(*result.unwrap(), 10); + + index_set.clear(); + let result = index_set.first(); + assert!(result.is_none()); +} + +#[test] +fn sort_by_cached_key() { + let mut index_set: OrderSet = OrderSet::new(); + index_set.insert(3); + index_set.insert(1); + index_set.insert(2); + index_set.insert(0); + index_set.sort_by_cached_key(|&x| -x); + assert_eq!(index_set.as_slice(), &[3, 2, 1, 0]); +} + +#[test] +fn insert_sorted() { + let mut set: OrderSet = OrderSet::::new(); + set.insert_sorted(1); + set.insert_sorted(3); + assert_eq!(set.insert_sorted(2), (1, true)); +} + +#[test] +fn binary_search() { + let mut set: OrderSet = OrderSet::new(); + set.insert(100); + set.insert(300); + set.insert(200); + set.insert(400); + let result = set.binary_search(&200); + assert_eq!(result, Ok(2)); + + let result = set.binary_search(&500); + assert_eq!(result, Err(4)); +} + +#[test] +fn sorted_unstable_by() { + let mut set: OrderSet = OrderSet::from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + set.sort_unstable_by(|a, b| b.cmp(a)); + assert_eq!(set.as_slice(), &[10, 9, 8, 7, 6, 5, 4, 3, 2, 1]); +} + +#[test] +fn last() { + let mut set: OrderSet = OrderSet::new(); + set.insert(1); + set.insert(2); + set.insert(3); + set.insert(4); + set.insert(5); + set.insert(6); + + assert_eq!(set.last(), Some(&6)); + + set.pop(); + assert_eq!(set.last(), Some(&5)); + + set.clear(); + assert_eq!(set.last(), None); +} + +#[test] +fn get_range() { + let set: OrderSet = OrderSet::from([1, 2, 3, 4, 5]); + let result = set.get_range(0..3); + let slice: &Slice = result.unwrap(); + assert_eq!(slice, &[1, 2, 3]); + + let result = set.get_range(0..0); + assert_eq!(result.unwrap().len(), 0); + + let result = set.get_range(2..1); + assert!(result.is_none()); +} + +#[test] +fn shift_take() { + let mut set: OrderSet = OrderSet::new(); + set.insert(1); + set.insert(2); + set.insert(3); + set.insert(4); + set.insert(5); + + let result = set.take(&2); + assert_eq!(result, Some(2)); + assert_eq!(set.len(), 4); + assert_eq!(set.as_slice(), &[1, 3, 4, 5]); + + let result = set.take(&5); + assert_eq!(result, Some(5)); + assert_eq!(set.len(), 3); + assert_eq!(set.as_slice(), &[1, 3, 4]); + + let result = set.take(&5); + assert_eq!(result, None); + assert_eq!(set.len(), 3); + assert_eq!(set.as_slice(), &[1, 3, 4]); +} + #[test] fn test_binary_search_by() { // adapted from std's test for binary_search From bf34d7709be5c6c66e52cb6d15913c1fabd55f37 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 25 Jun 2025 17:37:44 -0700 Subject: [PATCH 4/9] Switch to fastrand for bench shuffling (cherry picked from commit eaaaa56e717e19d6904645927a771abbeb9ae257) --- Cargo.toml | 2 +- benches/bench.rs | 20 ++++++++------------ benches/faststring.rs | 10 +++------- 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 97e5c89..bd6af94 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ rayon = { version = "1.9", optional = true } [dev-dependencies] itertools = "0.14" -rand = {version = "0.9", features = ["small_rng"] } +fastrand = { version = "2", default-features = false } quickcheck = { version = "1.0", default-features = false } fnv = "1.0" lazy_static = "1.3" diff --git a/benches/bench.rs b/benches/bench.rs index 574c667..1936ae1 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -16,14 +16,10 @@ use ordermap::OrderMap; use std::collections::HashMap; -use rand::rngs::SmallRng; -use rand::seq::SliceRandom; -use rand::SeedableRng; - /// Use a consistently seeded Rng for benchmark stability -fn small_rng() -> SmallRng { +fn small_rng() -> fastrand::Rng { let seed = u64::from_le_bytes(*b"ordermap"); - SmallRng::seed_from_u64(seed) + fastrand::Rng::with_seed(seed) } #[bench] @@ -280,7 +276,7 @@ where { let mut v = Vec::from_iter(iter); let mut rng = small_rng(); - v.shuffle(&mut rng); + rng.shuffle(&mut v); v } @@ -523,7 +519,7 @@ fn hashmap_merge_shuffle(b: &mut Bencher) { b.iter(|| { let mut merged = first_map.clone(); v.extend(second_map.iter().map(|(&k, &v)| (k, v))); - v.shuffle(&mut rng); + rng.shuffle(&mut v); merged.extend(v.drain(..)); merged @@ -550,7 +546,7 @@ fn ordermap_merge_shuffle(b: &mut Bencher) { b.iter(|| { let mut merged = first_map.clone(); v.extend(second_map.iter().map(|(&k, &v)| (k, v))); - v.shuffle(&mut rng); + rng.shuffle(&mut v); merged.extend(v.drain(..)); merged @@ -562,7 +558,7 @@ fn swap_remove_ordermap_100_000(b: &mut Bencher) { let map = IMAP_100K.clone(); let mut keys = Vec::from_iter(map.keys().copied()); let mut rng = small_rng(); - keys.shuffle(&mut rng); + rng.shuffle(&mut keys); b.iter(|| { let mut map = map.clone(); @@ -579,7 +575,7 @@ fn remove_ordermap_100_000_few(b: &mut Bencher) { let map = IMAP_100K.clone(); let mut keys = Vec::from_iter(map.keys().copied()); let mut rng = small_rng(); - keys.shuffle(&mut rng); + rng.shuffle(&mut keys); keys.truncate(50); b.iter(|| { @@ -600,7 +596,7 @@ fn remove_ordermap_2_000_full(b: &mut Bencher) { map.insert(key, key); } let mut rng = small_rng(); - keys.shuffle(&mut rng); + rng.shuffle(&mut keys); b.iter(|| { let mut map = map.clone(); diff --git a/benches/faststring.rs b/benches/faststring.rs index 8ac4bc2..53f35a8 100644 --- a/benches/faststring.rs +++ b/benches/faststring.rs @@ -8,19 +8,15 @@ use ordermap::OrderMap; use std::collections::HashMap; -use rand::rngs::SmallRng; -use rand::seq::SliceRandom; -use rand::SeedableRng; - use std::hash::{Hash, Hasher}; use std::borrow::Borrow; use std::ops::Deref; /// Use a consistently seeded Rng for benchmark stability -fn small_rng() -> SmallRng { +fn small_rng() -> fastrand::Rng { let seed = u64::from_le_bytes(*b"ordermap"); - SmallRng::seed_from_u64(seed) + fastrand::Rng::with_seed(seed) } #[derive(PartialEq, Eq, Copy, Clone)] @@ -68,7 +64,7 @@ where { let mut v = Vec::from_iter(iter); let mut rng = small_rng(); - v.shuffle(&mut rng); + rng.shuffle(&mut v); v } From 47cb612eb93fe40dd2b109198b6daf071b80db35 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 25 Jun 2025 17:45:52 -0700 Subject: [PATCH 5/9] Drop lazy_static for LazyLock (cherry picked from commit 68201eb0a389f6f96cafd7b3f887e013557be041) --- Cargo.toml | 1 - benches/bench.rs | 83 +++++++++++++++++++++--------------------------- 2 files changed, 37 insertions(+), 47 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bd6af94..28d6a2f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,7 +27,6 @@ itertools = "0.14" fastrand = { version = "2", default-features = false } quickcheck = { version = "1.0", default-features = false } fnv = "1.0" -lazy_static = "1.3" serde_derive = "1.0" [features] diff --git a/benches/bench.rs b/benches/bench.rs index 1936ae1..6233413 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -1,12 +1,11 @@ #![feature(test)] extern crate test; -#[macro_use] -extern crate lazy_static; use fnv::FnvHasher; use std::hash::BuildHasherDefault; use std::hash::Hash; +use std::sync::LazyLock; type FnvBuilder = BuildHasherDefault; use test::black_box; @@ -353,53 +352,45 @@ const LOOKUP_MAP_SIZE: u32 = 100_000_u32; const LOOKUP_SAMPLE_SIZE: u32 = 5000; const SORT_MAP_SIZE: usize = 10_000; -// use lazy_static so that comparison benchmarks use the exact same inputs -lazy_static! { - static ref KEYS: Vec = shuffled_keys(0..LOOKUP_MAP_SIZE); -} +// use (lazy) statics so that comparison benchmarks use the exact same inputs -lazy_static! { - static ref HMAP_100K: HashMap = { - let c = LOOKUP_MAP_SIZE; - let mut map = HashMap::with_capacity(c as usize); - let keys = &*KEYS; - for &key in keys { - map.insert(key, key); - } - map - }; -} +static KEYS: LazyLock> = LazyLock::new(|| shuffled_keys(0..LOOKUP_MAP_SIZE)); -lazy_static! { - static ref IMAP_100K: OrderMap = { - let c = LOOKUP_MAP_SIZE; - let mut map = OrderMap::with_capacity(c as usize); - let keys = &*KEYS; - for &key in keys { - map.insert(key, key); - } - map - }; -} +static HMAP_100K: LazyLock> = LazyLock::new(|| { + let c = LOOKUP_MAP_SIZE; + let mut map = HashMap::with_capacity(c as usize); + let keys = &*KEYS; + for &key in keys { + map.insert(key, key); + } + map +}); -lazy_static! { - static ref IMAP_SORT_U32: OrderMap = { - let mut map = OrderMap::with_capacity(SORT_MAP_SIZE); - for &key in &KEYS[..SORT_MAP_SIZE] { - map.insert(key, key); - } - map - }; -} -lazy_static! { - static ref IMAP_SORT_S: OrderMap = { - let mut map = OrderMap::with_capacity(SORT_MAP_SIZE); - for &key in &KEYS[..SORT_MAP_SIZE] { - map.insert(format!("{:^16x}", &key), String::new()); - } - map - }; -} +static IMAP_100K: LazyLock> = LazyLock::new(|| { + let c = LOOKUP_MAP_SIZE; + let mut map = OrderMap::with_capacity(c as usize); + let keys = &*KEYS; + for &key in keys { + map.insert(key, key); + } + map +}); + +static IMAP_SORT_U32: LazyLock> = LazyLock::new(|| { + let mut map = OrderMap::with_capacity(SORT_MAP_SIZE); + for &key in &KEYS[..SORT_MAP_SIZE] { + map.insert(key, key); + } + map +}); + +static IMAP_SORT_S: LazyLock> = LazyLock::new(|| { + let mut map = OrderMap::with_capacity(SORT_MAP_SIZE); + for &key in &KEYS[..SORT_MAP_SIZE] { + map.insert(format!("{:^16x}", &key), String::new()); + } + map +}); #[bench] fn lookup_hashmap_100_000_multi(b: &mut Bencher) { From b0efbc3ab21fa5b508f187085c06bc56f91b89ae Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Sat, 3 Feb 2024 10:15:15 -0800 Subject: [PATCH 6/9] Add map and set `extract_if` (cherry picked from commit 5aa1b8319013978b44fe7bd491e521713447d86d) --- Cargo.toml | 2 +- src/map.rs | 42 ++++++++++++++++++++++++++++++++++++++++-- src/set.rs | 38 +++++++++++++++++++++++++++++++++++++- tests/quick.rs | 41 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 28d6a2f..cd61b91 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ rust-version = "1.63" bench = false [dependencies] -indexmap = { version = "2.9.0", default-features = false } +indexmap = { version = "2.10.0", default-features = false } arbitrary = { version = "1.0", optional = true, default-features = false } quickcheck = { version = "1.0", optional = true, default-features = false } diff --git a/src/map.rs b/src/map.rs index 49d97a0..ee454d0 100644 --- a/src/map.rs +++ b/src/map.rs @@ -28,8 +28,8 @@ pub use self::mutable::MutableEntryKey; pub use self::mutable::MutableKeys; pub use self::raw_entry_v1::RawEntryApiV1; pub use indexmap::map::{ - Drain, IntoIter, IntoKeys, IntoValues, Iter, IterMut, IterMut2, Keys, Slice, Splice, Values, - ValuesMut, + Drain, ExtractIf, IntoIter, IntoKeys, IntoValues, Iter, IterMut, IterMut2, Keys, Slice, Splice, + Values, ValuesMut, }; #[cfg(feature = "rayon")] @@ -282,6 +282,44 @@ impl OrderMap { self.inner.drain(range) } + /// Creates an iterator which uses a closure to determine if an element should be removed. + /// + /// If the closure returns true, the element is removed from the map and yielded. + /// If the closure returns false, or panics, the element remains in the map and will not be + /// yielded. + /// + /// Note that `extract_if` lets you mutate every value in the filter closure, regardless of + /// whether you choose to keep or remove it. + /// + /// If the returned `ExtractIf` is not exhausted, e.g. because it is dropped without iterating + /// or the iteration short-circuits, then the remaining elements will be retained. + /// Use [`retain`] with a negated predicate if you do not need the returned iterator. + /// + /// [`retain`]: OrderMap::retain + /// + /// # Examples + /// + /// Splitting a map into even and odd keys, reusing the original map: + /// + /// ``` + /// use ordermap::OrderMap; + /// + /// let mut map: OrderMap = (0..8).map(|x| (x, x)).collect(); + /// let extracted: OrderMap = map.extract_if(|k, _v| k % 2 == 0).collect(); + /// + /// let evens = extracted.keys().copied().collect::>(); + /// let odds = map.keys().copied().collect::>(); + /// + /// assert_eq!(evens, vec![0, 2, 4, 6]); + /// assert_eq!(odds, vec![1, 3, 5, 7]); + /// ``` + pub fn extract_if(&mut self, pred: F) -> ExtractIf<'_, K, V, F> + where + F: FnMut(&K, &mut V) -> bool, + { + self.inner.extract_if(.., pred) + } + /// Splits the collection into two at the given index. /// /// Returns a newly allocated map containing the elements in the range diff --git a/src/set.rs b/src/set.rs index 640b595..2efc989 100644 --- a/src/set.rs +++ b/src/set.rs @@ -18,7 +18,8 @@ mod tests; pub use self::mutable::MutableValues; pub use indexmap::set::{ - Difference, Drain, Intersection, IntoIter, Iter, Slice, Splice, SymmetricDifference, Union, + Difference, Drain, ExtractIf, Intersection, IntoIter, Iter, Slice, Splice, SymmetricDifference, + Union, }; #[cfg(feature = "rayon")] @@ -233,6 +234,41 @@ impl OrderSet { self.inner.drain(range) } + /// Creates an iterator which uses a closure to determine if a value should be removed. + /// + /// If the closure returns true, then the value is removed and yielded. + /// If the closure returns false, the value will remain in the list and will not be yielded + /// by the iterator. + /// + /// If the returned `ExtractIf` is not exhausted, e.g. because it is dropped without iterating + /// or the iteration short-circuits, then the remaining elements will be retained. + /// Use [`retain`] with a negated predicate if you do not need the returned iterator. + /// + /// [`retain`]: OrderSet::retain + /// + /// # Examples + /// + /// Splitting a set into even and odd values, reusing the original set: + /// + /// ``` + /// use ordermap::OrderSet; + /// + /// let mut set: OrderSet = (0..8).collect(); + /// let extracted: OrderSet = set.extract_if(|v| v % 2 == 0).collect(); + /// + /// let evens = extracted.into_iter().collect::>(); + /// let odds = set.into_iter().collect::>(); + /// + /// assert_eq!(evens, vec![0, 2, 4, 6]); + /// assert_eq!(odds, vec![1, 3, 5, 7]); + /// ``` + pub fn extract_if(&mut self, pred: F) -> ExtractIf<'_, T, F> + where + F: FnMut(&T) -> bool, + { + self.inner.extract_if(.., pred) + } + /// Splits the collection into two at the given index. /// /// Returns a newly allocated set containing the elements in the range diff --git a/tests/quick.rs b/tests/quick.rs index 9a229b5..fcf3991 100644 --- a/tests/quick.rs +++ b/tests/quick.rs @@ -191,6 +191,47 @@ quickcheck_limit! { } } + fn extract_if_odd(insert: Vec) -> bool { + let mut map = OrderMap::new(); + for &x in &insert { + map.insert(x, x.to_string()); + } + + let (odd, even): (Vec<_>, Vec<_>) = map.keys().copied().partition(|k| k % 2 == 1); + + let extracted: Vec<_> = map + .extract_if(|k, _| k % 2 == 1) + .map(|(k, _)| k) + .collect(); + + even.iter().all(|k| map.contains_key(k)) + && map.keys().eq(&even) + && extracted == odd + } + + fn extract_if_odd_limit(insert: Vec, limit: usize) -> bool { + let mut map = OrderMap::new(); + for &x in &insert { + map.insert(x, x.to_string()); + } + let limit = limit % (map.len() + 1); + + let mut i = 0; + let (odd, other): (Vec<_>, Vec<_>) = map.keys().copied().partition(|k| { + k % 2 == 1 && i < limit && { i += 1; true } + }); + + let extracted: Vec<_> = map + .extract_if(|k, _| k % 2 == 1) + .map(|(k, _)| k) + .take(limit) + .collect(); + + other.iter().all(|k| map.contains_key(k)) + && map.keys().eq(&other) + && extracted == odd + } + // aka shift_remove fn remove(insert: Vec, remove: Vec) -> bool { let mut map = OrderMap::new(); From e3c00c9272151bbd461c5cf63a4a397c97301b26 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 19 Nov 2024 17:48:49 -0800 Subject: [PATCH 7/9] Add range support to `extract_if` (cherry picked from commit a8d7dc50a250971fe2203bdf23c2b5a307db7c86) --- src/map.rs | 15 +++++++++++---- src/set.rs | 15 +++++++++++---- tests/quick.rs | 4 ++-- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/map.rs b/src/map.rs index ee454d0..d261815 100644 --- a/src/map.rs +++ b/src/map.rs @@ -282,7 +282,8 @@ impl OrderMap { self.inner.drain(range) } - /// Creates an iterator which uses a closure to determine if an element should be removed. + /// Creates an iterator which uses a closure to determine if an element should be removed, + /// for all elements in the given range. /// /// If the closure returns true, the element is removed from the map and yielded. /// If the closure returns false, or panics, the element remains in the map and will not be @@ -291,6 +292,11 @@ impl OrderMap { /// Note that `extract_if` lets you mutate every value in the filter closure, regardless of /// whether you choose to keep or remove it. /// + /// The range may be any type that implements [`RangeBounds`], + /// including all of the `std::ops::Range*` types, or even a tuple pair of + /// `Bound` start and end values. To check the entire map, use `RangeFull` + /// like `map.extract_if(.., predicate)`. + /// /// If the returned `ExtractIf` is not exhausted, e.g. because it is dropped without iterating /// or the iteration short-circuits, then the remaining elements will be retained. /// Use [`retain`] with a negated predicate if you do not need the returned iterator. @@ -305,7 +311,7 @@ impl OrderMap { /// use ordermap::OrderMap; /// /// let mut map: OrderMap = (0..8).map(|x| (x, x)).collect(); - /// let extracted: OrderMap = map.extract_if(|k, _v| k % 2 == 0).collect(); + /// let extracted: OrderMap = map.extract_if(.., |k, _v| k % 2 == 0).collect(); /// /// let evens = extracted.keys().copied().collect::>(); /// let odds = map.keys().copied().collect::>(); @@ -313,11 +319,12 @@ impl OrderMap { /// assert_eq!(evens, vec![0, 2, 4, 6]); /// assert_eq!(odds, vec![1, 3, 5, 7]); /// ``` - pub fn extract_if(&mut self, pred: F) -> ExtractIf<'_, K, V, F> + pub fn extract_if(&mut self, range: R, pred: F) -> ExtractIf<'_, K, V, F> where F: FnMut(&K, &mut V) -> bool, + R: RangeBounds, { - self.inner.extract_if(.., pred) + self.inner.extract_if(range, pred) } /// Splits the collection into two at the given index. diff --git a/src/set.rs b/src/set.rs index 2efc989..bf4d50f 100644 --- a/src/set.rs +++ b/src/set.rs @@ -234,12 +234,18 @@ impl OrderSet { self.inner.drain(range) } - /// Creates an iterator which uses a closure to determine if a value should be removed. + /// Creates an iterator which uses a closure to determine if a value should be removed, + /// for all values in the given range. /// /// If the closure returns true, then the value is removed and yielded. /// If the closure returns false, the value will remain in the list and will not be yielded /// by the iterator. /// + /// The range may be any type that implements [`RangeBounds`], + /// including all of the `std::ops::Range*` types, or even a tuple pair of + /// `Bound` start and end values. To check the entire set, use `RangeFull` + /// like `set.extract_if(.., predicate)`. + /// /// If the returned `ExtractIf` is not exhausted, e.g. because it is dropped without iterating /// or the iteration short-circuits, then the remaining elements will be retained. /// Use [`retain`] with a negated predicate if you do not need the returned iterator. @@ -254,7 +260,7 @@ impl OrderSet { /// use ordermap::OrderSet; /// /// let mut set: OrderSet = (0..8).collect(); - /// let extracted: OrderSet = set.extract_if(|v| v % 2 == 0).collect(); + /// let extracted: OrderSet = set.extract_if(.., |v| v % 2 == 0).collect(); /// /// let evens = extracted.into_iter().collect::>(); /// let odds = set.into_iter().collect::>(); @@ -262,11 +268,12 @@ impl OrderSet { /// assert_eq!(evens, vec![0, 2, 4, 6]); /// assert_eq!(odds, vec![1, 3, 5, 7]); /// ``` - pub fn extract_if(&mut self, pred: F) -> ExtractIf<'_, T, F> + pub fn extract_if(&mut self, range: R, pred: F) -> ExtractIf<'_, T, F> where F: FnMut(&T) -> bool, + R: RangeBounds, { - self.inner.extract_if(.., pred) + self.inner.extract_if(range, pred) } /// Splits the collection into two at the given index. diff --git a/tests/quick.rs b/tests/quick.rs index fcf3991..3c86255 100644 --- a/tests/quick.rs +++ b/tests/quick.rs @@ -200,7 +200,7 @@ quickcheck_limit! { let (odd, even): (Vec<_>, Vec<_>) = map.keys().copied().partition(|k| k % 2 == 1); let extracted: Vec<_> = map - .extract_if(|k, _| k % 2 == 1) + .extract_if(.., |k, _| k % 2 == 1) .map(|(k, _)| k) .collect(); @@ -222,7 +222,7 @@ quickcheck_limit! { }); let extracted: Vec<_> = map - .extract_if(|k, _| k % 2 == 1) + .extract_if(.., |k, _| k % 2 == 1) .map(|(k, _)| k) .take(limit) .collect(); From 6bed3a55a76dcfced440c5e83de690602ead3070 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 22 May 2025 18:00:07 -0700 Subject: [PATCH 8/9] Document and track `extract_if` panics (cherry picked from commit e09eaaf8e547ec56e564980b20f62b7c4d459fdd) --- src/map.rs | 4 ++++ src/set.rs | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/map.rs b/src/map.rs index d261815..ae64ca9 100644 --- a/src/map.rs +++ b/src/map.rs @@ -303,6 +303,9 @@ impl OrderMap { /// /// [`retain`]: OrderMap::retain /// + /// ***Panics*** if the starting point is greater than the end point or if + /// the end point is greater than the length of the map. + /// /// # Examples /// /// Splitting a map into even and odd keys, reusing the original map: @@ -319,6 +322,7 @@ impl OrderMap { /// assert_eq!(evens, vec![0, 2, 4, 6]); /// assert_eq!(odds, vec![1, 3, 5, 7]); /// ``` + #[track_caller] pub fn extract_if(&mut self, range: R, pred: F) -> ExtractIf<'_, K, V, F> where F: FnMut(&K, &mut V) -> bool, diff --git a/src/set.rs b/src/set.rs index bf4d50f..e6c7747 100644 --- a/src/set.rs +++ b/src/set.rs @@ -252,6 +252,9 @@ impl OrderSet { /// /// [`retain`]: OrderSet::retain /// + /// ***Panics*** if the starting point is greater than the end point or if + /// the end point is greater than the length of the set. + /// /// # Examples /// /// Splitting a set into even and odd values, reusing the original set: @@ -268,6 +271,7 @@ impl OrderSet { /// assert_eq!(evens, vec![0, 2, 4, 6]); /// assert_eq!(odds, vec![1, 3, 5, 7]); /// ``` + #[track_caller] pub fn extract_if(&mut self, range: R, pred: F) -> ExtractIf<'_, T, F> where F: FnMut(&T) -> bool, From 77e9a9e740faf9c8ac4dee09b6498cd67f1a86ce Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 26 Jun 2025 14:32:38 -0700 Subject: [PATCH 9/9] Release 0.5.8 --- Cargo.toml | 2 +- RELEASES.md | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index cd61b91..82d846e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "ordermap" edition = "2021" -version = "0.5.7" +version = "0.5.8" documentation = "https://docs.rs/ordermap/" repository = "https://github.com/indexmap-rs/ordermap" license = "Apache-2.0 OR MIT" diff --git a/RELEASES.md b/RELEASES.md index 54454ae..405baa9 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,5 +1,11 @@ # Releases +## 0.5.8 (2025-06-26) + +- Added `extract_if` methods to `OrderMap` and `OrderSet`, similar to the + methods for `HashMap` and `HashSet` with ranges like `Vec::extract_if`. +- Added more `#[track_caller]` annotations to functions that may panic. + ## 0.5.7 (2025-04-04) - Added a `get_disjoint_mut` method to `OrderMap`, matching Rust 1.86's