Repository navigation
fix panics at newtype_index Step impl - #164891
Closed
illiasheshyn wants to merge 1 commit into
Closed
illiasheshyn wants to merge 1 commit into
illiasheshyn wants to merge 1 commit into
Conversation
- `forward_checked` prevents stepping past `MAX`, which led to panic at `from_usize`, instead of returning `None` - `forward_overflowing` and `backward_overflowing` saturate at 0 and `MAX` instead of wrapping at `usize` boundaries
rustbot
added
S-waiting-on-review
Collaborator
|
Thanks for the pull request, and welcome! The Rust Project has assigned @oli-obk (or someone else) to review your changes, you should hear from them (or someone else) within the next two weeks. Please see the contribution instructions and our LLM policy for more information. Why was this reviewer chosen?The reviewer was selected based on:
|
Contributor
|
considering we never hit those as ICEs, I would rather keep them or make them explicit. I don't think anyone is using those newtype indices in a fallible way that would be relevant here. |
rustbot
removed
the
S-waiting-on-review
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
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.
forward_checked(#69659) only checked forusizewrapping, allowing stepping pastMAXand panicking atfrom_usize. It now returnsNonewhen exceeding the maximum value.Similarly,
forward_overflowingandbackward_overflowing(#155114) exceededMAXand wrapped past zero respectively, reaching the samefrom_usizeassertion. Both now saturate, following precedents atlibrary/core/src/iter/range.rs(e.g.,impl Step for char). Regression tests have been addedLLM disclosure: I've used an LLM to summarise existing github issues, discover the bug and review the solution. The tests were generated by an LLM and edited by me. The solution was chosen and written by me based on existing
Stepimplementations.