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

Remove needless unsafety from BTreeMap::drain_filter #74677

Merged
merged 1 commit into from
Jul 24, 2020

Conversation

ssomers
Copy link
Contributor

@ssomers ssomers commented Jul 23, 2020

Remove one piece of unsafe code in the iteration over the iterator returned by BTreeMap::drain_filter.

  • Changes an explicitly unspecified part of the API: when the user-supplied predicate (or some of BTreeMap's code) panicked, and the caller tries to use the iterator again, we no longer offer the same key/value pair to the predicate again but pretend the iterator has finished. Note that Miri does not find UB in the test case added here with the unsafe code (or without).
  • Makes the code a little easier on the eyes.
  • Makes the code a little harder on the CPU:
benchcmp c0 c2 --threshold 3
 name                                         c0 ns/iter  c2 ns/iter  diff ns/iter  diff %  speedup
 btree::set::clone_100_and_drain_all          2,794       2,900                106   3.79%   x 0.96
 btree::set::clone_100_and_drain_half         2,604       2,964                360  13.82%   x 0.88
 btree::set::clone_10k_and_drain_half         287,770     322,755           34,985  12.16%   x 0.89

r? @Amanieu

@Dylan-DPC-zz
Copy link

r? @KodrAus

@Amanieu
Copy link
Member

Amanieu commented Jul 23, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jul 23, 2020

📌 Commit facc46f has been approved by Amanieu

@bors
Copy link
Contributor

bors commented Jul 23, 2020

🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 23, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 24, 2020
Rollup of 12 pull requests

Successful merges:

 - rust-lang#74361 (Improve doc theme logo display)
 - rust-lang#74504 (Add right border bar to Dark and Light theme)
 - rust-lang#74572 (Internally unify rustc_deprecated and deprecated)
 - rust-lang#74601 (Clean up E0724 explanation)
 - rust-lang#74623 (polymorphize GlobalAlloc::Function)
 - rust-lang#74665 (Don't ICE on unconstrained anonymous lifetimes inside associated types.)
 - rust-lang#74666 (More BTreeMap test cases, some exposing undefined behaviour)
 - rust-lang#74669 (Fix typo)
 - rust-lang#74677 (Remove needless unsafety from BTreeMap::drain_filter)
 - rust-lang#74680 (Add missing backticks in diagnostics note)
 - rust-lang#74694 (Clean up E0727 explanation)
 - rust-lang#74703 (Fix ICE while building MIR with type errors)

Failed merges:

r? @ghost
@bors bors merged commit fab9b1d into rust-lang:master Jul 24, 2020
@ssomers ssomers deleted the btree_cleanup_2 branch July 24, 2020 15:50
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants