Skip to content

Align function construction and metadata controls - #9717

Merged
youknowone merged 8 commits into
RustPython:mainfrom
youknowdot:fix/function-controls-20261104
Nov 8, 2026
Merged

youknowone merged 8 commits into
RustPython:mainfrom
youknowdot:fix/function-controls-20261104

Conversation

@youknowdot

@youknowdot youknowdot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Align function construction and public metadata controls with Python 3.15 with focused Python regression coverage.

  • Use the default Clinic-compatible FromArgs binder before validating supplied objects. Accept code/globals keywords, retain raw argument owners, and match closure/default/kwdefault validation order and constructor audit veto.
  • Emit public code/default/kwdefault read and mutation audits with the correct payloads and None-as-delete behavior. Read live fields after hooks and propagate vetoes before changes. Keep internal defaults accessors unaudited and release all locks before hooks or finalizers.
  • Match code/name/type-parameter setter diagnostics, using the live function name after audit. Preserve lone-surrogate names in function repr and closure-count errors without invoking string-subclass formatting callbacks.
  • Initialize doc only from a string first constant when the code declares a docstring. Preserve existing metadata identity, version invalidation and release-outside-lock behavior.

No handwritten per-function argument parser, Lib, markers or workflow changes. A direct function.__new__ error-formatting lifetime regression is covered in extra_tests/snippets/syntax_function.py. Existing bound-method and annotation changes remain outside this patch.

Validation

Compared with pinned CPython function controls and its generated Clinic signature. Independent source review passed, including owner lifetimes, audit timing/payloads, lock boundaries and unaudited internal callers.

Reference probes passed 71 metadata cases, four doc initialization cases, 1,024 positional/keyword layouts compared with the shared binder model, and 27 additional metadata/Unicode/hook cases. Constructed or modified code/functions were only inspected, never executed. These are CPython/source checks, not a RustPython runtime pass.

Limits and integration

Old code ownership remains deferred by the existing swap_to_temporary_refs mechanism. CPython releases old code constants earlier, but changing that here would undermine the outstanding-borrow safety design; no immediate-destruction parity is claimed. Shared unexpected-keyword surrogate formatting also remains unchanged. Function deallocation/GC redesign, JIT behavior and annotation reentrancy are outside scope.

Fresh relevant PR patches were checked and do not duplicate this group. Composing #9715 has one textual conflict in function.rs imports: retain this patch's PyDict/PyTypeRef imports and remove PyList once its custom method directory is removed. Independent read-only apply checks confirmed all other hunks apply after that import-only resolution. Combined branches were not compiled or executed. Preserve #9700 annotation diagnostics; that region is unchanged here. #6894 changes freevar representation, and #8624 has adjacent JIT invalidation work that requires preserving its logic if integrated. No combined runtime claim is made.

The latest local validation is recorded below. Separate cross-platform graphs still require hosted CI.

AI-assisted implementation and independent review using OpenAI Codex; model version is not exposed.

Clinic binding follow-up (2026-11-07)

  • Remove the custom slot_new, its ArgSpec slot vector, and duplicate argument-owner clones. The default Clinic binder handles the structural checks.
  • Keep raw argument owners intentionally. Eager typed conversion can run earlier arguments' finalizers before a later argument error is formatted in direct function.__new__ calls. A code constant finalizer can even rename the rejected globals type and change the error text. This is an ownership requirement, not a remaining Clinic binding mismatch. No new runtime guard or converter is introduced.
  • Replace the two remaining redundant setter tail drops with scoped _old bindings, preserving lock release, version invalidation, and finalizer order.

