Actionable comments posted: 0

crates/derive-impl/src/pyclass.rs (1)

911-915: Remove commented-out code or add a tracking TODO.

Commented-out error-handling code is dead code that reduces readability. If the __init__ restriction is permanently removed (as the PR intent suggests), delete these lines. If this is temporary or under consideration for reinstatement, add a // TODO: or // FIXME: comment explaining the context.

🔎 Suggested fix: remove dead code

         if py_name == "__init__" {
-            // return Err(syn::Error::new(
-            //     ident.span(),
-            //     "#[pymethod] cannot define '__init__'. Use #[pyclass(with(Initializer))] instead.",
-            // ));
+            // __init__ is now exposed via PySlotWrapper descriptor in class.rs
         }

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 014622a and 3a803e3.

• Lib/test/test_descr.py is excluded by !Lib/**
• Lib/test/test_weakref.py is excluded by !Lib/**

• crates/derive-impl/src/pyclass.rs
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/builtins/object.rs
• crates/vm/src/builtins/weakref.rs
• crates/vm/src/class.rs
• crates/vm/src/stdlib/io.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/zoo.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

• crates/derive-impl/src/pyclass.rs
• crates/vm/src/types/zoo.rs
• crates/vm/src/class.rs
• crates/vm/src/builtins/object.rs
• crates/vm/src/builtins/weakref.rs
• crates/vm/src/stdlib/io.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/builtins/descriptor.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6358
File: crates/vm/src/exception_group.rs:173-185
Timestamp: 2025-12-09T08:46:58.660Z
Learning: In crates/vm/src/exception_group.rs, the derive() method intentionally always creates a BaseExceptionGroup instance rather than preserving the original exception class type. This is a deliberate design decision that differs from CPython's behavior.

Applied to files:

• crates/derive-impl/src/pyclass.rs

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

• crates/vm/src/class.rs
• crates/vm/src/builtins/descriptor.rs

crates/vm/src/class.rs (1)

• init_builtin_type (40-49)

crates/vm/src/types/slot.rs (1)

• slot_init (949-974)

crates/vm/src/types/slot.rs (1)

• init (976-976)

crates/vm/src/stdlib/ctypes/pointer.rs (2)

• init (22-65)
• init (251-259)

crates/vm/src/stdlib/ctypes/structure.rs (1)

• init (69-128)

crates/vm/src/types/zoo.rs (2)

97-98: LGTM!

The new wrapper_descriptor_type and method_wrapper_type fields are correctly declared with the standard &'static Py<PyType> pattern, consistent with other descriptor types in the struct.

---

192-193: LGTM!

Initialization follows the established pattern using init_builtin_type() and is correctly placed alongside other descriptor type initializations. The descriptor::init(context) call at line 244 will handle extending these types.

crates/vm/src/class.rs (2)

4-5: LGTM!

The imports for PyPayload and descriptor::PySlotWrapper are correctly added to support the new __init__ wrapper functionality.

---

139-151: PySlotWrapper struct definition matches code usage perfectly.

The __init__ slot wrapper logic correctly mirrors the __new__ pattern:

• Checks for slot existence before creating wrapper
• Avoids overwriting existing __init__ definitions in the class dict
• Uses appropriate CPython-style docstring
• Properly creates and attaches the PySlotWrapper descriptor with all required fields (typ, name, wrapped, doc)

crates/vm/src/types/slot.rs (1)

468-471: LGTM!

The improved error message now includes the actual returned type name with a .200 width limiter, which matches CPython's behavior and provides more actionable diagnostic information to users.

crates/vm/src/builtins/object.rs (2)

498-501: LGTM!

The __init__ pymethod correctly delegates to <Self as Initializer>::slot_init, providing the public Python-facing entry point for initialization that routes through the proper excess-args validation logic.

---

121-162: Verify the excess-args validation logic matches CPython's object_init.

The implementation correctly handles CPython's nuanced behavior:

1. Compares init/new slots via function pointer addresses for built-in types
2. For heap types, traverses MRO to detect __new__ definitions before object
3. Allows excess args only when both __init__ and __new__ are overridden

The MRO traversal at lines 140-143 uses take_while to stop before object_type, which is safe given Python's type system guarantees. The slot-level comparison approach in slot_init correctly complements the Python attribute-level comparison in slot_new (called during construction). The error message format matches CPython's conventions.

crates/vm/src/stdlib/io.rs (1)

1464-1473: LGTM!

The change to directly call zelf.init(raw, BufferSize { buffer_size }, vm) instead of routing through a public __init__ is appropriate. This simplifies the initialization path while maintaining the same behavior, aligning with the PR's goal of restructuring initialization to use slot-based wrappers.

crates/vm/src/builtins/weakref.rs (2)

54-61: LGTM!

The no-op Initializer implementation correctly mirrors CPython's weakref_tp_init behavior where __init__ accepts arguments but performs no action since all initialization is handled in slot_new. This is necessary for the wrapper-based __init__ exposure mechanism while maintaining compatibility with Python's weakref semantics.

---

63-73: LGTM!

The #[pyclass] attribute correctly includes Initializer in the trait list, enabling the new slot wrapper mechanism to expose __init__ on the weakref type.

crates/vm/src/builtins/descriptor.rs (6)

388-390: LGTM!

The init function is correctly extended to register the new PySlotWrapper and PyMethodWrapper types, ensuring they are properly initialized in the type system.

---

411-448: LGTM!

The GetDescriptor and Callable implementations for PySlotWrapper are well-structured:

• The descriptor correctly returns itself when accessed from a class or on None, and creates a bound PyMethodWrapper when accessed from an instance.
• The callable implementation properly validates the object type before delegating to the wrapped slot function.
• Returning None after calling the wrapped function is correct for __init__ semantics.

---

564-570: LGTM!

The Hashable implementation correctly combines the object's hash with the wrapper's identity hash using XOR. This follows the standard pattern for hashing bound method objects where both the method and the bound instance contribute to the hash.

---

572-584: LGTM!

The Comparable implementation correctly uses eq_only to restrict comparisons to equality only, and properly checks both wrapper identity (is) and object value equality (bool_eq). This matches CPython's method-wrapper comparison semantics.

---

534-549: LGTM!

The __reduce__ implementation correctly enables pickling by returning getattr with the bound object and method name, allowing the method wrapper to be reconstructed on unpickling.

---

491-497: Clarify the GC traversal design for obj field.

The #[pytraverse(skip)] on obj appears intentional (the wrapper field is traversed), but the design rationale should be confirmed. If PyMethodWrapper can be stored long-term in user code where obj forms cyclic references, this skip could prevent proper garbage collection. Verify this matches the intended lifecycle of method-wrapper objects—whether they're truly ephemeral or can persist as stored references.

Actionable comments posted: 1

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 3a803e3 and 6ffb2d8.

• Lib/test/test_descr.py is excluded by !Lib/**
• Lib/test/test_weakref.py is excluded by !Lib/**

• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/builtins/object.rs
• crates/vm/src/builtins/weakref.rs
• crates/vm/src/class.rs
• crates/vm/src/stdlib/io.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/zoo.rs

• crates/vm/src/class.rs
• crates/vm/src/types/zoo.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

• crates/vm/src/builtins/weakref.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/builtins/object.rs
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/stdlib/io.rs

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

• crates/vm/src/builtins/descriptor.rs

crates/vm/src/types/slot.rs (1)

• slot_init (949-974)

crates/vm/src/builtins/type.rs (5)

• zelf (1569-1569)
• zelf (1571-1571)
• typ (1322-1330)
• __new__ (548-558)
• name (579-584)

crates/vm/src/types/slot.rs (5)

• hash (1107-1107)
• descr_get (1029-1034)
• obj (1345-1346)
• repr_str (1133-1133)
• cmp (1155-1160)

crates/vm/src/class.rs (1)

• extend_class (80-158)

crates/vm/src/builtins/descriptor.rs (1)

• zelf (422-422)

crates/vm/src/stdlib/ctypes/base.rs (2)

• zelf (1332-1333)
• zelf (1613-1614)

crates/vm/src/exception_group.rs (1)

• zelf (212-213)

crates/vm/src/builtins/type.rs (2)

• zelf (1569-1569)
• zelf (1571-1571)

[error] 499-499: #[pymethod] cannot define 'init'. Use #[pyclass(with(Initializer))] instead.

crates/vm/src/stdlib/io.rs (1)

1465-1473: BufferedMixin::slot_init delegation looks correct

Directly delegating to Self::init after binding BufferSize keeps initialization logic centralized and consistent with the new slot/initializer model; no behavioral issues spotted.

crates/vm/src/types/slot.rs (1)

465-471: Improved init return error message is fine

Including the actual return type in "__init__ should return None, not '<type>'" matches CPython’s style and doesn’t alter control flow.

crates/vm/src/builtins/object.rs (1)

121-162: LGTM: Comprehensive excess argument validation.

The slot_init implementation correctly handles the complex logic for validating excess arguments to __init__. The differentiation between heap types (checking MRO) and built-in types (slot comparison) aligns with CPython's behavior, and the error messages are appropriately descriptive.

crates/vm/src/builtins/weakref.rs (1)

7-7: LGTM: Clean Initializer integration.

The addition of the Initializer trait to PyWeak is correctly implemented. The no-op init method is appropriate since all initialization is handled in slot_new (as documented in the comment on line 57), and the import additions and pyclass extension are properly structured.

Also applies to: 10-12, 54-61, 64-71

crates/vm/src/builtins/descriptor.rs (5)

6-10: LGTM: Import additions support new wrapper types.

The added imports (PyHash, InitFunc, Hashable, Comparable, PyComparisonOp, Representable) are all properly utilized in the new PySlotWrapper and PyMethodWrapper implementations.

---

392-427: LGTM: PySlotWrapper follows descriptor protocol.

The PySlotWrapper struct and its GetDescriptor implementation correctly implement the descriptor protocol: returning the descriptor itself when accessed from the class, and creating a bound PyMethodWrapper when accessed from an instance.

---

429-485: LGTM: PySlotWrapper Callable implementation is sound.

The Callable implementation correctly:

1. Binds the first argument as the object and validates its type
2. Provides a clear error message for type mismatches
3. Delegates to the wrapped function and returns None (appropriate for __init__ semantics)

The accessor methods and Representable implementation follow standard descriptor patterns.

---

487-585: LGTM: PyMethodWrapper implementation is comprehensive.

The PyMethodWrapper implementation correctly:

1. Binds the wrapper to an instance via the obj field
2. Delegates calls to the wrapped function with the bound object
3. Implements __reduce__ for pickle support using the standard (getattr, (obj, name)) pattern
4. Provides sound Hashable (XOR of object hash and wrapper identity) and Comparable (wrapper identity + object equality) implementations

All implementations align with Python's descriptor and method-wrapper semantics.

---

385-390: LGTM: Descriptor type registration follows established patterns.

The addition of PySlotWrapper and PyMethodWrapper to the init function correctly follows the existing pattern for registering descriptor types alongside PyMemberDescriptor and PyMethodDescriptor.

Actionable comments posted: 0

crates/vm/src/builtins/weakref.rs (1)

54-61: Consider expanding the comment to explain the CPython pattern.

The no-op init implementation correctly mirrors CPython's behavior where tp_init exists but does nothing for types like weakref that complete initialization in tp_new. The current comment is clear, but you might optionally expand it to explain this is matching CPython's design pattern for future maintainers.

crates/vm/src/builtins/object.rs (1)

121-162: Re‑check override detection for object.init excess‑args logic

The new slot_init logic is a good idea, but the way overrides are detected may not match how slots are actually wired:

• typ_init / object_init and typ_new / object_new are read from typ.slots.init/new.
• However, PyType::update_slot sets these slots to generic wrappers (init_wrapper / new_wrapper) whenever __init__ / __new__ are installed as attributes (including via the new PySlotWrapper and the existing slot_new_wrapper).
• For builtins that use the new wrappers, and for many heap types, this likely makes typ_init == object_init and typ_new == object_new even when the logical __init__ or __new__ is overridden, so typ_init != object_init && new_overridden may never be true.
• In that case, object.__init__ would always reject extra args for such instances, even when both __init__ and __new__ are conceptually overridden, which may diverge from the intended CPython‑like semantics for super().__init__(...) and direct object.__init__(...) calls on subclasses.

Consider basing “override” detection on attribute/MRO inspection for both heap and non‑heap types (e.g., scanning the MRO for __new__ / __init__ definitions before object) instead of, or in addition to, raw slot function pointer comparisons. It would be good to verify this behavior against a few representative cases (builtin subclass, pure Python subclass, subclass overriding only __init__, only __new__, and both) to ensure excess‑arg handling matches expectations.

crates/vm/src/builtins/descriptor.rs (1)

492-503: Consider letting GC traverse PyMethodWrapper.obj instead of skipping it

PyMethodWrapper stores a PyObjectRef in obj but marks it with #[pytraverse(skip)]:

#[pyclass(name = "method-wrapper", module = false, traverse)]
#[derive(Debug)]
pub struct PyMethodWrapper {
    pub wrapper: PyRef<PySlotWrapper>,
    #[pytraverse(skip)]
    pub obj: PyObjectRef,
}

Because obj is a real Python object reference, skipping it in traversal means cycles like obj.attr = obj.__init__ won’t be visible to the GC, and those graphs may never be collected if reference counting alone can’t break them.

Unless there’s a specific reason to hide this edge, it would be safer to let the derive macro traverse obj normally (matching how other bound‑method‐like types typically behave), or to implement a custom traverse that still visits obj.

Possible adjustment

 #[pyclass(name = "method-wrapper", module = false, traverse)]
 #[derive(Debug)]
 pub struct PyMethodWrapper {
     pub wrapper: PyRef<PySlotWrapper>,
-    #[pytraverse(skip)]
-    pub obj: PyObjectRef,
+    pub obj: PyObjectRef,
 }

If a custom traversal is actually needed for performance or correctness, consider documenting it here and adding an explicit Traverse impl that still visits obj.

Also applies to: 552-585

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 6ffb2d8 and 3cb6b52.

• Lib/test/test_descr.py is excluded by !Lib/**
• Lib/test/test_weakref.py is excluded by !Lib/**

• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/builtins/object.rs
• crates/vm/src/builtins/weakref.rs
• crates/vm/src/class.rs
• crates/vm/src/stdlib/io.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/zoo.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

• crates/vm/src/types/zoo.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/builtins/weakref.rs
• crates/vm/src/stdlib/io.rs
• crates/vm/src/class.rs
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/builtins/object.rs

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

• crates/vm/src/class.rs
• crates/vm/src/builtins/descriptor.rs

crates/vm/src/class.rs (1)

• init_builtin_type (40-49)

crates/vm/src/types/slot.rs (3)

• hash (1107-1107)
• descr_get (1029-1034)
• obj (1345-1346)

crates/vm/src/class.rs (1)

• extend_class (80-158)

crates/vm/src/builtins/getset.rs (2)

• descr_get (50-69)
• zelf (110-110)

crates/vm/src/builtins/type.rs (5)

• zelf (1569-1569)
• zelf (1571-1571)
• typ (1322-1330)
• __new__ (548-558)
• name (579-584)

• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Run rust tests (windows-latest)
• GitHub Check: Check the WASM package and demo

crates/vm/src/builtins/weakref.rs (2)

7-7: LGTM! Imports are correctly updated.

The addition of PyRef and Initializer imports are necessary for the new Initializer implementation below.

Also applies to: 10-12

---

63-73: LGTM! Correctly adds Initializer to the pyclass traits.

The pyclass attribute is properly updated to include Initializer in the with() clause, grouping it logically with Constructor.

crates/vm/src/types/zoo.rs (1)

97-98: Wrapper / method-wrapper types are correctly integrated into TypeZoo

The added wrapper_descriptor_type and method_wrapper_type fields, initialized via PySlotWrapper::init_builtin_type() and PyMethodWrapper::init_builtin_type(), follow the existing pattern for builtin descriptor types and cleanly expose these new types through Context::types.

Also applies to: 192-193

crates/vm/src/stdlib/io.rs (1)

1465-1473: slot_init now delegates directly to the trait initializer

Routing slot_init through zelf.init(raw, BufferSize { buffer_size }, vm) keeps all initialization logic in the init method while preserving the existing error-wrapping on args.bind(vm). This avoids any extra public __init__ indirection and looks consistent with the new slot-wrapper design.

crates/vm/src/class.rs (1)

4-5: init slot wrapper wiring is sound and non-intrusive

Importing PySlotWrapper and creating a wrapper only when class.slots.init is set and __init__ is absent ensures:

• Builtin types with an init slot get a proper wrapper_descriptor-style __init__.
• Explicit __init__ definitions (Python-level or Rust-level) are not overwritten.
• The captured init_func gives the wrapper a stable InitFunc even though update_slot later repoints slots.init to the generic init_wrapper.

This matches the existing __new__ handling pattern and fits the new descriptor types cleanly.

Also applies to: 139-151

crates/vm/src/types/slot.rs (1)

465-474: More informative init return-value error

Updating init_wrapper to report "__init__ should return None, not '<type>'" keeps the same guard while giving callers the concrete return type, which makes debugging bad initializers much easier without changing control flow.

crates/vm/src/builtins/descriptor.rs (1)

6-10: Slot / method wrapper descriptors are well‑integrated into the descriptor system

The additions of PySlotWrapper and PyMethodWrapper plus:

• importing InitFunc, Callable, Representable, Hashable, Comparable, and PyComparisonOp;
• registering them in init(ctx) alongside existing descriptor types; and
• implementing GetDescriptor + Callable so that unbound calls go through the slot and bound access returns a method-wrapper

form a coherent wrapper_descriptor/method-wrapper pair. The type / name / doc plumbing and reprs all look consistent with the existing method_descriptor / member_descriptor patterns and should make slot‑backed methods like __init__ behave naturally from Python.

Also applies to: 385-390, 392-485