-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix an off by 1 error in test-float-parse #137948
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: |