Skip to content

Commit 6628f6d

Browse files
committed
Remove drain-on-drop behavior from DrainFilter
1 parent 5d49276 commit 6628f6d

File tree

2 files changed

+13
-53
lines changed

2 files changed

+13
-53
lines changed

src/map.rs

+7-34
Original file line numberDiff line numberDiff line change
@@ -975,12 +975,9 @@ impl<K, V, S, A: Allocator + Clone> HashMap<K, V, S, A> {
975975
/// Note that `drain_filter` lets you mutate every value in the filter closure, regardless of
976976
/// whether you choose to keep or remove it.
977977
///
978-
/// When the returned DrainedFilter is dropped, any remaining elements that satisfy
979-
/// the predicate are dropped from the table.
980-
///
981-
/// It is unspecified how many more elements will be subjected to the closure
982-
/// if a panic occurs in the closure, or a panic occurs while dropping an element,
983-
/// or if the `DrainFilter` value is leaked.
978+
/// If the returned `DrainFilter` is not exhausted, e.g. because it is dropped without iterating
979+
/// or the iteration short-circuits, then the remaining elements will be retained.
980+
/// Use [`retain()`] with a negated predicate if you do not need the returned iterator.
984981
///
985982
/// Keeps the allocated memory for reuse.
986983
///
@@ -1007,9 +1004,8 @@ impl<K, V, S, A: Allocator + Clone> HashMap<K, V, S, A> {
10071004
/// let d = map.drain_filter(|k, _v| k % 2 != 0);
10081005
/// }
10091006
///
1010-
/// // But the map lens have been reduced by half
1011-
/// // even if we do not use DrainFilter iterator.
1012-
/// assert_eq!(map.len(), 4);
1007+
/// // DrainFilter was not exhausted, therefore no elements were drained.
1008+
/// assert_eq!(map.len(), 8);
10131009
/// ```
10141010
#[cfg_attr(feature = "inline-more", inline)]
10151011
pub fn drain_filter<F>(&mut self, f: F) -> DrainFilter<'_, K, V, F, A>
@@ -2739,6 +2735,7 @@ impl<K, V, A: Allocator + Clone> Drain<'_, K, V, A> {
27392735
///
27402736
/// assert_eq!(map.len(), 1);
27412737
/// ```
2738+
#[must_use = "Iterators are lazy unless consumed"]
27422739
pub struct DrainFilter<'a, K, V, F, A: Allocator + Clone = Global>
27432740
where
27442741
F: FnMut(&K, &mut V) -> bool,
@@ -2747,30 +2744,6 @@ where
27472744
inner: DrainFilterInner<'a, K, V, A>,
27482745
}
27492746

2750-
impl<'a, K, V, F, A> Drop for DrainFilter<'a, K, V, F, A>
2751-
where
2752-
F: FnMut(&K, &mut V) -> bool,
2753-
A: Allocator + Clone,
2754-
{
2755-
#[cfg_attr(feature = "inline-more", inline)]
2756-
fn drop(&mut self) {
2757-
while let Some(item) = self.next() {
2758-
let guard = ConsumeAllOnDrop(self);
2759-
drop(item);
2760-
mem::forget(guard);
2761-
}
2762-
}
2763-
}
2764-
2765-
pub(super) struct ConsumeAllOnDrop<'a, T: Iterator>(pub &'a mut T);
2766-
2767-
impl<T: Iterator> Drop for ConsumeAllOnDrop<'_, T> {
2768-
#[cfg_attr(feature = "inline-more", inline)]
2769-
fn drop(&mut self) {
2770-
self.0.for_each(drop);
2771-
}
2772-
}
2773-
27742747
impl<K, V, F, A> Iterator for DrainFilter<'_, K, V, F, A>
27752748
where
27762749
F: FnMut(&K, &mut V) -> bool,
@@ -8159,7 +8132,7 @@ mod test_map {
81598132
}
81608133
{
81618134
let mut map: HashMap<i32, i32> = (0..8).map(|x| (x, x * 10)).collect();
8162-
drop(map.drain_filter(|&k, _| k % 2 == 0));
8135+
map.drain_filter(|&k, _| k % 2 == 0).for_each(drop);
81638136
assert_eq!(map.len(), 4);
81648137
}
81658138
}

src/set.rs

+6-19
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@ use alloc::borrow::ToOwned;
55
use core::fmt;
66
use core::hash::{BuildHasher, Hash};
77
use core::iter::{Chain, FromIterator, FusedIterator};
8-
use core::mem;
98
use core::ops::{BitAnd, BitOr, BitXor, Sub};
109

11-
use super::map::{self, ConsumeAllOnDrop, DefaultHashBuilder, DrainFilterInner, HashMap, Keys};
10+
use super::map::{self, DefaultHashBuilder, DrainFilterInner, HashMap, Keys};
1211
use crate::raw::{Allocator, Global};
1312

1413
// Future Optimization (FIXME!)
@@ -380,8 +379,9 @@ impl<T, S, A: Allocator + Clone> HashSet<T, S, A> {
380379
/// In other words, move all elements `e` such that `f(&e)` returns `true` out
381380
/// into another iterator.
382381
///
383-
/// When the returned DrainedFilter is dropped, any remaining elements that satisfy
384-
/// the predicate are dropped from the set.
382+
/// If the returned `DrainFilter` is not exhausted, e.g. because it is dropped without iterating
383+
/// or the iteration short-circuits, then the remaining elements will be retained.
384+
/// Use [`retain()`] with a negated predicate if you do not need the returned iterator.
385385
///
386386
/// # Examples
387387
///
@@ -1552,6 +1552,7 @@ pub struct Drain<'a, K, A: Allocator + Clone = Global> {
15521552
///
15531553
/// [`drain_filter`]: struct.HashSet.html#method.drain_filter
15541554
/// [`HashSet`]: struct.HashSet.html
1555+
#[must_use = "Iterators are lazy unless consumed"]
15551556
pub struct DrainFilter<'a, K, F, A: Allocator + Clone = Global>
15561557
where
15571558
F: FnMut(&K) -> bool,
@@ -1748,20 +1749,6 @@ impl<K: fmt::Debug, A: Allocator + Clone> fmt::Debug for Drain<'_, K, A> {
17481749
}
17491750
}
17501751

1751-
impl<'a, K, F, A: Allocator + Clone> Drop for DrainFilter<'a, K, F, A>
1752-
where
1753-
F: FnMut(&K) -> bool,
1754-
{
1755-
#[cfg_attr(feature = "inline-more", inline)]
1756-
fn drop(&mut self) {
1757-
while let Some(item) = self.next() {
1758-
let guard = ConsumeAllOnDrop(self);
1759-
drop(item);
1760-
mem::forget(guard);
1761-
}
1762-
}
1763-
}
1764-
17651752
impl<K, F, A: Allocator + Clone> Iterator for DrainFilter<'_, K, F, A>
17661753
where
17671754
F: FnMut(&K) -> bool,
@@ -2840,7 +2827,7 @@ mod test_set {
28402827
}
28412828
{
28422829
let mut set: HashSet<i32> = (0..8).collect();
2843-
drop(set.drain_filter(|&k| k % 2 == 0));
2830+
set.drain_filter(|&k| k % 2 == 0).for_each(drop);
28442831
assert_eq!(set.len(), 4, "Removes non-matching items on drop");
28452832
}
28462833
}

0 commit comments

Comments
 (0)