Skip to content

Bind interpreter arguments with borrowed Clinic structs - #9763

Merged
youknowone merged 7 commits into
RustPython:mainfrom
youknowdot:fix/signature-next-20261109
Nov 11, 2026
Merged

youknowone merged 7 commits into
RustPython:mainfrom
youknowdot:fix/signature-next-20261109

Conversation

@youknowdot

@youknowdot youknowdot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Replace the display-only signature_args wrappers and _interpreters' duplicated ArgSpec parsers with borrowed Clinic argument structs.

  • Extend #[derive(FromArgs)] with #[from_args(borrowed)]. The native wrapper owns the original FuncArgs through argument conversion, the implementation, and return conversion. Generated fields borrow that storage.
  • Bind and validate each slot once before any converter runs, using the existing Clinic shape/count rules and Count matched keywords in one binding walk #9790's matched-keyword approach. Positional calls without keywords skip keyword lookup.
  • Generate binding and signature metadata from the same fields. _interpreters no longer opts out of Clinic validation to expose signatures.
  • Convert borrowed objects, typed payload references, and flags in Clinic declaration order. Preserve payload-specific validation such as PyUtf8Str's surrogate error.
  • Release borrowed native arguments in reverse evaluation-stack order. Retain original expanded-call containers through native, bound-method, callable-object, and instrumented calls, and release tuple members in reverse order.
  • Keep the existing owned and explicit legacy paths. Borrowing currently supports finite structs with one lifetime, passed as the sole arguments struct plus the VM. Unsupported layouts and display defaults without omission handling are rejected at compile time.

This builds on #9563 and #9729 and is based on main dd8c34399.

Validation

Reference: CPython 3.15.1 Clinic wrappers.

  • 273 binding/error cases match CPython exactly, including call(-1, None, args=None, unexpected=1) reporting the unexpected keyword first.
  • 15 inspected and whitespace-normalized raw signatures match. call's raw line wrapping still differs; no exact raw-match claim is made for that string.
  • 585 lifetime/error comparisons across 15 Clinic functions and nine call forms match CPython 3.15.1, including reordered keywords, positional/keyword mixtures, unpacking, binding errors, typed conversion errors, and boolean conversion exceptions.
  • New regressions cover direct/expanded native cleanup, exact Python functions versus bound methods and callable objects, tuple-subclass iteration, nested/subclass tuple destruction, raising bodies, and monitoring transitions.
  • Existing owned native converters and Python-function binding-failure cleanup are not migrated here. This is not a universal call-lifetime parity claim.
  • Derive tests check for one binding invocation and no owning copies, allocation containers, or second binding pass. VM tests check reference identity/counts and Clinic error precedence.
  • A local two-field borrowed-object counting-allocator probe over 100,000 positional and 100,000 keyword bindings measured zero allocations and unchanged input reference counts. Generated and equivalent handwritten extraction perform the same successful-path work in optimized x86-64 assembly; cold panic locations and function addresses differ. This is scoped evidence for the abstraction, not a claim that conversions or diagnostics can never allocate.

Local validation:

  • cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi: passed, including the borrowed derive/payload regressions and doctests.
  • C-API tests from crates/capi on the final head: 149 passed, 2 ignored.
  • Workspace and separate C-API Clippy runs with --all-targets -- -D warnings: passed.
  • The full extra_tests run 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, filesystem EINVAL, 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, and test_generators: passed (426 tests reported, 193 skipped). test_interpreters.test_api remains skipped because this build has the GIL disabled, so it is not claimed as coverage.
  • Repository pre-commit hooks: passed on every commit.
  • Common borrowed binding still adds zero allocations or reference-count clones. Non-tuple *iterable calls 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.
  • Final Linux x86-64 alternating-process microbenchmarks put native *tuple calls about 3–4% slower and native *list calls 8–9% slower than 635291497. Ordinary Python expansion paths were approximately baseline; native **kwargs improved 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 5157ca191102b4985c23144dd37af5fdc52b4e71 is tracked in this run. This PR remains a draft.

AI-assisted implementation, tests, and review using OpenAI Codex.

@coderabbitai

coderabbitai Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Caution

Review failed

The pull request is closed.

Pre-merge checks: Passed 1 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Title check Pending This assessment has not completed. The check will be retried on the next pre-merge check run.
Passed Passed checks: 1 passed

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
  • 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.

Share

@youknowdot
youknowdot force-pushed the fix/signature-next-20261109 branch from 128877d to 6352914 Compare November 11, 2026 09:08
@youknowdot youknowdot changed the title Expose interpreter signatures without changing argument ownership Bind interpreter arguments with borrowed Clinic structs Nov 11, 2026
@youknowone

Copy link
Copy Markdown
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
youknowone force-pushed the fix/signature-next-20261109 branch from 5157ca1 to b1874da Compare November 11, 2026 18:30
@youknowone
youknowone marked this pull request as ready for review November 11, 2026 23:31
@chatgpt-codex-connector

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-11T23:31:26.112451Z
We couldn’t complete this review. Comment @codex review to try again.
b1874da Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@youknowone
youknowone enabled auto-merge (squash) November 11, 2026 23:31
@youknowone
youknowone disabled auto-merge November 11, 2026 23:31
@youknowone
youknowone merged commit b99f38d into RustPython:main Nov 11, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants