From 30b9364f99add15b34e5d88a6c618b723c31e0db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Tue, 28 Jul 2026 22:20:50 +0200 Subject: [PATCH 1/3] index_set: enable clippy::undocumented_unsafe_blocks IndexSet doesn't have any unsafe. This will help ensure any future `unsafe` is properly documented. --- src/index_set.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/index_set.rs b/src/index_set.rs index 803fa29107..000325657c 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, From d130019365fd3fe75544313d9fe1bebfa25dfd2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Wed, 29 Jul 2026 13:41:17 +0200 Subject: [PATCH 2/3] `IndexSet` and `IndexMap`: Add `.swap_remove()` Remove changed the ordering of the map and this was not made clear in either the documentation or the name, this patch follows the original indexmap crate in naming it `swap_remove` instead. --- CHANGELOG.md | 1 + src/index_map.rs | 67 +++++++++++++++++++++++++++++++++++++----------- src/index_set.rs | 32 ++++++++++++++++++++++- 3 files changed, 84 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31d8625326..d70720ff27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +- Added `swap_remove()` to `IndexMap` and `IndexSet`. - 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..072a441aa1 100644 --- a/src/index_map.rs +++ b/src/index_map.rs @@ -691,8 +691,19 @@ 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()`, replacing this entry’s position + /// with the last element. 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 +742,20 @@ 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()`, replacing this entry’s position + /// with the last element. 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 +1343,7 @@ where } } - /// Same as [`swap_remove`](Self::swap_remove) + /// Removes an element. /// /// Computes in *O*(1) time (average). /// @@ -1335,6 +1357,9 @@ where /// assert_eq!(map.remove(&1), Some("a")); /// assert_eq!(map.remove(&1), None); /// ``` + /// + /// **NOTE**: This is equivalent to `.swap_remove(key)`, replacing this entry’s position + /// with the last element. pub fn remove(&mut self, key: &Q) -> Option where K: Borrow, @@ -1346,11 +1371,23 @@ 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 +1944,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 +2067,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 +2086,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 +2149,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 +2321,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 +2410,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 +2427,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 000325657c..b771f652f8 100644 --- a/src/index_set.rs +++ b/src/index_set.rs @@ -495,12 +495,42 @@ where /// assert_eq!(set.remove(&2), true); /// assert_eq!(set.remove(&2), false); /// ``` + /// + /// **NOTE**: This is equivalent to `.swap_remove(key)`, replacing this entry’s position + /// with the last element. pub fn remove(&mut self, value: &Q) -> bool where T: Borrow, Q: ?Sized + Eq + Hash, { - self.map.remove(value).is_some() + 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 + /// + /// ``` + /// use heapless::index_set::FnvIndexSet; + /// + /// let mut set = FnvIndexSet::<_, 16>::new(); + /// + /// set.insert(2).unwrap(); + /// assert_eq!(set.swap_remove(&2), true); + /// assert_eq!(set.swap_remove(&2), false); + /// ``` + pub fn swap_remove(&mut self, value: &Q) -> bool + where + T: Borrow, + Q: ?Sized + Eq + Hash, + { + self.map.swap_remove(value).is_some() } /// Retains only the elements specified by the predicate. From 8a701f593ff52486efea7d8e4d008b63df9198c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Wed, 29 Jul 2026 13:22:58 +0200 Subject: [PATCH 3/3] Deprecate `.remove()` in `IndexMap` and `IndexSet` in favour of `.swap_remove()` Closes https://github.com/rust-embedded/heapless/issues/679 --- CHANGELOG.md | 1 + src/index_map.rs | 25 ++++++++++++++++++------- src/index_set.rs | 20 ++++++-------------- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d70720ff27..e380d4c478 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ 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 072a441aa1..05f9612c3e 100644 --- a/src/index_map.rs +++ b/src/index_map.rs @@ -693,8 +693,12 @@ where /// Removes this entry from the map and yields its corresponding key and value. /// - /// **NOTE**: This is equivalent to `.swap_remove_entry()`, replacing this entry’s position - /// with the last element. + /// **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() } @@ -744,8 +748,12 @@ where /// Removes this entry from the map and yields its value. /// - /// **NOTE**: This is equivalent to `.swap_remove()`, replacing this entry’s position - /// with the last element. + /// **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.swap_remove() } @@ -1358,8 +1366,12 @@ where /// assert_eq!(map.remove(&1), None); /// ``` /// - /// **NOTE**: This is equivalent to `.swap_remove(key)`, replacing this entry’s position - /// with the last element. + /// **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, @@ -1387,7 +1399,6 @@ where /// 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, diff --git a/src/index_set.rs b/src/index_set.rs index b771f652f8..cd5e7fd6e8 100644 --- a/src/index_set.rs +++ b/src/index_set.rs @@ -484,20 +484,12 @@ 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. /// - /// # Examples - /// - /// ``` - /// use heapless::index_set::FnvIndexSet; - /// - /// let mut set = FnvIndexSet::<_, 16>::new(); - /// - /// set.insert(2).unwrap(); - /// assert_eq!(set.remove(&2), true); - /// assert_eq!(set.remove(&2), false); - /// ``` - /// - /// **NOTE**: This is equivalent to `.swap_remove(key)`, replacing this entry’s position - /// with the last element. + /// **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,