Repository navigation
Bind interpreter arguments with borrowed Clinic structs - #9763
Merged
youknowone merged 7 commits intoNov 11, 2026
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. Pre-merge checks:
|
youknowdot
force-pushed
the
fix/signature-next-20261109
branch
from
November 11, 2026 09:08
128877d to
6352914
Compare
Member
|
TOOD: need review |
Extend the existing FromArgs derive with borrowed binding. Retain the native call input owners, validate and bind slots once before conversion, and generate signatures from the same fields. Remove the interpreter ArgSpec parsers and preserve conversion and finalization order without keepalive clones or allocation. Cover binding precedence, raw and inspected signatures, temporary lifetimes, reference counts, generated code, and borrowed payload validation. Assisted-by: OpenAI Codex:model version not exposed
Assisted-by: OpenAI Codex:model version not exposed
Release references extracted from tuples in reverse order, including nested and subclassed tuples. Select the order through a payload constant so other payloads keep their existing deallocation path and no new allocation is needed. Assisted-by: OpenAI Codex:model version not exposed
Keep the original keyword dictionary and positional tuple alive through non-Python and instrumented CALL_FUNCTION_EX invocations. Preserve the early container release for ordinary exact Python functions. Release keyword owners before positional owners, including exceptions and monitoring callbacks. Normalize tuple subclasses through iteration. A general iterable's normalized values retain their own storage for calls that need it, without allocating an otherwise unobservable tuple object. Cover exact functions, methods, callable objects, native calls, conversion failures, and monitoring transitions. Assisted-by: OpenAI Codex:model version not exposed
Release keyword and then positional owners in reverse evaluation order after binding, conversion, execution, and return conversion, including errors. The borrowed argument abstraction needs no additional allocation or reference clone. Expanded calls retain their original container ownership at the call boundary. Match CPython temporary lifetimes for reordered, mixed, expanded, successful, and failing interpreter calls instead of preserving the former keyword-order difference. Assisted-by: OpenAI Codex:model version not exposed
Combine expanded-argument collection with dispatch so original owners stay local instead of crossing two helper boundaries in a large aggregate. Select instrumentation at compile time and reserve keyword space when copying the positional values, avoiding a subsequent vector reallocation. Inline borrowed-owner cleanup into native wrappers so it does not introduce an additional call and owned FuncArgs transfer on every native invocation. Assisted-by: OpenAI Codex:model version not exposed
Parse `borrowed`, `legacy`, and `fname` through a single FromArgsMode so `#[from_args(borrowed, legacy)]` is rejected and the unknown-key error lists `borrowed`. Bind `_interpreters.list_all` with an owned struct instead of a borrowed struct carrying only a PhantomData lifetime. Reword two comments. Assisted-by: Claude:claude-opus-5-5 Assisted-by: Grok:grok-4.7
youknowone
force-pushed
the
fix/signature-next-20261109
branch
from
November 11, 2026 18:30
5157ca1 to
b1874da
Compare
youknowone
marked this pull request as ready for review
November 11, 2026 23:31
Codex Review Summary: ❌ FailedThis 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. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
youknowone
enabled auto-merge (squash)
November 11, 2026 23:31
youknowone
disabled auto-merge
November 11, 2026 23:31
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Replace the display-only
signature_argswrappers and_interpreters' duplicatedArgSpecparsers with borrowed Clinic argument structs.#[derive(FromArgs)]with#[from_args(borrowed)]. The native wrapper owns the originalFuncArgsthrough argument conversion, the implementation, and return conversion. Generated fields borrow that storage._interpretersno longer opts out of Clinic validation to expose signatures.PyUtf8Str's surrogate error.This builds on #9563 and #9729 and is based on main
dd8c34399.Validation
Reference: CPython 3.15.1 Clinic wrappers.
call(-1, None, args=None, unexpected=1)reporting the unexpected keyword first.call's raw line wrapping still differs; no exact raw-match claim is made for that string.Local validation:
cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi: passed, including the borrowed derive/payload regressions and doctests.crates/capion the final head: 149 passed, 2 ignored.--all-targets -- -D warnings: passed.extra_testsrun with the final release binary and CPython 3.15.1 had 491 passed, 20 failed, 15 subtests passed. All 20 failure IDs reproduce with the previous head binary: restricted multiprocessing/NETLINK operations, filesystemEINVAL, unavailable_sqlite3/CPython test modules, and existing CPython snippet incompatibilities. The three changed snippets pass on both interpreters. These failures are not counted as passes.test_call,test_extcall,test_tuple,test_monitoring,test_funcattrs, andtest_generators: passed (426 tests reported, 193 skipped).test_interpreters.test_apiremains skipped because this build has the GIL disabled, so it is not claimed as coverage.*iterablecalls to non-Python callables now retain a normalized argument snapshot, which requires one additional Vec and one clone/drop pair per positional value. Exact-tuple calls retain their existing owner without this snapshot allocation.*tuplecalls about 3–4% slower and native*listcalls 8–9% slower than635291497. Ordinary Python expansion paths were approximately baseline; native**kwargsimproved about 3%. Direct borrowed-call measurements varied by about +1–6% across focused/broad samples, so no universal zero-runtime-overhead claim is made. Expanded-call collection now keeps owner state local, preallocates keyword space, and specializes instrumentation at compile time; borrowed cleanup is inlined, confirmed in assembly.CI for head
5157ca191102b4985c23144dd37af5fdc52b4e71is tracked in this run. This PR remains a draft.AI-assisted implementation, tests, and review using OpenAI Codex.