You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Declare six nullable argument defaults directly in four Rust files:
mkfifo.dir_fd
pidfd_send_signal.siginfo
The shared match groups/groupdict default
Three ignored epoll exit arguments
These use Option<PyObjectRef> with default = None. Omission remains a native absent state, with existing lazy None fallbacks retained. Rendered signatures are unchanged; typed None metadata replaces Raw metadata. The existing path, file-descriptor conversion and warning order remain intact. The three epoll exit fields now use their signature names directly, removing redundant name attributes while keeping the ignored owning argument and its drop timing unchanged.
The earlier nine non-None changes have been restored: two compression max lengths, array start, four binascii canonical flags, recursive, and mkfifo mode. Their original native or Missing paths avoid Python default-object construction and conversion. In particular, the current derive implementation does not recognize the PySsize alias as a primitive integer, so changing its defaults here would add boxing and reconversion.
Independent exact-head source review, normal pre-commit hooks, rustfmt and diff checks passed for 04317ac07270288efbb81b8efda97446d95aa32c.
No test assertions, test files or Lib files changed.
Workspace/C-API tests and Clippy plus canonical test_os, test_epoll, test_signal and test_re were attempted offline. All stopped before compilation because the pinned parking_lot_core git dependency is unavailable locally. No local native test pass is claimed.
CI for current head 1fddea5f30be3b846023c9fc6c7ec2da18f9c235 is in progress. The previous run for 04317ac07270288efbb81b8efda97446d95aa32c was cancelled after the author-only rewrite; both heads have identical source trees. The current head's full CI results remain pending.
AI-assisted implementation and review with OpenAI Codex; model version not exposed.
Summary by CodeRabbit
Bug Fixes
pidfd_send_signal now raises a TypeError when siginfo is explicitly provided as None; omitting the argument continues to use its default. Only an omitted siginfo is accepted, while any explicitly supplied value is rejected. No other user-visible behavior changes are noted in this update.
Declare fifteen concrete argument defaults directly across compression, array, binascii, process helpers, epoll and match methods. Keep bool conversion and argument release at their current body locations, acquired buffer cleanup order, mkfifo path-before-fd conversion, and existing signed bound semantics.
Preserve symbolic, callback-sensitive and stateful omission defaults. No tests, assertions or Lib files changed. Source-reviewed against CPython 3.15.0 d220409a13b77de7a6c30abe16ea90db01528edc; native validation remains limited by the available dependency closure.
Assisted-by: OpenAI Codex:model version not exposed
No actionable comments were generated in the recent review.
Walkthrough
Several standard-library argument wrappers now use Option<PyObjectRef> with direct None defaults. Call-site handling changes in pidfd_send_signal, findall, and mkfifo.
Changes: Nullable argument handling
Layer / File(s)
Summary
Epoll and signal arguments crates/stdlib/src/select.rs, crates/vm/src/stdlib/_signal.rs
EpollExitArgs uses named optional fields. PidfdSendSignalArgs.siginfo uses Option<PyObjectRef>, and pidfd_send_signal rejects any present value, including None.
Regex match defaults crates/vm/src/stdlib/_sre.rs
DefaultArg.default uses Option<PyObjectRef>. findall passes the empty unmatched-group value as Some(empty.clone()).
RustPython/RustPython#9559: Uses Option<PyObjectRef> with default = None for nullable arguments in _datetime.rs.
Suggested reviewers:youknowone
Merge Risk: Minimal · up to 9b00a
The nullable-default changes preserve the inspected behavior. No merge-blocking issue was identified; normal build and test checks should still complete before merging.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
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.
Passed checks: 4 passed
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: declaring nullable argument defaults directly instead of through OptionalArg across the affected Rust 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.
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 2 functions across 4 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.
Reviewing files that changed from the base of the PR and between 1fddea5 and 9b00ab3.
Files selected for processing: 1
crates/vm/src/stdlib/_sre.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.
Restore runtime defaults whose concrete literal lowering adds Python object creation, conversion, or reference management on omitted arguments. Keep only cost-preserving None default cleanup, where applicable. No test code or expectations changed.
Assisted-by: OpenAI Codex:model version not exposed
youknowdot
changed the title
Replace concrete omission wrappers with typed defaults
Declare nullable argument defaults directly
Nov 3, 2026
Use the signature names as the Rust field names so explicit name attributes are redundant. Match the existing multiprocessing ExitArgs dead-code annotation for intentionally ignored context-manager arguments; keep the function body and argument lifetimes unchanged.
Assisted-by: OpenAI Codex:model version not exposed
The nullable match default no longer uses OptionalArg. Remove the stale import so Clippy with -Dwarnings passes; no runtime or test changes.
Assisted-by: OpenAI Codex:model version not exposed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
youknowdot
changed the title
Summary
Declare six nullable argument defaults directly in four Rust files:
mkfifo.dir_fdpidfd_send_signal.siginfogroups/groupdictdefaultThese use
Option<PyObjectRef>withdefault = None. Omission remains a native absent state, with existing lazy None fallbacks retained. Rendered signatures are unchanged; typed None metadata replaces Raw metadata. The existing path, file-descriptor conversion and warning order remain intact. The three epoll exit fields now use their signature names directly, removing redundantnameattributes while keeping the ignored owning argument and its drop timing unchanged.The earlier nine non-None changes have been restored: two compression max lengths, array start, four binascii canonical flags, recursive, and mkfifo mode. Their original native or Missing paths avoid Python default-object construction and conversion. In particular, the current derive implementation does not recognize the
PySsizealias as a primitive integer, so changing its defaults here would add boxing and reconversion.CPython 3.15 source pin: d220409a13b77de7a6c30abe16ea90db01528edc. References: mkfifo, signal, match defaults, epoll exit.
Validation
04317ac07270288efbb81b8efda97446d95aa32c.test_os,test_epoll,test_signalandtest_rewere attempted offline. All stopped before compilation because the pinnedparking_lot_coregit dependency is unavailable locally. No local native test pass is claimed.1fddea5f30be3b846023c9fc6c7ec2da18f9c235is in progress. The previous run for04317ac07270288efbb81b8efda97446d95aa32cwas cancelled after the author-only rewrite; both heads have identical source trees. The current head's full CI results remain pending.AI-assisted implementation and review with OpenAI Codex; model version not exposed.
Summary by CodeRabbit
pidfd_send_signalnow raises aTypeErrorwhensiginfois explicitly provided asNone; omitting the argument continues to use its default. Only an omittedsiginfois accepted, while any explicitly supplied value is rejected. No other user-visible behavior changes are noted in this update.