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

Tracking issue for HashMap::extract_if and HashSet::extract_if #59618

Closed
DutchGhost opened this issue Apr 1, 2019 · 20 comments · Fixed by #134655
Closed

Tracking issue for HashMap::extract_if and HashSet::extract_if #59618

DutchGhost opened this issue Apr 1, 2019 · 20 comments · Fixed by #134655
Labels
A-collections Area: `std::collections` C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting

Comments

@DutchGhost
Copy link
Contributor

DutchGhost commented Apr 1, 2019

The feature gate for the issue is #![feature(hash_extract_if)] (previously hash_drain_filter)

Currently only Vec and LinkedList have a drain_filter method, while other collections such as HashMap and HashSet do not.

This means that currently, removing items from a collection, and getting ownership of those items is fairly...unidiomatic and cumbersome.

For references, see rust-lang/rfcs#2140

pub mod collections {
    pub mod hash_map {
        impl<K, V, S> HashMap<K, V, S> {
            pub fn extract_if<F>(&mut self, pred: F) -> ExtractIf<'_, K, V, F>
            where
                F: FnMut(&K, &mut V) -> bool;
        }

        pub struct ExtractIf<'a, K, V, F>
        where
            F: FnMut(&K, &mut V) -> bool;

        impl<K, V, F> Iterator for ExtractIf<'_, K, V, F>
        where
            F: FnMut(&K, &mut V) -> bool;

        impl<K, V, F> FusedIterator for ExtractIf<'_, K, V, F>
        where
            F: FnMut(&K, &mut V) -> bool;

        impl<K, V, F> Debug for ExtractIf<'_, K, V, F>
        where
            F: FnMut(&K, &mut V) -> bool;
    }

    pub mod hash_set {
        impl<T, S> HashSet<T, S> {
            pub fn extract_if<F>(&mut self, pred: F) -> ExtractIf<'_, T, F>
            where
                F: FnMut(&T) -> bool;
        }

        pub struct ExtractIf<'a, K, F>
        where
            F: FnMut(&K) -> bool;

        impl<K, F> Iterator for ExtractIf<'_, K, F>
        where
            F: FnMut(&K) -> bool;

        impl<K, F> FusedIterator for ExtractIf<'_, K, F>
        where
            F: FnMut(&K) -> bool;

        impl<K, F> Debug for ExtractIf<'_, K, F>
        where
            F: FnMut(&K) -> bool;
    }
}

Implementation History

@jonas-schievink jonas-schievink added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. A-collections Area: `std::collections` labels Apr 1, 2019
@hellow554
Copy link
Contributor

Related #42849

@mbrubeck
Copy link
Contributor

mbrubeck commented Jul 9, 2020

This may be easy to implement since drain_filter was added to the underlying hashbrown types in rust-lang/hashbrown#135 and rust-lang/hashbrown#179. (Requires hashbrown 0.8.1 or later; see #70052.)

@mbrubeck mbrubeck changed the title Add drain_filter to HashMap, HashSet, and other collections Add drain_filter to HashMap and HashSet Aug 11, 2020
@mbrubeck
Copy link
Contributor

mbrubeck commented Aug 11, 2020

I have implemented this on a branch, but it will need to wait for a new release of the hashbrown crate because it depends on rust-lang/hashbrown#185, rust-lang/hashbrown#187, and rust-lang/hashbrown#188:

master...mbrubeck:hash_drain_filter

mbrubeck added a commit to mbrubeck/rust that referenced this issue Sep 9, 2020
tmandry added a commit to tmandry/rust that referenced this issue Sep 10, 2020
Add drain_filter method to HashMap and HashSet

Add `HashMap::drain_filter` and `HashSet::drain_filter`, implementing part of rust-lang/rfcs#2140.  These new methods are unstable.  The tracking issue is rust-lang#59618.

The added iterators behave the same as `BTreeMap::drain_filter` and `BTreeSet::drain_filter`, except their iteration order is arbitrary.  The unit tests are adapted from `alloc::collections::btree`.

