Repository navigation
Validate sleep against Python 3.14 signed time bounds - #9051
Conversation
Assisted-by: OpenAI Codex:model identifier unavailable
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to time.sleep now validates its argument against signed 64-bit nanosecond bounds and raises the expected exceptions for out-of-range, NaN and unsupported inputs. No actionable merge-blocking risk was found. The author reports bounded Linux testing only, with full CI still pending. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: validating time.sleep against Python 3.14 signed time bounds. |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. |
| 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. |
✨ 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.
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d22a003464
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dict.get_item_opt(identifier!(vm, __module__), vm)? | ||
| .ok_or_else(|| vm.new_attribute_error("__module__"))? |
There was a problem hiding this comment.
Preserve TypeError when heap types have no module
On WebAssembly, time.sleep(sys.stdout) reaches this branch because make_stdstream_object creates the stream class with py_class!, whose new_class(None, ...) leaves the heap-type dictionary without __module__ (crates/wasm/src/wasm_builtins.rs:32 and crates/vm/src/macros.rs:38). This ok_or_else therefore replaces the original unsupported-input TypeError with AttributeError("__module__"); before this change the diagnostic used class().name() and remained a TypeError. Treat an absent module as an unqualified type name, or populate it for every internal heap type.
AGENTS.md reference: AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I checked the missing-module case against exact CPython 3.14.7 using a heap type whose real namespace is exposed through classdictcell, then removing module from that namespace. CPython raises AttributeError("module"), and the published RustPython head does the same. The existing five-case type-dictionary oracle also verifies that lookup exceptions retain their identity. Falling back to an unqualified name here would therefore change verified CPython behavior. The internal WASM construction path is a separate type-metadata question; I have not run WASM locally or changed that construction.
Summary
Extract the independent signed-time range validation from #8954 (
43065e79ddc46eafeca58cb39398ac007b5d25f7), adapted locally to Python 3.14 on main3af5f8a15314446d2b28a90e429c938dc7b8beeb.time.sleepusing signed 64-bit nanoseconds, with checked integer multiplication and timeout rounding away from zero.__index__protocol, including subclass behavior. Do not adopt the mega's 3.15 input widening or its new shared pytime module.Reference: CPython 3.14.7 sleep, signed-time conversion, and type metadata formatting.
Related work
Open #5907 addresses related invalid sleep inputs and is not being replaced or modified by this PR. Its current patch targets the old split-platform
vm/srcimplementation, applies u64-second versus u32-millisecond bounds, accepts general float conversion, and expectsValueErrorfor infinity/huge values. Current main has a unified audited sleep function, while the exact 3.14 contract requires signed nanoseconds andOverflowError. Reusing that patch would require replacing its central implementation and test expectations. Merged #5906 and #7073 are already in the base.Validation
test_sleep_exceptions.Coverage is in the existing Python snippet; canonical tests, shared duration conversion, platform sleepers and README are unchanged. No Rust behavioral tests or implementation-name guards were added. Local validation is bounded Linux validation, not a complete workspace/cross-platform test claim or a test of very large accepted sleeps. Full hosted CI is pending; this remains a draft for review.
AI-assisted implementation and validation with OpenAI Codex; model identifier unavailable. No human review is implied.
Summary by CodeRabbit
time.sleepnow handles fractional and integer durations consistently, rejects invalid values with the appropriate errors, and reports unsupported argument types more accurately.