Fix decompose_float subnormals, float format grouping/empty type; add HashSecret::from_keys by youknowone · Pull Request #8232 · RustPython/RustPython

youknowone · 2026-07-06T22:10:43Z

Three fixes in rustpython-common, found while building an external embedder (pyre) against the runtime-independent crates. Each was proven against CPython 3.14 ground truth.

1. `float_ops::decompose_float` mishandles subnormals

The bit-twiddling frexp assumes an implicit leading mantissa bit, which subnormals do not have.

Observable in RustPython before this fix:

hash(5e-324)  -> 8404992     # CPython 3.14: 16777216

Fix: scale subnormals into the normal range by an exact 2**54 shift before decomposing, fold the shift into the returned exponent. const fn preserved. Adds a frexp-contract unit test (mantissa in [0.5, 1), m * 2**e == |value|) over normals, subnormals, and boundary values.

2. `hash::HashSecret::from_keys`

k0/k1 are private and the only constructor derives them from a u32 seed, so an embedder cannot reproduce a fixed keying (deterministic PYTHONHASHSEED=0-style runs). Adds a const constructor taking explicit SipHash keys; the seeded path is unchanged. Unit test locks a fixed-key digest.

3. `format` float presentation fixes

Two defects surfaced by a large format-spec differential battery:

Thousands grouping corrupted non-integer tails: grouping split the magnitude only on ., so exponent/percent tails received separators (format(1e20, ',e') produced 1e+,20-class output). Now only the leading integer digits are grouped for decimal/float output; hex/octal/binary keep whole-magnitude grouping.
Empty presentation type: with a precision, it is now clamped to ≥ 1 and integer-looking results restore the trailing .0 (Py_DTSF_ADD_DOT_0); without a precision, alternate form inserts the forced . before an exponent (1e+16 → 1.e+16) instead of appending it.

Verification