This branch rewrites `HashSet` to be a wrapper around `hashbrown::HashSet` rather than `std::collections::HashMap`.
 (Both are themselves wrappers around `hashbrown::HashMap`, so the in-memory representation is the same either way.)  This lets `std` re-use more iterator code from `hashbrown`.  Without this change, we would need to duplicate much more code to implement `HashSet::drain_filter`.

This branch also updates the `hashbrown` crate to version 0.9.0.  Aside from changes related to the `DrainFilter` iterators, this version only changes features that are not used in libstd or rustc.  And it updates `indexmap` to version 1.6.0, whose only change is compatibility with `hashbrown` 0.9.0.
@mbrubeck mbrubeck changed the title Add drain_filter to HashMap and HashSet Tracking issue for HashMap::drain_filter and HashSet::drain_filter (hash_drain_filter) Sep 10, 2020
@mbrubeck
Copy link
Contributor

This is now implemented in nightly behind the unstable hash_drain_filter feature:

@benesch
Copy link
Contributor

benesch commented Dec 29, 2020

Any reason not to stabilize this @mbrubeck?

@mbrubeck
Copy link
Contributor

There are some open questions about naming and API design; see the summary in #43244 (comment) for some details.

@blagowtf
Copy link

@the8472 Nice. How often are these releases (and do they automatically land in std?) Is the vision ultimately to stabilize the extract_if version in std::collections::HashMap.

@Folyd
Copy link
Contributor

Folyd commented Jun 4, 2023

The new version of hashbrown will be released soon. 🎉
rust-lang/hashbrown#434

bors added a commit to rust-lang-ci/rust that referenced this issue Jun 15, 2023
Don't drain-on-drop in DrainFilter impls of various collections.

This removes drain-on-drop behavior from various unstable DrainFilter impls (not yet for HashSet/Map) because that behavior [is problematic](rust-lang#43244 (comment)) (because it can lead to panic-in-drop when user closures panic) and may become forbidden if [this draft RFC passes](rust-lang/rfcs#3288).

closes rust-lang#101122

[ACP](rust-lang/libs-team#136)

affected tracking issues
* rust-lang#43244
* rust-lang#70530
* rust-lang#59618

Related hashbrown update: rust-lang/hashbrown#374
@Nemo157 Nemo157 changed the title Tracking issue for HashMap::drain_filter and HashSet::drain_filter (hash_drain_filter) Tracking issue for HashMap::extract_if and HashSet::extract_if Jun 16, 2023
@saiintbrisson
Copy link
Contributor

saiintbrisson commented Sep 14, 2023

Is this ready for the FCP? hashbrown released Jun 5.

@the8472
Copy link
Member

the8472 commented Sep 14, 2023

On the companion Vec issue some people are asking for a mapped version (passing an owned value) instead so if we implemented that it might make sense to do that for hashmap too.

But that could be discussed as part of the FCP.

@Dylan-DPC Dylan-DPC added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Dec 4, 2023
@esemeniuc
Copy link

Would love to use this feature in stable!

@joshtriplett
Copy link
Member

Let's see if we have consensus to stabilize these:

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Nov 19, 2024

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Nov 19, 2024
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Feb 11, 2025
@rfcbot
Copy link

rfcbot commented Feb 11, 2025

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Feb 21, 2025
@rfcbot
Copy link

rfcbot commented Feb 21, 2025

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@bors bors closed this as completed in 23e1132 Feb 25, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 25, 2025
Rollup merge of rust-lang#134655 - GrigorenkoPV:hash_extract_if, r=cuviper

Stabilize `hash_extract_if`

FCP complete: rust-lang#59618 (comment)

Tracking issue: rust-lang#59618
Closes rust-lang#59618
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Feb 26, 2025
Stabilize `hash_extract_if`

FCP complete: rust-lang/rust#59618 (comment)

Tracking issue: #59618
Closes #59618
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-collections Area: `std::collections` C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects
None yet
Development

Successfully merging a pull request may close this issue.