Consistent trait bounds for ExtractIf Debug impls #139764
Open
+48
−39
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
|
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
|
Marking as draft until rust-lang/hashbrown#616 ends up in a |
This comment has been minimized.
This comment has been minimized.
This was referenced Apr 13, 2025
dtolnay
added
T-libs-api
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 28, 2025
[beta] Delay `hash_extract_if` stabilization from 1.87 to 1.88 This PR is a revert of: - rust-lang#134655 for use in the event that we are unable to get rust-lang#139764 into 1.87 (current beta).
|
hashbrown update may have an impact on perf. @bors rollup=never r=me once tests pass |
VlaDexa
added a commit
to VlaDexa/rust
that referenced
this pull request
May 2, 2025
Update hashbrown dependency to unblock ExtractIf improvements Release notes: https://github.com/rust-lang/hashbrown/releases/tag/v0.15.3 Relevant to me, this release includes rust-lang/hashbrown#616 which unblocks rust-lang#139764.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 3, 2025
Update hashbrown dependency to unblock ExtractIf improvements Release notes: https://github.com/rust-lang/hashbrown/releases/tag/v0.15.3 Relevant to me, this release includes rust-lang/hashbrown#616 which unblocks rust-lang#139764.
github-actions bot
pushed a commit
to LegNeato/miri
that referenced
this pull request
May 4, 2025
Update hashbrown dependency to unblock ExtractIf improvements Release notes: https://github.com/rust-lang/hashbrown/releases/tag/v0.15.3 Relevant to me, this release includes rust-lang/hashbrown#616 which unblocks rust-lang/rust#139764.
This comment has been minimized.
This comment has been minimized.
dtolnay
force-pushed
the
extractif
branch
2 times, most recently
from
4543ce5 to
8ddda56
Compare
This comment has been minimized.
This comment has been minimized.
|
@rust-lang/libs-api: This PR alters 2 recent stabilizations, the Both stabilization PRs landed in nightly for 1.87.0: #137109 and #134655. The second one ( This PR makes breaking changes to Refer to a summary of the API changes in #139764 (comment) that is more readable than the content of the PR. If someone would prefer, we have time to revert #137109 in beta, either out of an abundance of caution or so that |
|
Team member @dtolnay 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
added
the
proposed-final-comment-period
rfcbot
added
disposition-merge
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
github-actions bot
pushed a commit
to rust-lang/rustc-dev-guide
that referenced
this pull request
May 5, 2025
Update hashbrown dependency to unblock ExtractIf improvements Release notes: https://github.com/rust-lang/hashbrown/releases/tag/v0.15.3 Relevant to me, this release includes rust-lang/hashbrown#616 which unblocks rust-lang/rust#139764.
|
Beta for 1.88.0 branches from master on May 9, so if this PR does not merge within 4 days, it is going to need a beta backport at least for the With that in mind, I am short-circuiting the final comment period. This proposed change has already been discussed across at least 3 standard library API meetings and in #137654. Please still provide blocking feedback if you have it, and we can deal with that in beta. @bors r=Amanieu |
bors
added
S-waiting-on-bors
lnicola
pushed a commit
to lnicola/rust-analyzer
that referenced
this pull request
May 5, 2025
Update hashbrown dependency to unblock ExtractIf improvements Release notes: https://github.com/rust-lang/hashbrown/releases/tag/v0.15.3 Relevant to me, this release includes rust-lang/hashbrown#616 which unblocks rust-lang/rust#139764.
|
The job Click to see the possible cause of the failure (guessed by this bot) |
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this pull request
May 5, 2025
Consistent trait bounds for ExtractIf Debug impls Closes rust-lang#137654. Refer to that issue for a table of the **4** different impl signatures we previously had in the standard library for Debug impls of various ExtractIf iterator types. The one we are standardizing on is the one so far only used by `alloc::collections::linked_list::ExtractIf`, which is _no_ `F: Debug` bound, _no_ `F: FnMut` bound, only `T: Debug` bound. This PR applies the following signature changes: ```diff /* alloc::collections::btree_map */ pub struct ExtractIf<'a, K, V, F, A = Global> where - F: 'a + FnMut(&K, &mut V) -> bool, Allocator + Clone, impl Debug for ExtractIf<'a, K, V, F, + A, > where K: Debug, V: Debug, - F: FnMut(&K, &mut V) -> bool, + A: Allocator + Clone, ``` ```diff /* alloc::collections::btree_set */ pub struct ExtractIf<'a, T, F, A = Global> where - T: 'a, - F: 'a + FnMut(&T) -> bool, Allocator + Clone, impl Debug for ExtractIf<'a, T, F, A> where T: Debug, - F: FnMut(&T) -> bool, A: Allocator + Clone, ``` ```diff /* alloc::collections::linked_list */ impl Debug for ExtractIf<'a, T, F, + A, > where T: Debug, + A: Allocator, ``` ```diff /* alloc::vec */ impl Debug for ExtractIf<'a, T, F, A> where T: Debug, - F: Debug, A: Allocator, - A: Debug, ``` ```diff /* std::collections::hash_map */ pub struct ExtractIf<'a, K, V, F> where - F: FnMut(&K, &mut V) -> bool, impl Debug for ExtractIf<'a, K, V, F> where + K: Debug, + V: Debug, - F: FnMut(&K, &mut V) -> bool, ``` ```diff /* std::collections::hash_set */ pub struct ExtractIf<'a, T, F> where - F: FnMut(&T) -> bool, impl Debug for ExtractIf<'a, T, F> where + T: Debug, - F: FnMut(&T) -> bool, ``` I have made the following changes to bring these types into better alignment with one another. - Delete `F: Debug` bounds. These are especially problematic because Rust closures do not come with a Debug impl, rendering the impl useless. - Delete `A: Debug` bounds. Allocator parameters are unstable for now, but in the future this would become an API commitment that we do not debug-print a representation of the allocator when printing an iterator. - Delete `F: FnMut` bounds. Requires `hashbrown` PR: rust-lang/hashbrown#616. **API commitment:** we commit to not doing RefCell voodoo inside ExtractIf to have some way for its Debug impl (which takes &self) to call a FnMut closure, if this is even possible. - Add `T: Debug` bounds (or `K`/`V`), even on Debug impls that do not currently make use of them, but might in the future. **Breaking change.** Must backport into Rust 1.87 (current beta) or do a de-stabilization PR in beta to delay those types by one release. - Render using `debug_struct` + `finish_non_exhaustive`, instead of `debug_tuple`. - Do not render the _entire_ underlying collection. - Show a "peek" field indicating the current position of the iterator.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 5, 2025
…llaumeGomez Rollup of 11 pull requests Successful merges: - rust-lang#139764 (Consistent trait bounds for ExtractIf Debug impls) - rust-lang#140035 (Implement RFC 3503: frontmatters) - rust-lang#140080 (mir-opt: Use one MirPatch in MatchBranchSimplification) - rust-lang#140115 (mir-opt: execute MatchBranchSimplification after GVN) - rust-lang#140357 (bypass linker configuration and cross target check on `x check`) - rust-lang#140374 (Resolve instance for SymFn in global/naked asm) - rust-lang#140393 (std: get rid of `sys_common::process`) - rust-lang#140532 (Fix RustAnalyzer discovery of rustc's `stable_mir` crate) - rust-lang#140559 (Removing rustc_type_ir in the rustc_infer codebase) - rust-lang#140636 (implement `PanicTracker` to track `t` panics) - rust-lang#140661 (Make `-Zfixed-x18` into a target modifier) r? `@ghost` `@rustbot` modify labels: rollup
|
@bors r- |
bors
added
S-waiting-on-author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #137654. Refer to that issue for a table of the 4 different impl signatures we previously had in the standard library for Debug impls of various ExtractIf iterator types.
The one we are standardizing on is the one so far only used by
alloc::collections::linked_list::ExtractIf, which is noF: Debugbound, noF: FnMutbound, onlyT: Debugbound.This PR applies the following signature changes:
/* alloc::collections::btree_map */ pub struct ExtractIf<'a, K, V, F, A = Global> where - F: 'a + FnMut(&K, &mut V) -> bool, Allocator + Clone, impl Debug for ExtractIf<'a, K, V, F, + A, > where K: Debug, V: Debug, - F: FnMut(&K, &mut V) -> bool, + A: Allocator + Clone,/* alloc::collections::btree_set */ pub struct ExtractIf<'a, T, F, A = Global> where - T: 'a, - F: 'a + FnMut(&T) -> bool, Allocator + Clone, impl Debug for ExtractIf<'a, T, F, A> where T: Debug, - F: FnMut(&T) -> bool, A: Allocator + Clone,/* alloc::collections::linked_list */ impl Debug for ExtractIf<'a, T, F, + A, > where T: Debug, + A: Allocator,/* alloc::vec */ impl Debug for ExtractIf<'a, T, F, A> where T: Debug, - F: Debug, A: Allocator, - A: Debug,/* std::collections::hash_map */ pub struct ExtractIf<'a, K, V, F> where - F: FnMut(&K, &mut V) -> bool, impl Debug for ExtractIf<'a, K, V, F> where + K: Debug, + V: Debug, - F: FnMut(&K, &mut V) -> bool,/* std::collections::hash_set */ pub struct ExtractIf<'a, T, F> where - F: FnMut(&T) -> bool, impl Debug for ExtractIf<'a, T, F> where + T: Debug, - F: FnMut(&T) -> bool,I have made the following changes to bring these types into better alignment with one another.
Delete
F: Debugbounds. These are especially problematic because Rust closures do not come with a Debug impl, rendering the impl useless.Delete
A: Debugbounds. Allocator parameters are unstable for now, but in the future this would become an API commitment that we do not debug-print a representation of the allocator when printing an iterator.Delete
F: FnMutbounds. RequireshashbrownPR: Drop FnMut trait bounds from ExtractIf data structures hashbrown#616. API commitment: we commit to not doing RefCell voodoo inside ExtractIf to have some way for its Debug impl (which takes &self) to call a FnMut closure, if this is even possible.Add
T: Debugbounds (orK/V), even on Debug impls that do not currently make use of them, but might in the future. Breaking change. Must backport into Rust 1.87 (current beta) or do a de-stabilization PR in beta to delay those types by one release.Render using
debug_struct+finish_non_exhaustive, instead ofdebug_tuple.Do not render the entire underlying collection.
Show a "peek" field indicating the current position of the iterator.