Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add drain_filter method to HashMap and HashSet #76458

Merged
merged 4 commits into from
Sep 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1259,11 +1259,10 @@ dependencies = [

[[package]]
name = "hashbrown"
version = "0.8.2"
version = "0.9.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e91b62f79061a0bc2e046024cb7ba44b08419ed238ecbd9adbd787434b9e8c25"
checksum = "00d63df3d41950fb462ed38308eea019113ad1508da725bbedcd0fa5a85ef5f7"
dependencies = [
"autocfg",
"compiler_builtins",
"rustc-std-workspace-alloc",
"rustc-std-workspace-core",
Expand Down Expand Up @@ -1401,9 +1400,9 @@ dependencies = [

[[package]]
name = "indexmap"
version = "1.5.1"
version = "1.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "86b45e59b16c76b11bf9738fd5d38879d3bd28ad292d7b313608becb17ae2df9"
checksum = "55e2e4c765aa53a0424761bf9f41aa7a6ac1efa87238f59560640e27fca028f2"
dependencies = [
"autocfg",
"hashbrown",
Expand Down
2 changes: 1 addition & 1 deletion library/std/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ libc = { version = "0.2.74", default-features = false, features = ['rustc-dep-of
compiler_builtins = { version = "0.1.35" }
profiler_builtins = { path = "../profiler_builtins", optional = true }
unwind = { path = "../unwind" }
hashbrown = { version = "0.8.1", default-features = false, features = ['rustc-dep-of-std'] }
hashbrown = { version = "0.9.0", default-features = false, features = ['rustc-dep-of-std'] }

# Dependencies of the `backtrace` crate
addr2line = { version = "0.13.0", optional = true, default-features = false }
Expand Down
99 changes: 93 additions & 6 deletions library/std/src/collections/hash/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,50 @@ impl<K, V, S> HashMap<K, V, S> {
Drain { base: self.base.drain() }
}

/// Creates an iterator which uses a closure to determine if an element should be removed.
///
/// If the closure returns true, the element is removed from the map and yielded.
/// If the closure returns false, or panics, the element remains in the map and will not be
/// yielded.
///
/// Note that `drain_filter` lets you mutate every value in the filter closure, regardless of
/// whether you choose to keep or remove it.
///
/// If the iterator is only partially consumed or not consumed at all, each of the remaining
/// elements will still be subjected to the closure and removed and dropped if it returns true.
///
/// 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.
///
/// # Examples
///
/// Splitting a map into even and odd keys, reusing the original map:
///
/// ```
/// #![feature(hash_drain_filter)]
/// use std::collections::HashMap;
///
/// let mut map: HashMap<i32, i32> = (0..8).map(|x| (x, x)).collect();
/// let drained: HashMap<i32, i32> = map.drain_filter(|k, _v| k % 2 == 0).collect();
///
/// let mut evens = drained.keys().copied().collect::<Vec<_>>();
/// let mut odds = map.keys().copied().collect::<Vec<_>>();
/// evens.sort();
/// odds.sort();
///
/// assert_eq!(evens, vec![0, 2, 4, 6]);
/// assert_eq!(odds, vec![1, 3, 5, 7]);
/// ```
#[inline]
#[unstable(feature = "hash_drain_filter", issue = "59618")]
pub fn drain_filter<F>(&mut self, pred: F) -> DrainFilter<'_, K, V, F>
where
F: FnMut(&K, &mut V) -> bool,
{
DrainFilter { base: self.base.drain_filter(pred) }
}

/// Clears the map, removing all key-value pairs. Keeps the allocated memory
/// for reuse.
///
Expand Down Expand Up @@ -1190,6 +1234,19 @@ impl<'a, K, V> Drain<'a, K, V> {
}
}

/// A draining, filtering iterator over the entries of a `HashMap`.
///
/// This `struct` is created by the [`drain_filter`] method on [`HashMap`].
///
/// [`drain_filter`]: HashMap::drain_filter
#[unstable(feature = "hash_drain_filter", issue = "59618")]
pub struct DrainFilter<'a, K, V, F>
where
F: FnMut(&K, &mut V) -> bool,
{
base: base::DrainFilter<'a, K, V, F>,
}

/// A mutable iterator over the values of a `HashMap`.
///
/// This `struct` is created by the [`values_mut`] method on [`HashMap`]. See its
Expand Down Expand Up @@ -1247,16 +1304,16 @@ pub struct RawEntryBuilderMut<'a, K: 'a, V: 'a, S: 'a> {
#[unstable(feature = "hash_raw_entry", issue = "56167")]
pub enum RawEntryMut<'a, K: 'a, V: 'a, S: 'a> {
/// An occupied entry.
Occupied(RawOccupiedEntryMut<'a, K, V>),
Occupied(RawOccupiedEntryMut<'a, K, V, S>),
/// A vacant entry.
Vacant(RawVacantEntryMut<'a, K, V, S>),
}

/// A view into an occupied entry in a `HashMap`.
/// It is part of the [`RawEntryMut`] enum.
#[unstable(feature = "hash_raw_entry", issue = "56167")]
pub struct RawOccupiedEntryMut<'a, K: 'a, V: 'a> {
base: base::RawOccupiedEntryMut<'a, K, V>,
pub struct RawOccupiedEntryMut<'a, K: 'a, V: 'a, S: 'a> {
base: base::RawOccupiedEntryMut<'a, K, V, S>,
}

/// A view into a vacant entry in a `HashMap`.
Expand Down Expand Up @@ -1457,7 +1514,7 @@ impl<'a, K, V, S> RawEntryMut<'a, K, V, S> {
}
}

impl<'a, K, V> RawOccupiedEntryMut<'a, K, V> {
impl<'a, K, V, S> RawOccupiedEntryMut<'a, K, V, S> {
/// Gets a reference to the key in the entry.
#[inline]
#[unstable(feature = "hash_raw_entry", issue = "56167")]
Expand Down Expand Up @@ -1597,7 +1654,7 @@ impl<K: Debug, V: Debug, S> Debug for RawEntryMut<'_, K, V, S> {
}

#[unstable(feature = "hash_raw_entry", issue = "56167")]
impl<K: Debug, V: Debug> Debug for RawOccupiedEntryMut<'_, K, V> {
impl<K: Debug, V: Debug, S> Debug for RawOccupiedEntryMut<'_, K, V, S> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("RawOccupiedEntryMut")
.field("key", self.key())
Expand Down Expand Up @@ -1990,6 +2047,36 @@ where
}
}

#[unstable(feature = "hash_drain_filter", issue = "59618")]
impl<K, V, F> Iterator for DrainFilter<'_, K, V, F>
where
F: FnMut(&K, &mut V) -> bool,
{
type Item = (K, V);

#[inline]
fn next(&mut self) -> Option<(K, V)> {
self.base.next()
}
#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
self.base.size_hint()
}
}

#[unstable(feature = "hash_drain_filter", issue = "59618")]
impl<K, V, F> FusedIterator for DrainFilter<'_, K, V, F> where F: FnMut(&K, &mut V) -> bool {}

#[unstable(feature = "hash_drain_filter", issue = "59618")]
impl<'a, K, V, F> fmt::Debug for DrainFilter<'a, K, V, F>
where
F: FnMut(&K, &mut V) -> bool,
{
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.pad("DrainFilter { .. }")
}
}

impl<'a, K, V> Entry<'a, K, V> {
#[stable(feature = "rust1", since = "1.0.0")]
/// Ensures a value is in the entry by inserting the default if empty, and returns
Expand Down Expand Up @@ -2698,7 +2785,7 @@ fn map_entry<'a, K: 'a, V: 'a>(raw: base::RustcEntry<'a, K, V>) -> Entry<'a, K,
}

#[inline]
fn map_try_reserve_error(err: hashbrown::TryReserveError) -> TryReserveError {
pub(super) fn map_try_reserve_error(err: hashbrown::TryReserveError) -> TryReserveError {
match err {
hashbrown::TryReserveError::CapacityOverflow => TryReserveError::CapacityOverflow,
hashbrown::TryReserveError::AllocError { layout } => {
Expand Down
161 changes: 161 additions & 0 deletions library/std/src/collections/hash/map/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -924,3 +924,164 @@ fn test_raw_entry() {
}
}
}

mod test_drain_filter {
use super::*;

use crate::panic::{catch_unwind, AssertUnwindSafe};
use crate::sync::atomic::{AtomicUsize, Ordering};

trait EqSorted: Iterator {
fn eq_sorted<I: IntoIterator<Item = Self::Item>>(self, other: I) -> bool;
}

impl<T: Iterator> EqSorted for T
where
T::Item: Eq + Ord,
{
fn eq_sorted<I: IntoIterator<Item = Self::Item>>(self, other: I) -> bool {
let mut v: Vec<_> = self.collect();
v.sort_unstable();
v.into_iter().eq(other)
}
}

#[test]
fn empty() {
let mut map: HashMap<i32, i32> = HashMap::new();
map.drain_filter(|_, _| unreachable!("there's nothing to decide on"));
assert!(map.is_empty());
}

#[test]
fn consuming_nothing() {
let pairs = (0..3).map(|i| (i, i));
let mut map: HashMap<_, _> = pairs.collect();
assert!(map.drain_filter(|_, _| false).eq_sorted(crate::iter::empty()));
assert_eq!(map.len(), 3);
}

#[test]
fn consuming_all() {
let pairs = (0..3).map(|i| (i, i));
let mut map: HashMap<_, _> = pairs.clone().collect();
assert!(map.drain_filter(|_, _| true).eq_sorted(pairs));
assert!(map.is_empty());
}

#[test]
fn mutating_and_keeping() {
let pairs = (0..3).map(|i| (i, i));
let mut map: HashMap<_, _> = pairs.collect();
assert!(
map.drain_filter(|_, v| {
*v += 6;
false
})
.eq_sorted(crate::iter::empty())
);
assert!(map.keys().copied().eq_sorted(0..3));
assert!(map.values().copied().eq_sorted(6..9));
}

#[test]
fn mutating_and_removing() {
let pairs = (0..3).map(|i| (i, i));
let mut map: HashMap<_, _> = pairs.collect();
assert!(
map.drain_filter(|_, v| {
*v += 6;
true
})
.eq_sorted((0..3).map(|i| (i, i + 6)))
);
assert!(map.is_empty());
}

#[test]
fn drop_panic_leak() {
static PREDS: AtomicUsize = AtomicUsize::new(0);
static DROPS: AtomicUsize = AtomicUsize::new(0);

struct D;
impl Drop for D {
fn drop(&mut self) {
if DROPS.fetch_add(1, Ordering::SeqCst) == 1 {
panic!("panic in `drop`");
}
}
}

let mut map = (0..3).map(|i| (i, D)).collect::<HashMap<_, _>>();

catch_unwind(move || {
drop(map.drain_filter(|_, _| {
PREDS.fetch_add(1, Ordering::SeqCst);
true
}))
})
.unwrap_err();

assert_eq!(PREDS.load(Ordering::SeqCst), 3);
assert_eq!(DROPS.load(Ordering::SeqCst), 3);
}

#[test]
fn pred_panic_leak() {
static PREDS: AtomicUsize = AtomicUsize::new(0);
static DROPS: AtomicUsize = AtomicUsize::new(0);

struct D;
impl Drop for D {
fn drop(&mut self) {
DROPS.fetch_add(1, Ordering::SeqCst);
}
}

let mut map = (0..3).map(|i| (i, D)).collect::<HashMap<_, _>>();

catch_unwind(AssertUnwindSafe(|| {
drop(map.drain_filter(|_, _| match PREDS.fetch_add(1, Ordering::SeqCst) {
0 => true,
_ => panic!(),
}))
}))
.unwrap_err();

assert_eq!(PREDS.load(Ordering::SeqCst), 2);
assert_eq!(DROPS.load(Ordering::SeqCst), 1);
assert_eq!(map.len(), 2);
}

// Same as above, but attempt to use the iterator again after the panic in the predicate
#[test]
fn pred_panic_reuse() {
static PREDS: AtomicUsize = AtomicUsize::new(0);
static DROPS: AtomicUsize = AtomicUsize::new(0);

struct D;
impl Drop for D {
fn drop(&mut self) {
DROPS.fetch_add(1, Ordering::SeqCst);
}
}

let mut map = (0..3).map(|i| (i, D)).collect::<HashMap<_, _>>();

{
let mut it = map.drain_filter(|_, _| match PREDS.fetch_add(1, Ordering::SeqCst) {
0 => true,
_ => panic!(),
});
catch_unwind(AssertUnwindSafe(|| while it.next().is_some() {})).unwrap_err();
// Iterator behaviour after a panic is explicitly unspecified,
// so this is just the current implementation:
let result = catch_unwind(AssertUnwindSafe(|| it.next()));
assert!(result.is_err());
}

assert_eq!(PREDS.load(Ordering::SeqCst), 3);
assert_eq!(DROPS.load(Ordering::SeqCst), 1);
assert_eq!(map.len(), 2);
}
}
Loading