cargo test -p rustpython-common: 45 pass (includes new tests)
Differential battery vs CPython 3.14 (subnormal/boundary hashes + 12 values × 16 format specs incl. ,e, ,.3e, ,g, ,%, .3, #g, 08,): byte-identical
-m test test_format test_float test_fstring test_hash: run=192 skipped=4 SUCCESS

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Added support for constructing hashing behavior directly from fixed key values.
Bug Fixes
- Improved float decomposition for subnormal f64 values to match frexp-like expectations.
- Refined float formatting edge cases, including grouping rules around exponent/percent parts and more consistent “empty presentation type” output.
- Adjusted character-format validation precedence to report precision-related errors consistently.
Tests
- Added/extended coverage for float decomposition and hash stability, plus formatting boundary cases.

coderabbitai · 2026-07-06T22:11:54Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 7dd50a6c-54de-4f1c-85c1-876413d43ecb

📥 Commits

Reviewing files that changed from the base of the PR and between 701b70e and 2e84dee.

📒 Files selected for processing (3)

crates/common/src/float_ops.rs
crates/common/src/format.rs
crates/common/src/hash.rs

🚧 Files skipped from review as they are similar to previous changes (2)

crates/common/src/hash.rs
crates/common/src/format.rs

📝 Walkthrough

Walkthrough

This PR updates subnormal float decomposition, float and integer formatting edge cases, and explicit-key HashSecret construction. It also adds unit tests covering the new float, formatting, and hashing behavior.

Changes

Float and formatting correctness fixes

Layer / File(s)	Summary
Subnormal-aware float decomposition `crates/common/src/float_ops.rs`	`decompose_float` now handles subnormal `f64` values by normalizing with a power-of-two shift and adjusting the exponent; tests cover the frexp-like contract and CPython hash constants.
Magnitude separator grouping fix `crates/common/src/format.rs`	`add_magnitude_separators_for_char` now groups only the leading digit prefix for decimal and float formats, leaves exponent and percent suffixes unchanged, and keeps non-decimal magnitudes fully groupable.
Float repr formatting and integer validation order `crates/common/src/format.rs`	`format_float` now appends `.0` for precision-based empty presentation output when needed and forces a decimal point for alternate-form exponent output; `format_int` now checks precision before sign or alternate-form for character formatting, with tests for these paths.
Explicit-key HashSecret constructor `crates/common/src/hash.rs`	Adds `HashSecret::from_keys(k0, k1)` for direct SipHash key construction, with tests for determinism, seed independence, and locked hash outputs.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately summarizes the main changes: subnormal float decomposition, float formatting fixes, and the new HashSecret constructor.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (1)

crates/common/src/hash.rs (1)
316-316: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Locked digest correctness depends on runtime verification.

LOCKED_DIGEST is a hardcoded regression value; its correctness can't be confirmed statically and requires cargo test -p rustpython-common to pass. If the underlying siphasher crate version or keyed_hash derivation ever changes, this constant will need updating — worth a brief comment noting that intent so future changes aren't mistaken for bugs.

Also applies to: 334-334
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/common/src/hash.rs` at line 316, Add a brief inline comment near
LOCKED_DIGEST (and the related second occurrence) in hash.rs explaining that it
is a regression lock value verified by runtime tests, not a statically derivable
constant. Make clear that changes to siphasher or keyed_hash derivation may
require updating the value, so future maintainers know this is intentional.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/common/src/hash.rs`:
- Line 316: Add a brief inline comment near LOCKED_DIGEST (and the related
second occurrence) in hash.rs explaining that it is a regression lock value
verified by runtime tests, not a statically derivable constant. Make clear that
changes to siphasher or keyed_hash derivation may require updating the value, so
future maintainers know this is intentional.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 6c8d180e-2c74-4afb-9382-a684f7e264c9

📥 Commits

Reviewing files that changed from the base of the PR and between a92c1ae and 701b70e.

📒 Files selected for processing (3)

crates/common/src/float_ops.rs
crates/common/src/format.rs
crates/common/src/hash.rs

Subnormals carry a biased exponent of 0 and no implicit leading mantissa bit, so the bit-twiddling frexp misread them: hash(5e-324) returned 8404992 instead of CPython's 16777216. Scale subnormals into the normal range by 2**54 (exact) before decomposing and fold the shift back into the returned exponent. Adds a frexp-contract unit test over normals, subnormals, and boundary values. Assisted-by: Claude

HashSecret's fields are private and the only constructor derives k0/k1 from a u32 seed, so an embedder cannot reproduce a fixed keying (e.g. a deterministic PYTHONHASHSEED=0-style run). Add a const constructor taking explicit SipHash keys; the seeded path is unchanged. Assisted-by: Claude

Thousands grouping split the magnitude only on '.', so exponent and percent tails received separators (format(1e20, ',e') gave '1e+,20'-class corruption). Group only the leading integer digits for interval-3 output; hex/octal/binary keep whole-magnitude grouping. Empty presentation type: with a precision, clamp it to at least 1 and restore a trailing '.0' on integer-looking results (Py_DTSF_ADD_DOT_0); without one, alternate form now inserts the forced decimal point before an exponent instead of appending it. Verified byte-identical to CPython 3.14 over a hash/format differential battery; test_format, test_float, test_fstring, test_hash pass. Assisted-by: Claude

- float_ops: use core::f64::consts::PI in the frexp-contract battery (clippy::approx_constant) - hash: rewrite the keyed_hash test via BuildHasher::hash_one (clippy::manual_hash_one) - rustfmt the new float_ops/hash/format test modules Assisted-by: Claude

coderabbitai Bot reviewed Jul 6, 2026

View reviewed changes

youknowone added 4 commits July 7, 2026 13:00

youknowone force-pushed the hash-float-fixes branch from 701b70e to 2e84dee Compare July 7, 2026 04:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix decompose_float subnormals, float format grouping/empty type; add HashSecret::from_keys#8232

Fix decompose_float subnormals, float format grouping/empty type; add HashSecret::from_keys#8232
youknowone wants to merge 4 commits into
RustPython:mainfrom
youknowone:hash-float-fixes

youknowone commented Jul 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

youknowone commented Jul 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. float_ops::decompose_float mishandles subnormals

2. hash::HashSecret::from_keys

3. format float presentation fixes

Verification

Summary by CodeRabbit

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Jul 6, 2026 •

edited by coderabbitai Bot

Loading

1. `float_ops::decompose_float` mishandles subnormals

2. `hash::HashSecret::from_keys`

3. `format` float presentation fixes

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading