From 28a13cf861f9cd4afac34bf548a06d5919efece1 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Tue, 15 Nov 2022 22:02:00 +0100 Subject: [PATCH] Remove drain-on-drop behavior from DrainFilter --- src/map.rs | 42 ++++++++---------------------------------- src/set.rs | 25 ++++++------------------- 2 files changed, 14 insertions(+), 53 deletions(-) diff --git a/src/map.rs b/src/map.rs index 5049aa2b5b..9e9c1c381e 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1,3 +1,4 @@ +use std::fmt; use crate::raw::{Allocator, Bucket, Global, RawDrain, RawIntoIter, RawIter, RawTable}; use crate::{Equivalent, TryReserveError}; use core::borrow::Borrow; @@ -975,12 +976,9 @@ impl HashMap { /// Note that `drain_filter` lets you mutate every value in the filter closure, regardless of /// whether you choose to keep or remove it. /// - /// When the returned DrainedFilter is dropped, any remaining elements that satisfy - /// the predicate are dropped from the table. - /// - /// It is unspecified how many more elements will be subjected to the closure - /// if a panic occurs in the closure, or a panic occurs while dropping an element, - /// or if the `DrainFilter` value is leaked. + /// If the returned `DrainFilter` 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. /// /// Keeps the allocated memory for reuse. /// @@ -1007,9 +1005,8 @@ impl HashMap { /// let d = map.drain_filter(|k, _v| k % 2 != 0); /// } /// - /// // But the map lens have been reduced by half - /// // even if we do not use DrainFilter iterator. - /// assert_eq!(map.len(), 4); + /// // DrainFilter was not exhausted, therefore no elements were drained. + /// assert_eq!(map.len(), 8); /// ``` #[cfg_attr(feature = "inline-more", inline)] pub fn drain_filter(&mut self, f: F) -> DrainFilter<'_, K, V, F, A> @@ -2739,6 +2736,7 @@ impl Drain<'_, K, V, A> { /// /// assert_eq!(map.len(), 1); /// ``` +#[must_use = "Iterators are lazy unless consumed"] pub struct DrainFilter<'a, K, V, F, A: Allocator + Clone = Global> where F: FnMut(&K, &mut V) -> bool, @@ -2747,30 +2745,6 @@ where inner: DrainFilterInner<'a, K, V, A>, } -impl<'a, K, V, F, A> Drop for DrainFilter<'a, K, V, F, A> -where - F: FnMut(&K, &mut V) -> bool, - A: Allocator + Clone, -{ - #[cfg_attr(feature = "inline-more", inline)] - fn drop(&mut self) { - while let Some(item) = self.next() { - let guard = ConsumeAllOnDrop(self); - drop(item); - mem::forget(guard); - } - } -} - -pub(super) struct ConsumeAllOnDrop<'a, T: Iterator>(pub &'a mut T); - -impl Drop for ConsumeAllOnDrop<'_, T> { - #[cfg_attr(feature = "inline-more", inline)] - fn drop(&mut self) { - self.0.for_each(drop); - } -} - impl Iterator for DrainFilter<'_, K, V, F, A> where F: FnMut(&K, &mut V) -> bool, @@ -8159,7 +8133,7 @@ mod test_map { } { let mut map: HashMap = (0..8).map(|x| (x, x * 10)).collect(); - drop(map.drain_filter(|&k, _| k % 2 == 0)); + map.drain_filter(|&k, _| k % 2 == 0).for_each(drop); assert_eq!(map.len(), 4); } } diff --git a/src/set.rs b/src/set.rs index 60c1b1cd19..ae63ab0a21 100644 --- a/src/set.rs +++ b/src/set.rs @@ -5,10 +5,9 @@ use alloc::borrow::ToOwned; use core::fmt; use core::hash::{BuildHasher, Hash}; use core::iter::{Chain, FromIterator, FusedIterator}; -use core::mem; use core::ops::{BitAnd, BitOr, BitXor, Sub}; -use super::map::{self, ConsumeAllOnDrop, DefaultHashBuilder, DrainFilterInner, HashMap, Keys}; +use super::map::{self, DefaultHashBuilder, DrainFilterInner, HashMap, Keys}; use crate::raw::{Allocator, Global}; // Future Optimization (FIXME!) @@ -380,8 +379,9 @@ impl HashSet { /// In other words, move all elements `e` such that `f(&e)` returns `true` out /// into another iterator. /// - /// When the returned DrainedFilter is dropped, any remaining elements that satisfy - /// the predicate are dropped from the set. + /// If the returned `DrainFilter` 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. /// /// # Examples /// @@ -1552,6 +1552,7 @@ pub struct Drain<'a, K, A: Allocator + Clone = Global> { /// /// [`drain_filter`]: struct.HashSet.html#method.drain_filter /// [`HashSet`]: struct.HashSet.html +#[must_use = "Iterators are lazy unless consumed"] pub struct DrainFilter<'a, K, F, A: Allocator + Clone = Global> where F: FnMut(&K) -> bool, @@ -1748,20 +1749,6 @@ impl fmt::Debug for Drain<'_, K, A> { } } -impl<'a, K, F, A: Allocator + Clone> Drop for DrainFilter<'a, K, F, A> -where - F: FnMut(&K) -> bool, -{ - #[cfg_attr(feature = "inline-more", inline)] - fn drop(&mut self) { - while let Some(item) = self.next() { - let guard = ConsumeAllOnDrop(self); - drop(item); - mem::forget(guard); - } - } -} - impl Iterator for DrainFilter<'_, K, F, A> where F: FnMut(&K) -> bool, @@ -2840,7 +2827,7 @@ mod test_set { } { let mut set: HashSet = (0..8).collect(); - drop(set.drain_filter(|&k| k % 2 == 0)); + set.drain_filter(|&k| k % 2 == 0).for_each(drop); assert_eq!(set.len(), 4, "Removes non-matching items on drop"); } }