⚡ Sangsakala Proxy 🏠 Home
Skip to content

Fix integer-to-float overflow rounding boundary - #9584

Merged
youknowone merged 2 commits into
RustPython:mainfrom
youknowdot:fix/int-float-rounding-20261103
Nov 4, 2026
Merged

youknowone merged 2 commits into
RustPython:mainfrom
youknowdot:fix/int-float-rounding-20261103

Conversation

@youknowdot

@youknowdot youknowdot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Keep the result of nearest rational-to-float rounding instead of converting every inexact maximum-finite result into infinity. A quotient just beyond the largest finite float should still round to it until the halfway overflow threshold.
  • Apply the same boundary behavior to float(int): retain the existing fast conversion and retry only when malachite-bigint reports the inexact finite-extreme case as None.
  • Both signs and the midpoint-to-infinity case follow the pinned malachite implementation's ties-to-even rounding. The VM's existing infinity-to-OverflowError conversion remains in place.

Validation

  • Independently source-reviewed against CPython 3.15 long_true_divide, integer-to-double conversion, and the pinned malachite 0.14.0 implementations. In particular, the dependency implementation was checked rather than relying on its description of overflow.
  • Normal commit hooks, prek run --files crates/common/src/int.rs, and git diff --check passed.
  • Native RustPython compilation and tests were not run locally. On the prior exact head, hosted Linux, macOS and Windows each reported LongTest.test_float_conversion as an unexpected success in both the initial run and rerun. The update_lib auto-mark function removed only that expected-failure decorator; the IEEE guard and assertions are unchanged. New-head CI remains pending. Other expected-failure markers are unchanged.

Scope

#8624 independently handles the negative-zero result for zero divided by a negative integer in the same function. Both fixes need to be preserved and tested when the branches are combined. The only Lib change is the verified expected-failure marker removal described above.

Summary by CodeRabbit

  • Bug Fixes
    • Improved rounding for rational results and conversion of large integers to finite floating-point values.

Preserve the nearest finite result until the actual halfway overflow threshold for integer division and integer-to-float conversion.

Assisted-by: OpenAI Codex:model version not exposed
@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The integer conversion code now returns the nearest-rounded result from true_div. When BigInt::to_f64() returns None, bigint_to_finite_float uses true_div and returns the result only if it is finite.

Changes: Integer-to-float conversion

Layer / File(s) Summary
Rounding and conversion fallback
crates/common/src/int.rs
true_div returns the nearest-rounded value without converting finite extrema to infinity. If to_f64() returns None, bigint_to_finite_float falls back to true_div(int, 1) and returns the result only if it is finite.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

Merge Risk: Minimal · up to 4fd58

No merge-blocking issue is established for the overflow-boundary rounding change. Merge after normal checks pass, including the pending native tests.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: correcting the integer-to-float overflow rounding boundary.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 18fc82b and 4fd589a.

Files ignored due to path filters: 1
  • Lib/test/test_long.py is excluded by !Lib/**
Files selected for processing: 1
  • crates/common/src/int.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Assisted-by: OpenAI Codex:model version not exposed
@youknowone
youknowone marked this pull request as ready for review November 3, 2026 18:51
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T18:55:43.372217Z 4fd589a Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@youknowone
youknowone enabled auto-merge (squash) November 3, 2026 18:51
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] lib: cpython/Lib/_pylong.py
[ ] test: cpython/Lib/test/test_int.py (TODO: 2)
[x] test: cpython/Lib/test/test_long.py (TODO: 2)
[x] test: cpython/Lib/test/test_int_literal.py

dependencies:

  • int

dependent tests: (no tests depend on int)

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

@youknowone
youknowone merged commit e9d1d9f into RustPython:main Nov 4, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants