Repository navigation
Fix 32-bit random bit counts and hash compatibility - #9437
Conversation
Accept u64 bit counts and reserve random word storage fallibly before changing RNG state. Match frozenset hashing to the native-width Set._hash mixin and use a fixed-width length prefix for byte/string hashes, preserving existing 64-bit results. Add allocation-boundary and set-hash regression coverage without changing existing test assertions. Implementation, investigation, validation, and focused automated review assisted by Codex. Assisted-by: Codex:GPT-5
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Byte and frozen-set hashing crates/common/src/hash.rs |
hash_bytes prefixes nonempty inputs with their length. FrozenSetHash uses usize arithmetic and converts its result through isize. A test compares results with the pointer-width set-mixin calculation. |
getrandbits allocation
| Layer / File(s) | Summary |
|---|---|
Checked word allocation crates/stdlib/src/random.rs |
getrandbits accepts counts representable as u64. For requests above 32 bits, it checks the word count and uses fallible reservation, returning MemoryError for invalid counts or reservation failures. Tests cover representative counts and platform limits. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Possibly related PRs
- RustPython/RustPython#9427: Both changes address hash-result compatibility on x86, though they update different hash routines.
- RustPython/RustPython#9233: Both changes affect
random.getrandbits; that PR adds a path for requests of 1–32 bits.
Suggested reviewers: shaharnaveh
Merge Risk: 
b78f2
-
No actionable merge-blocking issue is established. The reported compatibility fixes and successful platform tests support merging after normal checks, though those results have not been independently verified.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely identifies the two main changes: fixing 32-bit random bit-count handling and restoring hash compatibility. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. 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 6 functions across 2 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
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.
Comment @coderabbitai help to get the list of available commands.
Summary
Fix three reproducible Windows x86 compatibility failures without modifying
Lib/or existing test assertions:u64bit counts inrandom.getrandbits(), matching CPython 3.15. Previously2**31was rejected on x86. Check the word allocation size and reserve fallibly before advancing the RNG.collections.abc.Set._hash(), which masks usingsys.maxsize. Existing 64-bit results are unchanged.Add Rust regression coverage for allocation boundaries and set hashing. Only
crates/common/src/hash.rsandcrates/stdlib/src/random.rschange. C API, ctypes, PyO3, and CI are untouched.Validation
-D warningspass, excluding wasm, venvlauncher, and C API.pythonXY.lib; no C API workaround is included.Investigation, implementation, validation, and focused automated review were assisted by Codex. The automated review is not human review.
Summary by CodeRabbit
getrandbitsrequests: unrepresentable or excessively large requests now return a memory error instead of proceeding with an invalid allocation. Allocation failures are also handled safely.