diff --git a/CHANGELOG.md b/CHANGELOG.md index 31d8625326..e380d4c478 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +- Added `swap_remove()` to `IndexMap` and `IndexSet`. +- Deprecated `.remove()` in `IndexMap` and `IndexSet` in favour of `.swap_remove()`. - Fixed `IndexMap::truncate` leading to an inconsistent state. - Fixed `Deque::make_contigous` leading to an inconsistent state. - Added `resize_with` to `Vec` diff --git a/src/index_map.rs b/src/index_map.rs index e120deb16c..05f9612c3e 100644 --- a/src/index_map.rs +++ b/src/index_map.rs @@ -691,8 +691,23 @@ where &self.key } - /// Removes this entry from the map and yields its corresponding key and value + /// Removes this entry from the map and yields its corresponding key and value. + /// + /// **NOTE**: This is equivalent to [`.swap_remove_entry()`](OccupiedEntry::swap_remove_entry), + /// replacing this entry’s position with the last element, and it is deprecated in favor of + /// calling that explicitly. + #[deprecated( + note = "`remove_entry` disrupts the map order -- use `swap_remove_entry` for explicit behavior." + )] pub fn remove_entry(self) -> (K, V) { + self.swap_remove_entry() + } + + /// Removes this entry from the map and yields its corresponding key and value. + /// + /// Like `Vec::swap_remove`, the value is removed by swapping it with the last element of the + /// map and popping it off. **This perturbs the position of what used to be the last element!**. + pub fn swap_remove_entry(self) -> (K, V) { // SAFETY: We know that `pos` is valid from the creation of the entry // and that cannot have changed since we held a mutable entry to the map unsafe { self.core.remove_found(self.probe, self.pos) } @@ -731,9 +746,24 @@ where } } - /// Removes this entry from the map and yields its value + /// Removes this entry from the map and yields its value. + /// + /// **NOTE**: This is equivalent to [`.swap_remove()`](OccupiedEntry::swap_remove), replacing + /// this entry’s position with the last element, and it is deprecated in favor of calling + /// that explicitly. + #[deprecated( + note = "`remove` disrupts the map order -- use `swap_remove` for explicit behavior." + )] pub fn remove(self) -> V { - self.remove_entry().1 + self.swap_remove() + } + + /// Removes this entry from the map and yields its value. + /// + /// Like `Vec::swap_remove`, the pair is removed by swapping it with the last element of the map + /// and popping it off. **This perturbs the position of what used to be the last element!**. + pub fn swap_remove(self) -> V { + self.swap_remove_entry().1 } } @@ -1321,7 +1351,7 @@ where } } - /// Same as [`swap_remove`](Self::swap_remove) + /// Removes an element. /// /// Computes in *O*(1) time (average). /// @@ -1335,6 +1365,13 @@ where /// assert_eq!(map.remove(&1), Some("a")); /// assert_eq!(map.remove(&1), None); /// ``` + /// + /// **NOTE**: This is equivalent to [`.swap_remove(key)`](IndexMap::swap_remove), replacing this + /// entry’s position with the last element, and it is deprecated in favor of calling that + /// explicitly. + #[deprecated( + note = "`remove` disrupts the map order -- use `swap_remove` for explicit behavior." + )] pub fn remove(&mut self, key: &Q) -> Option where K: Borrow, @@ -1346,11 +1383,22 @@ where /// Remove the key-value pair equivalent to `key` and return its value. /// /// Like `Vec::swap_remove`, the pair is removed by swapping it with the last element of the map - /// and popping it off. **This perturbs the position of what used to be the last element!** + /// and popping it off. **This perturbs the position of what used to be the last element!**. /// /// Return `None` if `key` is not in map. /// /// Computes in *O*(1) time (average). + /// + /// # Examples + /// + /// ``` + /// use heapless::index_map::FnvIndexMap; + /// + /// let mut map = FnvIndexMap::<_, _, 8>::new(); + /// map.insert(1, "a").unwrap(); + /// assert_eq!(map.swap_remove(&1), Some("a")); + /// assert_eq!(map.swap_remove(&1), None); + /// ``` pub fn swap_remove(&mut self, key: &Q) -> Option where K: Borrow, @@ -1907,7 +1955,7 @@ mod tests { src.insert("k4", "v4").unwrap(); let clone = src.clone(); for (k, v) in clone.into_iter() { - assert_eq!(v, src.remove(k).unwrap()); + assert_eq!(v, src.swap_remove(k).unwrap()); } assert!(src.is_empty()); } @@ -2030,7 +2078,7 @@ mod tests { let entry = src.entry(key); match entry { Entry::Occupied(o) => { - assert_eq!((key, value), o.remove_entry()); + assert_eq!((key, value), o.swap_remove_entry()); } Entry::Vacant(_) => { panic!("Entry not found") @@ -2049,7 +2097,7 @@ mod tests { let entry = src.entry(key); match entry { Entry::Occupied(o) => { - assert_eq!(value, o.remove()); + assert_eq!(value, o.swap_remove()); } Entry::Vacant(_) => { panic!("Entry not found"); @@ -2112,7 +2160,7 @@ mod tests { for i in 0..MAP_SLOTS { match src.entry(i) { Entry::Occupied(o) => { - assert_eq!((i, i + add_mod), o.remove_entry()); + assert_eq!((i, i + add_mod), o.swap_remove_entry()); } Entry::Vacant(_) => { panic!("Entry not found after insert"); @@ -2284,14 +2332,14 @@ mod tests { for x in 0..=u16::MAX { assert_eq!(map.get(&CustomHashU16(x)).unwrap(), &x); } - assert_eq!(map.remove(&CustomHashU16(0x123)).unwrap(), 0x123); + assert_eq!(map.swap_remove(&CustomHashU16(0x123)).unwrap(), 0x123); for x in 0..=u16::MAX { if x == 0x123 { continue; } assert_eq!(map.get(&CustomHashU16(x)).unwrap(), &x); } - assert_eq!(map.remove(&CustomHashU16(u16::MAX)).unwrap(), u16::MAX); + assert_eq!(map.swap_remove(&CustomHashU16(u16::MAX)).unwrap(), u16::MAX); for x in 0..=u16::MAX { if x == 0x123 || x == u16::MAX { continue; @@ -2373,11 +2421,11 @@ mod tests { entry.insert(0x10000); assert_eq!(map.get(&ControlledHash(0xFFFF, 0)), Some(&0x10000)); - assert_eq!(map.remove(&ControlledHash(0xFFFF, 0)), Some(0x10000)); + assert_eq!(map.swap_remove(&ControlledHash(0xFFFF, 0)), Some(0x10000)); map.insert(ControlledHash(0xFFFF, 0), 0xFFFF).unwrap(); - assert_eq!(map.remove(&ControlledHash(0xFFFE, 0)), Some(0xFFFE)); + assert_eq!(map.swap_remove(&ControlledHash(0xFFFE, 0)), Some(0xFFFE)); assert_eq!(map.get(&ControlledHash(0xFFFF, 0)), Some(&0xFFFF)); - assert_eq!(map.remove(&ControlledHash(0xFFFF, 0)), Some(0xFFFF)); + assert_eq!(map.swap_remove(&ControlledHash(0xFFFF, 0)), Some(0xFFFF)); assert!(map.get(&ControlledHash(0xFFFF, 0)).is_none()); } @@ -2390,6 +2438,6 @@ mod tests { map.insert(0, 0).unwrap(); map.insert(4, 4).unwrap(); map.insert(8, 8).unwrap(); - map.remove(&0).unwrap(); // never returns + map.swap_remove(&0).unwrap(); // never returns } } diff --git a/src/index_set.rs b/src/index_set.rs index 803fa29107..cd5e7fd6e8 100644 --- a/src/index_set.rs +++ b/src/index_set.rs @@ -1,4 +1,6 @@ +#![deny(clippy::undocumented_unsafe_blocks)] //! A fixed-capacity hash set where the iteration order is independent of the hash values. + use core::{ borrow::Borrow, fmt, @@ -482,6 +484,28 @@ where /// The value may be any borrowed form of the set's value type, but `Hash` and `Eq` on the /// borrowed form must match those for the value type. /// + /// **NOTE**: This is equivalent to [`.swap_remove(key)`](IndexSet::swap_remove), replacing this + /// entry’s position with the last element, and it is deprecated in favor of calling that + /// explicitly. + #[deprecated( + note = "`remove` disrupts the set order -- use `swap_remove` for explicit behavior." + )] + pub fn remove(&mut self, value: &Q) -> bool + where + T: Borrow, + Q: ?Sized + Eq + Hash, + { + self.swap_remove(value) + } + + /// Removes a value from the set. Returns `true` if the value was present in the set. + /// + /// The value may be any borrowed form of the set's value type, but `Hash` and `Eq` on the + /// borrowed form must match those for the value type. + /// + /// Like `Vec::swap_remove`, the value is removed by swapping it with the last element of the + /// map and popping it off. **This perturbs the position of what used to be the last element!**. + /// /// # Examples /// /// ``` @@ -490,15 +514,15 @@ where /// let mut set = FnvIndexSet::<_, 16>::new(); /// /// set.insert(2).unwrap(); - /// assert_eq!(set.remove(&2), true); - /// assert_eq!(set.remove(&2), false); + /// assert_eq!(set.swap_remove(&2), true); + /// assert_eq!(set.swap_remove(&2), false); /// ``` - pub fn remove(&mut self, value: &Q) -> bool + pub fn swap_remove(&mut self, value: &Q) -> bool where T: Borrow, Q: ?Sized + Eq + Hash, { - self.map.remove(value).is_some() + self.map.swap_remove(value).is_some() } /// Retains only the elements specified by the predicate.