-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Fix panic
message for BTreeSet
's range
API and document panic
cases
#98039
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @thomcc (or someone else) soon. Please see the contribution instructions for more information. |
I'd like to see the changes mentioned in #98039 (comment). @rustbot label +S-waiting-on-author -S-waiting-on-review |
Can you rebase rather than merge? Or squash the commits? I think we try to avoid merge commits in the history. Other than that this looks great. |
17750ca
to
20cae01
Compare
Looks great, thanks for working through this! @bors r+ |
📌 Commit 774e814 has been approved by |
Sure, rebased and squashed. |
Fix `panic` message for `BTreeSet`'s `range` API and document `panic` cases Currently, the `panic` cases for [`BTreeSet`'s `range` API](https://doc.rust-lang.org/std/collections/struct.BTreeSet.html#method.range) are undocumented and produce a slightly wrong `panic` message (says `BTreeMap` instead of `BTreeSet`). Panic case 1 code: ```rust use std::collections::BTreeSet; use std::ops::Bound::Excluded; fn main() { let mut set = BTreeSet::new(); set.insert(3); set.insert(5); set.insert(8); for &elem in set.range((Excluded(&3), Excluded(&3))) { println!("{elem}"); } } ``` Panic case 1 message: ``` thread 'main' panicked at 'range start and end are equal and excluded in BTreeMap', /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/alloc/src/collections/btree/search.rs:105:17 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ``` Panic case 2 code: ```rust use std::collections::BTreeSet; use std::ops::Bound::Included; fn main() { let mut set = BTreeSet::new(); set.insert(3); set.insert(5); set.insert(8); for &elem in set.range((Included(&8), Included(&3))) { println!("{elem}"); } } ``` Panic case 2: ``` thread 'main' panicked at 'range start is greater than range end in BTreeMap', /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/alloc/src/collections/btree/search.rs:110:17 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ``` This PR fixes the output messages to say `BTreeSet`, adds the relevant unit tests, and updates the documentation for the API.
Rollup of 9 pull requests Successful merges: - rust-lang#91264 (Add macro support in jump to definition feature) - rust-lang#96955 (Remove (transitive) reliance on sorting by DefId in pretty-printer) - rust-lang#97633 (Session object: Set OS/ABI) - rust-lang#98039 (Fix `panic` message for `BTreeSet`'s `range` API and document `panic` cases) - rust-lang#98214 (rustc_target: Remove some redundant target properties) - rust-lang#98280 (Improve suggestion for calling fn-like expr on type mismatch) - rust-lang#98394 (Fixup missing renames from `#[main]` to `#[rustc_main]`) - rust-lang#98411 (Update tendril) - rust-lang#98419 (Remove excess rib while resolving closures) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Currently, the
panic
cases forBTreeSet
'srange
API are undocumented and produce a slightly wrongpanic
message (saysBTreeMap
instead ofBTreeSet
).Panic case 1 code:
Panic case 1 message:
Panic case 2 code:
Panic case 2:
This PR fixes the output messages to say
BTreeSet
, adds the relevant unit tests, and updates the documentation for the API.