Skip to content

Commit

Permalink
Remove drain-on-drop behavior from DrainFilter
Browse files Browse the repository at this point in the history
  • Loading branch information
the8472 committed Nov 15, 2022
1 parent 5d49276 commit 28a13cf
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 53 deletions.
42 changes: 8 additions & 34 deletions src/map.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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.
///
Expand All @@ -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>
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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);
}
}
Expand Down
25 changes: 6 additions & 19 deletions src/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!)
Expand Down Expand Up @@ -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
///
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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");
}
}
Expand Down

0 comments on commit 28a13cf

Please sign in to comment.