Validation on the final local tree:

  • Normal repository commit hooks passed.
  • Full workspace tests (documented exclusions), C-API tests, and workspace/C-API all-targets Clippy with -D warnings passed.
  • Release test_funcattrs: 36 tests, one skip, success.
  • 1,268 constructor layout/value/ownership probes match the previous PR head; direct-__new__ error-owner and setter-finalizer reentry probes pass.
  • Native snippet suite: 242 passed, six failed. The same six fail on the previous PR head (multiprocessing condition/fork-sharer/sharedctypes, os, socket, sqlite; sandbox/optional-module constraints). The remaining error-formatting lifetime regression passes.
  • CPython 3.15 source was inspected; the retained ownership probe also passes on the available system CPython 3.12. No CPython 3.15 local runtime claim. Hosted CI for the new commit is checked separately.

CPython 3.15 CI correction

The first follow-up test incorrectly fixed the relative cleanup order observed on CPython 3.12. Actual CPython 3.15 CI finalizes those temporary arguments in the opposite order. Commit 4623020bc8 removes that unsupported ordering assertion; it retains the regression requiring the globals TypeError to be formatted before a code-constant finalizer can rename the rejected type. CPython 3.15 func_new and _PyArg_BadArgument source independently confirm this remaining requirement. Production Rust code is unchanged from 311f2e81b7, so its workspace/C-API/Clippy validation still applies. The corrected exact snippet passes locally on RustPython and system CPython, and the native snippet suite again has the same 242 passes and six baseline environment failures. New hosted CI supplies the actual CPython 3.15 check.

Actual CPython 3.15 CI verification for 4623020bc864ae2ca9c8b99cbe76de69e35d5b69: the full run snippets step passed on Ubuntu, macOS, and Windows. This resolves the three reported snippet failures. Other CI stages were still running at verification time; this is not a claim that the whole workflow is green. Compatibility conclusions use CPython 3.15 source and these actual 3.15 runs, not the older local interpreter.

@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Caution

Review failed

The pull request is closed.

  • 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

Comment thread crates/vm/src/builtins/function.rs Outdated
}

#[pygetset(name = "__defaults__")]
fn get_defaults(&self, vm: &VirtualMachine) -> PyResult<Option<PyTupleRef>> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep pygetset function name as fn __defaults__ and rename the exising one to the other one

@youknowdot
youknowdot force-pushed the fix/function-controls-20261104 branch from f5acfd3 to 83dbeaf Compare November 6, 2026 13:22
youknowdot and others added 4 commits November 7, 2026 15:22
Preserve constructor binding, validation, audit veto and argument owners. Audit public metadata without changing internal reads, retaining live-field semantics and Unicode names.

Assisted-by: OpenAI Codex:model version not exposed
Rename the unaudited Rust helpers instead of overriding Python getter names, following the exception add_note review. Update internal cross-interpreter callers without introducing audits.

Assisted-by: Codex:model-version-unavailable
Assisted-by: OpenAI Codex:model version not exposed
Preserve callback ownership, release ordering, and retained argument owners.

Assisted-by: Codex:model-version-unavailable
@youknowdot
youknowdot force-pushed the fix/function-controls-20261104 branch from 038adc0 to f31c918 Compare November 7, 2026 09:51
codex added 3 commits November 7, 2026 21:10
Remove the custom ArgSpec slot and redundant parsed-owner allocation. Retain raw argument owners because eager typed failures can run finalizers before later error formatting in direct __new__ calls.

Assisted-by: OpenAI Codex:gpt-6.1-sol
Assisted-by: OpenAI Codex:gpt-6.1-sol
Both replaced values still drop after lock release and version invalidation, before returning the callback-free unit result.

Assisted-by: OpenAI Codex:gpt-6.1-sol
@codspeed

codspeed Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1


Comparing youknowdot:fix/function-controls-20261104 (311f2e8) with main (3d0f6fc)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

Keep the error-formatting lifetime regression, but do not impose CPython 3.12 finalizer ordering on CPython 3.15.

Assisted-by: OpenAI Codex:gpt-6.1-sol
@youknowone
youknowone marked this pull request as ready for review November 8, 2026 02:31
@youknowone
youknowone merged commit 5c2d115 into RustPython:main Nov 8, 2026
20 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 8, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-08T02:36:37.863130Z 4623020 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.

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.

3 participants