-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Fix an off by 1 error in test-float-parse #137948
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
speedy-lex
commented
Mar 3, 2025
rustbot has assigned @Mark-Simulacrum. Use |
r? @tgross35 |
The title had me worried for a minute, but this is in the total estimation :) thanks! @birs r+ rollup |
Whoop, sorry about that birs. @bors r+ rollup |
🌲 The tree is currently closed for pull requests below priority 102. This pull request will be tested once the tree is reopened. |
☔ The latest upstream changes (presumably #137927) made this pull request unmergeable. Please resolve the merge conflicts. |
@tgross35, I feel like this solution is better as it can't panic. I don't think an estimation should panic. |
I think it's actually preferable to panic here - that limit is only hit if attempting to run exhaustive tests on f64 or f128, I'd rather be notified immediately then coming back a few hours later and realizing I started a test that will run for the next 2159123 years :) (I just tried it to get that estimate) |
There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged. You can start a rebase with the following commands:
The following commits are merge commits: |