Remove drain-on-drop behavior from DrainFilter

Author: the8472

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<K, V, S, A: Allocator + Clone> HashMap<K, V, S, A> {
     /// 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<K, V, S, A: Allocator + Clone> HashMap<K, V, S, A> {
     ///     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<F>(&mut self, f: F) -> DrainFilter<'_, K, V, F, A>
@@ -2739,6 +2736,7 @@ impl<K, V, A: Allocator + Clone> 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<T: Iterator> Drop for ConsumeAllOnDrop<'_, T> {
-    #[cfg_attr(feature = "inline-more", inline)]
-    fn drop(&mut self) {
-        self.0.for_each(drop);
-    }
-}
-
 impl<K, V, F, A> 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<i32, i32> = (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);
         }
     }

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<T, S, A: Allocator + Clone> HashSet<T, S, A> {
     /// 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<K: fmt::Debug, A: Allocator + Clone> 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<K, F, A: Allocator + Clone> Iterator for DrainFilter<'_, K, F, A>
 where
     F: FnMut(&K) -> bool,
@@ -2840,7 +2827,7 @@ mod test_set {
         }
         {
             let mut set: HashSet<i32> = (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");
         }
     }