Document panic in RangeInclusive::from(legacy::RangeInclusive)#155421
Conversation
|
r? @jhpratt rustbot has assigned @jhpratt. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
Has this been approved by T-libs-api? The panic message clearly says unspecified, so adding this documentation would overrule that. |
|
If T-libs doesn't want to guarantee this, I can change the wording to "currently panics, may not panic in the future". Or even "may panic". |
This comment has been minimized.
This comment has been minimized.
766773c to
f43e3e7
Compare
This comment has been minimized.
This comment has been minimized.
|
ping @tbu-, CI is failing. You need to add |
f43e3e7 to
73b9155
Compare
This comment has been minimized.
This comment has been minimized.
73b9155 to
70df721
Compare
|
Thanks for the ping, @Amanieu. I somehow forgot about this PR. |
This comment has been minimized.
This comment has been minimized.
70df721 to
6502d57
Compare
This comment has been minimized.
This comment has been minimized.
6502d57 to
5a00cc6
Compare
|
Note: The current status of the PR adds a new stable guarantee that the conversion will panic for an exhausted iterator. |
|
Nominating given the implicit stability guarantee. |
This comment has been minimized.
This comment has been minimized.
|
@rfcbot merge libs-api |
|
@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. |
This comment has been minimized.
This comment has been minimized.
|
This pull request was unapproved. |
4ff75dc to
48d2671
Compare
|
@bors r=jhpratt |
This comment has been minimized.
This comment has been minimized.
|
A job failed! Check out the build log: (web) (plain enhanced) (plain) Click to see the possible cause of the failure (guessed by this bot) |
|
@bors retry |
|
❗ You can only retry pull requests that are approved and have a previously failed auto build. Hint: There is currently a pending auto build on this PR. To cancel it, run |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing a774017 (parent) -> 40f92b3 (this PR) Test differencesShow 32 test diffs32 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 40f92b37c4f7566de7348cec4d4c2a350da598d5 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (40f92b3): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis perf run didn't have relevant results for this metric. Max RSS (memory usage)Results (primary -4.0%, secondary 2.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 482.353s -> 479.892s (-0.51%) |
…=JohnTitor Weaken guarantee for `From<legacy::RangeInclusive> for RangeInclusive` As per rust-lang#155114 (comment), this `From` impl no longer guarantees panicking for exhausted iterators. Instead, it only guarantees that the conversion will either panic or produce an empty range. This is done so that we can optimize the implementation of `legacy::RangeInclusive` in a way such that we cannot check if it has been exhausted in a generic context without a `Step` and/or `PartialOrd` bound. If this PR and/or rust-lang#155114 merges after the 1.98.0 beta branch promotion from the main branch, then this will need a beta backport, since this PR changes the stable guarantee previously made in rust-lang#155421. This PR conflicts with and/or blocks rust-lang#155114.
…=JohnTitor Weaken guarantee for `From<legacy::RangeInclusive> for RangeInclusive` As per rust-lang#155114 (comment), this `From` impl no longer guarantees panicking for exhausted iterators. Instead, it only guarantees that the conversion will either panic or produce an empty range. This is done so that we can optimize the implementation of `legacy::RangeInclusive` in a way such that we cannot check if it has been exhausted in a generic context without a `Step` and/or `PartialOrd` bound. If this PR and/or rust-lang#155114 merges after the 1.98.0 beta branch promotion from the main branch, then this will need a beta backport, since this PR changes the stable guarantee previously made in rust-lang#155421. This PR conflicts with and/or blocks rust-lang#155114.
…=JohnTitor Weaken guarantee for `From<legacy::RangeInclusive> for RangeInclusive` As per rust-lang#155114 (comment), this `From` impl no longer guarantees panicking for exhausted iterators. Instead, it only guarantees that the conversion will either panic or produce an empty range. This is done so that we can optimize the implementation of `legacy::RangeInclusive` in a way such that we cannot check if it has been exhausted in a generic context without a `Step` and/or `PartialOrd` bound. If this PR and/or rust-lang#155114 merges after the 1.98.0 beta branch promotion from the main branch, then this will need a beta backport, since this PR changes the stable guarantee previously made in rust-lang#155421. This PR conflicts with and/or blocks rust-lang#155114.
Rollup merge of #158770 - theemathas:range-inclusive-from, r=JohnTitor Weaken guarantee for `From<legacy::RangeInclusive> for RangeInclusive` As per #155114 (comment), this `From` impl no longer guarantees panicking for exhausted iterators. Instead, it only guarantees that the conversion will either panic or produce an empty range. This is done so that we can optimize the implementation of `legacy::RangeInclusive` in a way such that we cannot check if it has been exhausted in a generic context without a `Step` and/or `PartialOrd` bound. If this PR and/or #155114 merges after the 1.98.0 beta branch promotion from the main branch, then this will need a beta backport, since this PR changes the stable guarantee previously made in #155421. This PR conflicts with and/or blocks #155114.
View all comments