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.

Sign up for GitHub

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

Jump to bottom

drain_filter has opposite behavior of standard library version #186

Closed
mbrubeck opened this issue Aug 10, 2020 · 1 comment · Fixed by #187
Closed

drain_filter has opposite behavior of standard library version #186

mbrubeck opened this issue Aug 10, 2020 · 1 comment · Fixed by #187

Comments

@mbrubeck
Copy link
Contributor

mbrubeck commented Aug 10, 2020 •
edited
Loading

In methods like std::vec::Vec::drain_filter, the element is removed if the closure returns true. But in hashbrown::HashMap::drain_filter (#135) and hashbrown::HashSet::drain_filter (#179), the element is removed if the closure returns false. 😦

cc rust-lang/rust#59618

Update: This is fixed in hashbrown 0.9.0. When upgrading from earlier versions, all callers must flip the meaning of the predicate passed to the drain_filter method. I have submitted pull requests to all the affected crates I could find.
@Amanieu
Copy link
Member

Amanieu commented Aug 10, 2020

That... seems unfortunate. I assume the proper behavior is the one in libstd and the behavior of hashbrown is incorrect.

This is going to have to be a (silent) breaking change, but that's fine since we can just do a major version bump.

mbrubeck added a commit to mbrubeck/hashbrown that referenced this issue Aug 11, 2020
@mbrubeck
[breaking change] Reverse drain_filter behavior to match libstd
6ea798c 
Fixes rust-lang#186.
@mbrubeck mbrubeck mentioned this issue Aug 11, 2020
Merged
bors added a commit that referenced this issue Aug 11, 2020
@bors
Auto merge of #187 - mbrubeck:drain_filter_reverse, r=Amanieu
9aa0043 
Reverse `drain_filter` behavior to match libstd

Fixes #186.  This is the worst sort of silent breaking change, but we don't have much choice here.
bors added a commit that referenced this issue Aug 11, 2020
@bors
Auto merge of #187 - mbrubeck:drain_filter_reverse, r=Amanieu
0c228e3 
Reverse `drain_filter` behavior to match libstd

Fixes #186.  This is the worst sort of silent breaking change, but we don't have much choice here.
mbrubeck added a commit to mbrubeck/hashbrown that referenced this issue Aug 11, 2020
@mbrubeck
[breaking change] Reverse drain_filter behavior to match libstd
9253606 
Fixes rust-lang#186.
bors added a commit that referenced this issue Aug 11, 2020
@bors
Auto merge of #187 - mbrubeck:drain_filter_reverse, r=Amanieu
8102afb 
Reverse `drain_filter` behavior to match libstd

Fixes #186.  This is the worst sort of silent breaking change, but we don't have much choice here.
bors added a commit that referenced this issue Aug 12, 2020
@bors
Auto merge of #187 - mbrubeck:drain_filter_reverse, r=Amanieu
17e6ecc 
Reverse `drain_filter` behavior to match libstd

Fixes #186.  This is the worst sort of silent breaking change, but we don't have much choice here.
@bors bors closed this as completed in 7654023 Aug 12, 2020
mbrubeck added a commit to mbrubeck/ya-net-p2p that referenced this issue Sep 3, 2020
@mbrubeck
Update to hashbrown 0.9
6370674 
The behavior of the `HashMap::drain_filter` method has changed in this
release.  For details, see:

rust-lang/hashbrown#186
@mbrubeck mbrubeck mentioned this issue Sep 3, 2020
Closed
mbrubeck added a commit to mbrubeck/sorbus that referenced this issue Sep 3, 2020
@mbrubeck
Update to hashbrown 0.9
e34e3d7 
The behavior of `drain_filter` has changed in this release:

rust-lang/hashbrown#186
@mbrubeck mbrubeck mentioned this issue Sep 3, 2020
Merged
mbrubeck added a commit to mbrubeck/lol-html that referenced this issue Sep 3, 2020
@mbrubeck
Update to hashbrown 0.9
81b1c71 
The behavior of the `drain_filter` method has changed in this release:

rust-lang/hashbrown#186
@mbrubeck mbrubeck mentioned this issue Sep 3, 2020
Merged
mbrubeck added a commit to mbrubeck/satsuma that referenced this issue Sep 3, 2020
@mbrubeck
Update to hashbrown 0.9
59d5d75 
The behavior of `drain_filter` has changed in this release:

rust-lang/hashbrown#186
@mbrubeck mbrubeck mentioned this issue Sep 3, 2020
Merged
mbrubeck added a commit to mbrubeck/redshirt that referenced this issue Sep 3, 2020
@mbrubeck
Update to hashbrown 0.9
8d1e96d 
The behavior of `drain_filter` has changed in this release:

rust-lang/hashbrown#186
@mbrubeck mbrubeck mentioned this issue Sep 3, 2020
Merged
bors bot added a commit to CAD97/sorbus that referenced this issue Sep 4, 2020
@bors @mbrubeck
Merge #72
fbb12b5 
72: Update to hashbrown 0.9 r=CAD97 a=mbrubeck

The behavior of `drain_filter` has changed in this release:

rust-lang/hashbrown#186

Co-authored-by: Matt Brubeck <[email protected]>
tomaka pushed a commit to tomaka/redshirt that referenced this issue Sep 4, 2020
@mbrubeck
Update to hashbrown 0.9 (#546)
bde2845 
The behavior of `drain_filter` has changed in this release:

rust-lang/hashbrown#186
inikulin pushed a commit to cloudflare/lol-html that referenced this issue Oct 13, 2020
@mbrubeck
Update to hashbrown 0.9 (#64)
3c53085 
* Update to hashbrown 0.9

The behavior of the `drain_filter` method has changed in this release:

rust-lang/hashbrown#186

* Use HashMap::retain instead of drain_filter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Reverse drain_filter behavior to match libstd mbrubeck/hashbrown
2 participants
@mbrubeck @Amanieu