PyType::py_new to return Self by youknowone · Pull Request #6398 · RustPython/RustPython

youknowone

Caution

Review failed

The pull request is closed.

Walkthrough

Widespread constructor API overhaul: introduce a slot_new/py_new contract where py_new takes &Py and returns PyResult (often unused), many constructors moved to slot_new(FuncArgs) with args.bind(vm), and object creation now returns owned Self directly instead of into_ref_with_type/.into().

Changes

Cohort / File(s)	Summary
Constructor trait & core slot machinery `crates/vm/src/types/slot.rs`	New `Constructor` trait: associated `Args: FromArgs`, `slot_new` (binds FuncArgs) and `py_new(&Py<PyType>) -> PyResult<Self>` contract; DefaultConstructor/Unconstructible helpers and default impls.
Global pattern: py_new → py_new(&Py) / slot_new(FuncArgs) many files (see cohorts below)	Many constructors changed to accept `_cls: &Py<PyType>` and return `PyResult<Self>` or were moved to `slot_new(cls: PyTypeRef, args: FuncArgs, vm)` with `args.bind(vm)?`; defensive `py_new` stubs with `unreachable!("use slot_new")` added where applicable.
Stdlib — core modules `crates/stdlib/src/array.rs`, `crates/stdlib/src/bz2.rs`, `crates/stdlib/src/json.rs`, `crates/stdlib/src/lzma.rs`, `crates/stdlib/src/mmap.rs`, `crates/stdlib/src/overlapped.rs`, `crates/stdlib/src/pystruct.rs`, `crates/stdlib/src/select.rs`, `crates/stdlib/src/ssl.rs`, `crates/stdlib/src/zlib.rs`, `crates/stdlib/src/sqlite.rs`, `crates/stdlib/src/csv.rs`	Constructor signatures adjusted to `_cls: &Py<PyType>` / return `PyResult<Self>`; imports updated to include `Py`/`PyType`; creation changed from `.into_ref_with_type(...).map(Into::into)` to direct `Ok(Self { ... })` or `Ok(Self::from(...))`.
VM builtins — iterators, itertools & containers `crates/vm/src/stdlib/itertools.rs`, `crates/vm/src/stdlib/collections.rs`, `crates/vm/src/builtins/enumerate.rs`, `crates/vm/src/builtins/filter.rs`, `crates/vm/src/builtins/map.rs`, `crates/vm/src/builtins/list.rs`, `crates/vm/src/builtins/tuple.rs`, `crates/vm/src/builtins/zip.rs`	Iterators and itertools constructors switched to new signatures/return pattern; many constructors now return `Ok(Self { ... })`; some slot_new entry points and py_new stubs added; common fast-paths preserved but relocated to slot_new where relevant.
VM builtins — primitives, callables, descriptors `crates/vm/src/builtins/bool.rs`, `.../bytes.rs`, `.../int.rs`, `.../float.rs`, `.../complex.rs`, `.../str.rs`, `.../class**.rs`, `.../function.rs`, `.../property.rs`, `.../object.rs`	Numerous constructors moved to `slot_new(cls, FuncArgs, vm)` with `args.bind(vm)?` and optimizations; `py_new(&Py<PyType>) -> PyResult<Self>` stubs added; PyFunction/PyBoundMethod/PyCell and descriptors adjusted to return payload Self and enforce slot-wrapping.
CTypes subsystem `crates/vm/src/stdlib/ctypes/*` (`array.rs`, `base.rs`, `function.rs`, `pointer.rs`, `structure.rs`, `union.rs`)	Standardize slot_new usage: constructors renamed to slot_new with FuncArgs binding; internal callsites updated to call slot_new; unreachable py_new stubs added; some returns still wrap via into_ref_with_type when needed.
Typing / TypeVar / ParamSpec `crates/vm/src/stdlib/typevar.rs`, `crates/vm/src/stdlib/typing.rs`	Typing constructors ported to slot_new flows; py_new signatures changed to &Py → PyResult for payload creation; slot_new wraps payload into PyObjectRef and sets module metadata (set_module_from_caller).
Exceptions, traceback, and misc builtin types `crates/vm/src/exceptions.rs`, `crates/vm/src/builtins/traceback.rs`, `crates/vm/src/builtins/singletons.rs`, `crates/vm/src/builtins/slice.rs`, `crates/vm/src/builtins/memory.rs`, `crates/vm/src/builtins/weakref.rs`, `crates/vm/src/builtins/super.rs`, `crates/vm/src/builtins/weakproxy.rs`	Constructors updated to slot_new/py_new(&Py) pattern; singleton constructors use slot_new and add unreachable py_new stubs; traceback/exceptions adopt direct `Ok(Self::new(...))` returns and import adjustments.
I/O, functools, sqlite, posix, thread, operator, etc. `crates/vm/src/stdlib/io.rs`, `crates/vm/src/stdlib/functools.rs`, `crates/vm/src/stdlib/posix.rs`, `crates/vm/src/stdlib/thread.rs`, `crates/vm/src/stdlib/operator.rs`, `crates/vm/src/stdlib/sqlite.rs`	Broad normalization: replace PyTypeRef usage with &Py or slot_new(FuncArgs), return `PyResult<Self>`, direct `Ok(Self { ... })` initializations, and add FuncArgs/KwArgs imports where applicable.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Files/areas needing extra attention:
- crates/vm/src/types/slot.rs — new trait, default impls and contract between slot_new and py_new.
- Callsites that previously expected PyObjectRef / PyRef — ensure wrapping still occurs where Python-visible objects are required.
- CTypes modules and internal callsite substitutions to slot_new (array/base/pointer/structure/union/function).
- Argument-binding changes (FuncArgs::bind) and fast-path correctness in numeric/tuple/list constructors.
- Typing/typevar module: set_module_from_caller and module metadata propagation.

Possibly related PRs

Separate Debug from PyPayload #6320 — related changes to Constructor/slot trait, py_new/slot_new contract.
ctypes buffer #6311 — overlapping ctypes constructor refactors and slot_new adjustments.
Pyfunction builtins and constructor #5823 — related PyFunction constructor changes and closure/constructor behavior.

Suggested reviewers

ShaharNaveh
arihant2math

"🐇
I hopped through types with nimble paws,
Swapped refs for slots and fixed the laws.
Constructors now return Self with cheer,
slot_new leads — py_new whispers 'not here'.
Hop, compile, and pass the tests — hooray! 🥕"

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.29% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main refactoring: changing PyType's py_new method to return Self instead of PyResult.

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2719731 and 2a3836d.

📒 Files selected for processing (3)

crates/vm/src/builtins/complex.rs (4 hunks)
crates/vm/src/builtins/float.rs (2 hunks)
crates/vm/src/builtins/int.rs (3 hunks)

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 @coderabbitai help to get the list of available commands and usage tips.}