Actionable comments posted: 0

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

82-98: Minor: Comment says "Bitwise operations (47-64)" but the range seems different.

The comment at line 83 says "(47-64)" but NumLshift starts after NumInplaceMatrixMultiply. This is a documentation nit - the code functionality is correct.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between c9bf8df and 5388680.

• Lib/test/test_descr.py is excluded by !Lib/**
• Lib/test/test_inspect/test_inspect.py is excluded by !Lib/**
• Lib/test/test_sqlite3/test_factory.py is excluded by !Lib/**
• Lib/test/test_unittest/testmock/testhelpers.py is excluded by !Lib/**

• .cspell.dict/cpython.txt
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/builtins/type.rs
• crates/vm/src/class.rs
• crates/vm/src/protocol/mod.rs
• crates/vm/src/types/mod.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/slotdefs.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/mod.rs
• crates/vm/src/protocol/mod.rs
• crates/vm/src/types/zoo.rs
• crates/vm/src/builtins/type.rs
• crates/vm/src/types/slotdefs.rs
• crates/vm/src/class.rs
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/types/slot.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In the RustPython codebase, only certain builtin types should be marked with the SEQUENCE flag for pattern matching. List and tuple are sequences, but bytes, bytearray, and range are not considered sequences in this context, even though they may implement sequence-like protocols.

Applied to files:

• crates/vm/src/protocol/mod.rs
• crates/vm/src/types/slot.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In RustPython's pattern matching implementation, only certain builtin types should have the SEQUENCE flag: list and tuple are confirmed sequences. The user youknowone indicated that bytes, bytearray are not considered sequences in this context, even though they implement sequence-like protocols.

Applied to files:

• crates/vm/src/protocol/mod.rs
• crates/vm/src/types/slot.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/protocol/number.rs (1)

• handle_bytes_to_int_err (558-578)

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

• init_builtin_type (86-95)

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

• find_slotdefs_by_name (1190-1192)
• inherit_from_mro (277-409)

• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run snippets and cpython tests (windows-latest)

.cspell.dict/cpython.txt (1)

4-4: LGTM!

The "attro" token is a valid CPython term (used in tp_getattro, tp_setattro) and is correctly placed in alphabetical order.

crates/vm/src/protocol/mod.rs (1)

13-16: LGTM!

The addition of PyNumberTernaryFunc to the public exports is necessary for the new ternary slot wrappers (power operations) and is properly ordered.

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

192-192: LGTM!

The update from PySlotWrapper to PyWrapper is consistent with the type rename across the codebase.

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

2-2: LGTM!

The new slotdefs module is properly declared and the key public items (SLOT_DEFS, SlotAccessor, SlotDef) are correctly re-exported.

Also applies to: 7-7

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

466-472: LGTM!

The refactor to iterate SLOT_DEFS for slot inheritance is clean and maintainable. The comment correctly notes that as_buffer is handled separately in inherit_readonly_slots (lines 459-462) since it's not an AtomicCell.

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

13-57: Well-structured implementation mirroring CPython's add_operators().

The function correctly:

1. Skips __new__ which has special handling via slot_new_wrapper
2. Handles __hash__ = None for unhashable types
3. Only creates wrappers for slots that have functions and aren't already in the dict

---

185-186: LGTM!

Clean replacement of the previous macro-based slot wrapper logic with the new add_operators call.

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

292-305: LGTM!

The new type aliases for sequence and mapping sub-slot function types improve code clarity and match the patterns used elsewhere in the codebase.

---

350-373: LGTM!

The ternary operation wrapper macros correctly handle the optional modulus argument for __pow__, __rpow__, and __ipow__. When the third argument is None, only two arguments are passed to the special method, matching Python's semantics.

---

550-561: LGTM!

The refactored update_slot method cleanly delegates to find_slotdefs_by_name and update_one_slot, aligning with the data-driven approach using SLOT_DEFS.

---

618-643: Correct handling of __hash__ = None for unhashable types.

The special case properly detects when __hash__ is explicitly set to None in the class dict or MRO, and stores hash_not_implemented accordingly. This matches CPython's behavior.

---

1174-1214: LGTM!

The lookup_slot_in_mro helper correctly distinguishes between slot wrappers (from which slot functions can be extracted) and real Python methods (which should use the generic wrapper). The early return on finding any attribute ensures the first definition in MRO wins.

---

1107-1120: The SeqConcat/SeqRepeat slot handling is intentional and correct.

When ADD=true (during type creation), the sequence concat/repeat slots are skipped to allow the dual __add__ definition to be processed via the NumAdd slot instead. This is the correct design: list and other sequence types define __add__ as a method (using #[pymethod]), which gets stored in the NumAdd slot. The "number protocol fallback" comment refers to the runtime behavior in _add(): the operation first tries NumAdd (number protocol), and only if that returns NotImplemented does it fall back to the sequence protocol's concat() method. This approach works correctly for types like list that define __add__ via the sequence protocol.

crates/vm/src/types/slotdefs.rs (6)

1-5: LGTM!

Clear module documentation explaining the purpose of this file as the CPython slotdefs[] equivalent.

---

27-124: Well-organized SlotAccessor enum with #[repr(u8)] for compact storage.

The enum comprehensively covers all CPython slot types with clear grouping via comments. The explicit discriminant values help ensure stable ordering.

---

463-475: Good implementation of SLOTDEFINED check for __init__ inheritance.

This correctly handles the multiple inheritance edge case where __init__ should only be inherited if it was explicitly defined in the base class (not inherited from a grandparent). This matches CPython's behavior.

---

762-938: Comprehensive SLOT_DEFS array covering main slots, comparison, descriptors, and sequence/mapping protocols.

The duplicate entries for __len__, __getitem__, __setitem__, __delitem__, __add__, __mul__, __iadd__, __imul__ correctly map to both sequence and number/mapping protocols, matching CPython's slotdefs behavior.

---

1189-1195: LGTM!

The find_slotdefs_by_name utility provides efficient lookup for slot updates, and SLOT_DEFS_COUNT is useful for testing/validation.

---

1197-1244: Good test coverage for the slot definitions.

Tests verify count bounds, name lookups (including duplicate entries), #[repr(u8)] size guarantee, and rich_compare_op mappings.

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

7-16: LGTM! Import additions support the expanded slot system.

The new imports properly support the SlotFunc enum expansion, bringing in function types for attribute access, descriptors, sequences, mappings, and numeric operations.

---

404-445: Well-structured slot function expansion.

The SlotFunc enum expansion is cleanly organized by protocol category with clear comments. The design choice to use the same function type for Set/Del variants (e.g., SetAttro/DelAttro both use SetattroFunc) with different PySetterValue handling is elegant and reduces code duplication.

---

447-483: LGTM! Complete Debug implementation.

All SlotFunc variants are properly covered with consistent formatting. The additional detail for RichCompare showing the comparison operator is helpful for debugging.

---

530-646: Comprehensive and correct slot call implementations.

All new SlotFunc::call implementations are well-structured and handle their respective protocols correctly:

• Attribute access slots (GetAttro, SetAttro, DelAttro): Properly use PySetterValue to distinguish set from delete operations
• Descriptor slots (DescrGet, DescrSet, DescrDel): Correctly implement Python's descriptor protocol with proper None handling for instance access
• Sequence/Mapping slots: Consistently use sequence_unchecked()/mapping_unchecked() for protocol access
• Number slots: Proper handling of unary, binary, and ternary operations with correct default argument handling for __pow__

The use of OptionalArg and into_option() for assignment slots (SeqAssItem, MapAssSubscript) elegantly handles both __setitem__ and __delitem__ semantics.

---

394-394: Clean and consistent rename from PySlotWrapper to PyWrapper.

The rename is thoroughly applied across all trait implementations (PyPayload, GetDescriptor, Callable, Representable), the struct definition, and class extension call. All references are updated consistently.

Also applies to: 655-655, 662-666, 668-683, 685-703, 709-729, 731-740

---

749-749: LGTM! PyMethodWrapper correctly updated to reference PyWrapper.

The wrapper field type is properly updated from PyRef<PySlotWrapper> to PyRef<PyWrapper>, maintaining consistency with the rename.

Actionable comments posted: 1

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

1223-1227: Test assumes SlotAccessor size is exactly 1 byte.

The assertion std::mem::size_of::<SlotAccessor>() == 1 enforces #[repr(u8)] sizing, but the enum doesn't have the #[repr(u8)] attribute. With 80+ variants, the compiler should still use 1 byte, but adding explicit #[repr(u8)] would make this guarantee explicit and prevent future breakage if variants exceed 256.

Consider adding explicit repr(u8)

 /// Slot accessor
 ///
 /// Flat enum with all slot types inlined for `#[repr(u8)]` support.
 /// Each variant directly corresponds to a slot field in PyTypeSlots.
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[repr(u8)]
 pub enum SlotAccessor {

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 5388680 and ade6c6e.

• Lib/test/test_descr.py is excluded by !Lib/**
• Lib/test/test_inspect/test_inspect.py is excluded by !Lib/**
• Lib/test/test_sqlite3/test_factory.py is excluded by !Lib/**
• Lib/test/test_unittest/testmock/testhelpers.py is excluded by !Lib/**

• .cspell.dict/cpython.txt
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/builtins/type.rs
• crates/vm/src/class.rs
• crates/vm/src/protocol/mod.rs
• crates/vm/src/types/mod.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/slot_defs.rs

• .cspell.dict/cpython.txt
• crates/vm/src/protocol/mod.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/mod.rs
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/class.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/builtins/type.rs
• crates/vm/src/types/slot_defs.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

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In RustPython's pattern matching implementation, only certain builtin types should have the SEQUENCE flag: list and tuple are confirmed sequences. The user youknowone indicated that bytes, bytearray are not considered sequences in this context, even though they implement sequence-like protocols.

Applied to files:

• crates/vm/src/types/slot.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In the RustPython codebase, only certain builtin types should be marked with the SEQUENCE flag for pattern matching. List and tuple are sequences, but bytes, bytearray, and range are not considered sequences in this context, even though they may implement sequence-like protocols.

Applied to files:

• crates/vm/src/types/slot.rs

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

• extend_class (126-194)

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

• find_slot_defs_by_name (1187-1189)
• inherit_from_mro (276-408)

• GitHub Check: Run snippets and cpython tests on wasm-wasi
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run rust tests (windows-latest)
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Ensure compilation on various targets
• GitHub Check: Run tests under miri
• GitHub Check: Check the WASM package and demo
• GitHub Check: Check Rust code with clippy

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

1-9: LGTM!

The module organization and public re-exports are clean. The new slot_defs module is properly exposed alongside existing exports.

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

466-472: LGTM!

The refactored inherit_slots is much cleaner than per-slot macros. The data-driven iteration via SLOT_DEFS improves maintainability and reduces boilerplate. The comment correctly notes that as_buffer has separate handling.

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

26-35: Function pointer comparison for hash_not_implemented is correct.

The pattern h as usize == hash_not_implemented as usize is a valid way to compare function pointers in Rust when the types match. This correctly identifies types that have been explicitly marked unhashable to set __hash__ = None in the type dict.

---

13-57: Well-structured add_operators function.

The implementation correctly:

1. Skips __new__ which has special handling
2. Handles __hash__ = None for unhashable types
3. Only creates wrappers for slots that have functions and don't already exist in the dict

This aligns well with CPython's add_operators behavior.

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

398-444: SlotFunc enum expansion looks comprehensive.

The new variants cover all necessary slot categories:

• Basic slots (Call, Del)
• Attribute access (GetAttro, SetAttro, DelAttro)
• Rich comparison with op parameter
• Descriptor protocol (DescrGet, DescrSet, DescrDel)
• Sequence/Mapping sub-slots
• Number sub-slots (boolean, unary, binary, ternary)

The design properly separates slots by their function signatures.

---

650-665: LGTM! Clean rename from PySlotWrapper to PyWrapper.

The struct definition and payload implementation are straightforward. The rename aligns with the PR's goal of matching CPython's PyWrapperDescrObject naming conventions.

---

599-604: The SeqAssItem argument binding correctly handles both __setitem__ and __delitem__ semantics.

When __delitem__ is called with a single argument, the OptionalArg<PyObjectRef> binding correctly interprets the missing second parameter as None. The OptionalArg enum declares arity() as 0..=1, and its from_args() implementation converts an absent argument to OptionalArg::Missing, which is then converted to None via into_option(). This design properly routes __delitem__ calls through SeqAssItem with the value parameter set to None.

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

350-373: Ternary wrapper macros handle optional modulus correctly.

The logic properly checks vm.is_none(c) to decide whether to pass mod argument. This matches Python's pow(x, y[, mod]) semantics where mod is optional.

---

549-561: Clean refactoring of update_slot to use SLOT_DEFS.

The method now iterates over all matching slot definitions for a given name and delegates to update_one_slot. This is more maintainable than the previous approach and correctly handles cases where one dunder name maps to multiple slots (e.g., __add__ for both sequence and number protocols).

---

1174-1214: lookup_slot_in_mro implementation is sound.

The function correctly:

1. Checks self's dict first
2. Falls through to MRO lookup
3. Returns None if attribute exists but isn't a wrapper descriptor (meaning a real method override)
4. Extracts the slot function only from PyWrapper instances

This ensures that user-defined Python methods properly override C-level slot wrappers.

---

1107-1120: The code is correct and matches CPython's behavior. In CPython, PySequence_Concat and PySequence_Repeat fall back to the numeric slots nb_add and nb_multiply when sequence slots are unavailable—exactly as the comment suggests. When ADD is true (number protocol present), the sequence slots don't require explicit update because CPython automatically handles the fallback to numeric operations. The !ADD branch correctly inherits sequence slots from the MRO when the number protocol is not defined.

crates/vm/src/types/slot_defs.rs (4)

1-21: Well-designed SlotDef structure.

The design with name, accessor, and doc fields provides a clean, extensible way to define slots. The static lifetime strings are appropriate for compile-time constants.

---

462-474: Init slot inheritance has special SLOTDEFINED logic.

The implementation correctly checks if the base type's init slot differs from its base's base - this matches CPython's behavior for SLOTDEFINED which prevents inheriting __init__ through multiple levels when intermediate classes don't define it. Good attention to detail.

---

869-934: Duplicate dunder names intentionally map to multiple slot accessors.

The SLOT_DEFS array correctly has entries like:

• __len__ → SeqLength and MapLength
• __getitem__ → SeqItem and MapSubscript
• __add__ → SeqConcat and NumAdd

This is by design to match CPython's slot system where the same dunder can trigger different C-level slots depending on the protocol. The find_slot_defs_by_name function returns all matching entries.

---

759-1184: Comprehensive SLOT_DEFS array covering all major protocols.

The array includes:

• Main slots (init, new, del, repr, str, hash, call, iter, next)
• Attribute access (getattribute, setattr, delattr)
• Rich comparison (eq, ne, lt, le, gt, ge)
• Descriptor protocol (get, set, delete)
• Sequence protocol (len, getitem, setitem, delitem, contains, add, mul, iadd, imul)
• Mapping protocol (len, getitem, setitem, delitem)
• Full numeric protocol (all binary, unary, and ternary operations)

This provides excellent coverage for the data-driven slot system.

Actionable comments posted: 0

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

292-305: Sequence/mapping sub-slot aliases and ternary number wrappers look consistent

The new Seq*Func / Map*Func aliases and their use in sequence/mapping update_sub_slot! arms line up with the protocol types and helpers like len_wrapper, getitem_wrapper, setitem_wrapper, and contains_wrapper. The ternary number wrapper macros correctly model __pow__(self, other, mod=None) and the right-hand variants by only passing the third argument when it’s not None, which matches Python’s pow semantics. The small Vec<PyObjectRef> allocation per call is acceptable for this path.

Also applies to: 350-373

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

3-10: SLOT_DEFS‑driven operator insertion via add_operators is well‑structured

add_operators’ logic:

• Skips __new__ (which is handled by the dedicated slot_new_wrapper binding) and maps hash_not_implemented to __hash__ = None for unhashable types, matching Python’s __hash__/unhashable semantics.
• Uses def.accessor.get_slot_func(&class.slots) to only create wrappers when the underlying slot is actually populated, and avoids clashes by skipping attributes that already exist in class.attributes.
• Resolves overlapping names like __len__ and __add__ correctly by relying on which protocol (sequence vs mapping vs number) has the slot set; the first non-None accessor wins and later duplicates see an existing attribute and are skipped.

Hooking this in from extend_class before inherit_slots and method extension is a clean replacement for the old per-slot macros.

If you find yourself frequently needing to reason about which protocol “owns” a name (e.g. __add__), consider adding a brief comment near the SLOT_DEFS entries that share names to document the intended precedence (sequence vs number vs mapping).

Also applies to: 13-57, 166-187

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

127-273: Inheritance and copy logic correctly walks the full MRO and all slot groups

The helper methods on SlotAccessor:

• inherit_from_mro walks the entire typ.mro (which excludes typ itself) without skipping the immediate base, and uses focused macros to handle main, number, sequence, and mapping slots; this fixes the earlier skip(1) issue called out in past reviews.
• copyslot_if_none mirrors CPython’s slot inheritance semantics, including the special Init handling (only copying when the base actually defines, rather than inherits, tp_init) and leaving New to be managed by set_new.
• Handle shared slots (SetAttro/DelAttro, DescrSet/DescrDel) correctly by copying/inheriting the underlying storage slot only once.

This makes both initial inheritance (PyType::inherit_slots) and per-attribute deletion updates (update_slot::<false>) coherent across all slot families.

Given how many match arms are kept in sync across inherit_from_mro, copyslot_if_none, and get_slot_func, consider a small macro or table-driven helper to reduce the risk of divergence when adding future slots.

Also applies to: 275-402, 404-552

---

554-747: get_slot_func and SLOT_DEFS entries align for all slot kinds, including ternary numbers

get_slot_func:

• Correctly wraps each populated slot field in the appropriate SlotFunc variant, including:
  • Binary number slots via get_number_slot → SlotFunc::NumBinary.
  • Ternary power slots as SlotFunc::NumTernary.
  • Unary, sequence, and mapping slots into their dedicated SlotFunc variants, with inplace sequence ops mapping back to the same underlying concat/repeat slot (matching CPython’s sharing).
• Respects the decision that __new__ has special handling and thus never produces a wrapper for it.

SLOT_DEFS itself:

• Covers the expected main, descriptor, sequence, mapping, and number dunders, with deliberate duplicates (__len__, __getitem__, __setitem__, __delitem__, __add__, etc.) to support multiple protocols, which is validated by the tests (test_find_by_name).
• Provides clear, CPython-like doc strings for wrapper descriptors, which add_operators can attach directly.

The unit tests around counts, name overlaps, size-of, and rich_compare_op give good basic coverage for this table.

SlotAccessor::extract_from_slot_func currently treats all RichCompare SlotFuncs as matching any RichCompare accessor, regardless of the embedded op. If you end up using it for finer-grained checks, you may want to also compare the PyComparisonOp to avoid accidental matches.

Also applies to: 753-1178

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between ade6c6e and 33e8547.

• Lib/test/test_descr.py is excluded by !Lib/**
• Lib/test/test_inspect/test_inspect.py is excluded by !Lib/**
• Lib/test/test_sqlite3/test_factory.py is excluded by !Lib/**
• Lib/test/test_unittest/testmock/testhelpers.py is excluded by !Lib/**

• .cspell.dict/cpython.txt
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/builtins/type.rs
• crates/vm/src/class.rs
• crates/vm/src/protocol/mod.rs
• crates/vm/src/types/mod.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/slot_defs.rs
• crates/vm/src/types/zoo.rs

• .cspell.dict/cpython.txt

📄 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/mod.rs
• crates/vm/src/types/zoo.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/protocol/mod.rs
• crates/vm/src/class.rs
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/types/slot_defs.rs
• crates/vm/src/builtins/type.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In RustPython's pattern matching implementation, only certain builtin types should have the SEQUENCE flag: list and tuple are confirmed sequences. The user youknowone indicated that bytes, bytearray are not considered sequences in this context, even though they implement sequence-like protocols.

Applied to files:

• crates/vm/src/types/slot.rs
• crates/vm/src/protocol/mod.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In the RustPython codebase, only certain builtin types should be marked with the SEQUENCE flag for pattern matching. List and tuple are sequences, but bytes, bytearray, and range are not considered sequences in this context, even though they may implement sequence-like protocols.

Applied to files:

• crates/vm/src/types/slot.rs
• crates/vm/src/protocol/mod.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/builtins/type.rs (1)

• slot (643-643)

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

• slot (69-69)
• slot (93-93)

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

• find_slot_defs_by_name (1181-1183)

crates/vm/src/protocol/number.rs (1)

• handle_bytes_to_int_err (558-578)

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

• hash_not_implemented (424-426)

• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Run rust tests (windows-latest)
• GitHub Check: Check Rust code with clippy
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Run tests under miri
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Ensure compilation on various targets
• GitHub Check: Check the WASM package and demo
• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Run snippets and cpython tests on wasm-wasi

crates/vm/src/protocol/mod.rs (1)

13-16: Expose PyNumberTernaryFunc to match new numeric ternary slots

Adding PyNumberTernaryFunc to the public number re-exports is consistent with the new SlotFunc::NumTernary and ternary number wrappers; no issues here.

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

549-561: MRO-based slot updating logic is sound and matches wrapper semantics

The new update_slot / update_one_slot flow, driven by SlotAccessor and find_slot_defs_by_name, correctly:

• Handles multiple slot defs per dunder (e.g. __len__ for sequence/mapping, __add__ for sequence/number) by updating each accessor separately.
• Prefers explicit wrapper descriptors over generic wrappers via lookup_slot_in_mro, and stops on the first non-wrapper attribute (treating it as a “real” override), which matches CPython’s “first hit in MRO” behavior.
• Gives __hash__ special treatment: respects __hash__ = None (mapping it to hash_not_implemented), otherwise prefers a real SlotFunc::Hash wrapper from the MRO, and only falls back to hash_wrapper when none is present.
• Shares slots correctly for SetAttro/DelAttro, DescrSet/DescrDel, and the various rich-compare accessors, while using accessor.inherit_from_mro(self) on deletion to restore the inherited slot.

Overall this centralizes slot wiring cleanly and should behave correctly for both builtin and heap types.

Also applies to: 572-611, 614-1171

---

1174-1214: lookup_slot_in_mro’s wrapper detection and early-return semantics are appropriate

The helper correctly:

• Recognizes wrapper descriptors via ctx.types.wrapper_descriptor_type and downcasts to PyWrapper to extract the inner SlotFunc.
• Returns None as soon as it encounters a non-wrapper attribute in self or the first MRO class, ensuring that a “real” Python method override prevents inheriting a base wrapper.
• Iterates the full MRO (which does not include self), avoiding the earlier skip(1) pitfall highlighted in past reviews.

This matches the documented contract (“Some(slot_func) if a wrapper, None otherwise”) and plays well with the new SLOT_DEFS-driven slot updates.

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

1-8: Public exposure of slot_defs is appropriate for the new slot system

Re-exporting SLOT_DEFS, SlotAccessor, and SlotDef from types::slot_defs cleanly centralizes slot metadata for consumers like builtins::type and class, without overexposing unrelated internals. Looks good.

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

185-193: Wrapper descriptor type wiring matches the new PyWrapper

Updating wrapper_descriptor_type to descriptor::PyWrapper::init_builtin_type() aligns the type zoo with the renamed/extended wrapper descriptor; it keeps ctx.types.wrapper_descriptor_type consistent with PyWrapper usages elsewhere (e.g., in lookup_slot_in_mro). No issues.

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

27-30: SLOT_DEFS‑driven slot initialization and inheritance look correct

Using SLOT_DEFS to:

• In init_slots: discover all dunder names in self and its MRO, then call update_slot::<true> once per unique name, ensures heap types wire all relevant slots based on the final attribute set.
• In inherit_slots: iterate SLOT_DEFS and delegate to def.accessor.copyslot_if_none(self, base) gives a uniform, order-preserving way to inherit slots across all groups (main, number, sequence, mapping), while leaving as_buffer to inherit_readonly_slots and tp_new to set_new.

The interaction with set_new (skipping SlotAccessor::New in copyslot_if_none) avoids clobbering tp_new, and the copy‑if‑none semantics remain compatible with multiple inheritance. This all aligns with the new slot_defs machinery.

Also applies to: 414-445, 465-472

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

7-17: Extended SlotFunc variants and dispatcher correctly cover new slot shapes

The expanded SlotFunc enum plus its call implementation:

• Enforces zero-argument contracts for slots like __hash__, __repr__, __str__, __iter__, __next__, and __del__, returning sensible TypeErrors when misused.
• Correctly binds arguments for attribute access (__getattribute__, __setattr__, __delattr__), rich comparison (returning either the raw object or PyComparisonValue converted via to_pyobject), and descriptor operations (__get__, __set__, __delete__).
• Uses the sequence/mapping views (sequence_unchecked(), mapping_unchecked()) and number view (number()) with the appropriate function types for Seq*/Map*/Num* variants, including ternary number slots where NumTernary receives (self, y, z) with z defaulted to None.

Overall the dispatcher matches the type signatures wired in types::slot and types::slot_defs, and should let PyWrapper/PyMethodWrapper uniformly expose all these slots as Python-callable descriptors.

Please run cargo test -p rustpython-vm --features full (or your usual subset) to exercise the new SlotFunc branches, especially for ternary numeric methods (__pow__, __rpow__, __ipow__) and seq/map __setitem__/__delitem__.

Also applies to: 400-444, 446-647

---

650-739: PyWrapper / PyMethodWrapper refactor preserves wrapper semantics

The rename to PyWrapper and the updated wiring:

• Keep the wrapper descriptor type consistent (ctx.types.wrapper_descriptor_type) and ensure lookup_slot_in_mro can downcast to PyWrapper.
• Maintain the descriptor protocol: PyWrapper::descr_get returns the descriptor when accessed on the class and a PyMethodWrapper bound to the instance when accessed on an object.
• Route calls for both unbound (Type.__foo__(obj, ...)) and bound (obj.__foo__(...)) cases through the same SlotFunc::call machinery, with the fast_isinstance(zelf.typ) guard enforcing correct receiver types.

This matches the previous PySlotWrapper behavior with the extended SlotFunc coverage.

Also applies to: 743-803, 817-838

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

10-23: Core slot metadata (SlotDef, SlotAccessor) design is consistent

The SlotDef + SlotAccessor setup:

• Gives a single authoritative place mapping dunder names to concrete slot accessors and docs, including shared-name cases like __len__ and __add__.
• Keeps SlotAccessor as a flat enum suitable for repr(u8) size constraints (validated by test_repr_u8), which is useful if you ever want to store it compactly.

This provides a solid foundation for SLOT_DEFS‑driven behavior in class.rs and slot.rs.

Also applies to: 27-123

Actionable comments posted: 2

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

465-472: SLOT_DEFS‑based inherit_slots looks correct; fix stale comment

The new loop over SLOT_DEFS with copyslot_if_none(self, base) is a clean replacement for per‑slot macros and matches the intended inheritance behavior. The only nit is the comment mentioning inherit_static_slots while the actual helper is inherit_readonly_slots, which can confuse future readers.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 33e8547 and 025c248.

• Lib/test/test_descr.py is excluded by !Lib/**
• Lib/test/test_inspect/test_inspect.py is excluded by !Lib/**
• Lib/test/test_sqlite3/test_factory.py is excluded by !Lib/**
• Lib/test/test_unittest/testmock/testhelpers.py is excluded by !Lib/**

• .cspell.dict/cpython.txt
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/builtins/type.rs
• crates/vm/src/class.rs
• crates/vm/src/protocol/mod.rs
• crates/vm/src/protocol/number.rs
• crates/vm/src/types/mod.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/slot_defs.rs
• crates/vm/src/types/zoo.rs

• crates/vm/src/protocol/mod.rs

• .cspell.dict/cpython.txt

📄 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/mod.rs
• crates/vm/src/class.rs
• crates/vm/src/builtins/type.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/protocol/number.rs
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/types/slot_defs.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In RustPython's pattern matching implementation, only certain builtin types should have the SEQUENCE flag: list and tuple are confirmed sequences. The user youknowone indicated that bytes, bytearray are not considered sequences in this context, even though they implement sequence-like protocols.

Applied to files:

• crates/vm/src/types/slot.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In the RustPython codebase, only certain builtin types should be marked with the SEQUENCE flag for pattern matching. List and tuple are sequences, but bytes, bytearray, and range are not considered sequences in this context, even though they may implement sequence-like protocols.

Applied to files:

• crates/vm/src/types/slot.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/class.rs (1)

• init_builtin_type (86-95)

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

• slot (69-69)
• slot (93-93)

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

• slot (643-643)

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

• hash_not_implemented (424-426)

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

• class (57-59)
• class (240-242)
• class (662-664)
• class (754-756)

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

• base (111-111)

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

• base (114-114)

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

• base (61-61)

• GitHub Check: Check the WASM package and demo
• GitHub Check: Run snippets and cpython tests on wasm-wasi
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run rust tests (windows-latest)
• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Ensure compilation on various targets
• GitHub Check: Run tests under miri
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Check Rust code with clippy

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

191-196: Rename to PyWrapper::init_builtin_type is consistent and safe

The field type and initialization pattern match other descriptor types; switching from PySlotWrapper to PyWrapper here is behavior-preserving.

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

2-8: Publicly re‑exporting slot_defs is reasonable

Making slot_defs a public module and re‑exporting SLOT_DEFS, SlotAccessor, and SlotDef provides a clear, centralized API for slot metadata with no local downsides.

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

13-57: add_operators wiring via SLOT_DEFS is solid

Centralizing wrapper_descriptor creation in add_operators (with __new__ skipping and hash_not_implemented ⇒ __hash__ = None) is a good replacement for the old macro boilerplate. Combined with the attribute‑existence guard and post‑processing in inherit_slots, this keeps behavior consistent while making slot wiring data‑driven; any remaining left/right or delete‑semantics tweaks belong in SlotAccessor::get_slot_func / SLOT_DEFS, not here.

Also applies to: 135-187

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

292-306: SLOT_DEFS‑driven update_slot implementation looks correct

The new update_slot/update_one_slot logic cleanly routes each dunder name through SlotAccessor, uses appropriate wrapper functions for number/sequence/mapping sub‑slots, and defers to inherit_from_mro on deletion. The __hash__ special‑case and use of lookup_slot_in_mro for existing PyWrappers match CPython’s behavior, and inherit_from_mro now walks the full MRO (no skipped immediate base), addressing the earlier inheritance gap.

Also applies to: 335-373, 549-585, 614-708, 710-1183

Actionable comments posted: 1

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

350-373: Consider avoiding Vec allocation in ternary wrappers.

The ternary wrappers allocate a Vec on every call. Since call_special_method accepts impl IntoPyArgs, you could use a tuple or array instead for zero-allocation.

🔎 Proposed optimization

 macro_rules! number_ternary_op_wrapper {
     ($name:ident) => {
         |a, b, c, vm: &VirtualMachine| {
-            let args: Vec<PyObjectRef> = if vm.is_none(c) {
-                vec![b.to_owned()]
+            if vm.is_none(c) {
+                vm.call_special_method(a, identifier!(vm, $name), (b.to_owned(),))
             } else {
-                vec![b.to_owned(), c.to_owned()]
-            };
-            vm.call_special_method(a, identifier!(vm, $name), args)
+                vm.call_special_method(a, identifier!(vm, $name), (b.to_owned(), c.to_owned()))
+            }
         }
     };
 }

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 025c248 and a6dbe23.

• Lib/test/test_descr.py is excluded by !Lib/**
• Lib/test/test_inspect/test_inspect.py is excluded by !Lib/**
• Lib/test/test_sqlite3/test_factory.py is excluded by !Lib/**
• Lib/test/test_unittest/testmock/testhelpers.py is excluded by !Lib/**

• .cspell.dict/cpython.txt
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/builtins/type.rs
• crates/vm/src/class.rs
• crates/vm/src/protocol/mod.rs
• crates/vm/src/protocol/number.rs
• crates/vm/src/types/mod.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/slot_defs.rs
• crates/vm/src/types/zoo.rs

• .cspell.dict/cpython.txt
• crates/vm/src/types/mod.rs
• crates/vm/src/protocol/number.rs
• crates/vm/src/builtins/type.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 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/protocol/mod.rs
• crates/vm/src/class.rs
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/zoo.rs
• crates/vm/src/types/slot_defs.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In the RustPython codebase, only certain builtin types should be marked with the SEQUENCE flag for pattern matching. List and tuple are sequences, but bytes, bytearray, and range are not considered sequences in this context, even though they may implement sequence-like protocols.

Applied to files:

• crates/vm/src/protocol/mod.rs
• crates/vm/src/types/slot.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In RustPython's pattern matching implementation, only certain builtin types should have the SEQUENCE flag: list and tuple are confirmed sequences. The user youknowone indicated that bytes, bytearray are not considered sequences in this context, even though they implement sequence-like protocols.

Applied to files:

• crates/vm/src/protocol/mod.rs
• crates/vm/src/types/slot.rs

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.011Z
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/protocol/number.rs (1)

• handle_bytes_to_int_err (558-578)

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

• find_slot_defs_by_name (869-871)
• inherit_from_mro (408-515)

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

• init_builtin_type (86-95)

• GitHub Check: Run rust tests (windows-latest)
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Run snippets and cpython tests on wasm-wasi
• GitHub Check: Ensure compilation on various targets
• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Run tests under miri
• GitHub Check: Check Rust code with clippy
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Check the WASM package and demo

crates/vm/src/protocol/mod.rs (1)

13-16: LGTM!

Trivial reordering of exports within the PyNumber group. No functional changes.

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

192-192: LGTM!

Correctly updates to use the renamed PyWrapper type, aligning with the broader refactor from PySlotWrapper to PyWrapper.

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

3-10: LGTM!

Imports correctly updated to bring in PyWrapper from the descriptor module and SLOT_DEFS, hash_not_implemented from the types module.

---

13-57: Well-structured SLOT_DEFS-driven operator addition.

The add_operators function cleanly replaces the previous macro-based approach with a data-driven loop. Key observations:

• Correctly skips __new__ which has special handling elsewhere
• Properly handles __hash__ = None for unhashable types by detecting hash_not_implemented
• Uses get_slot_func_with_op to correctly wire right-hand and delete variants
• Respects existing attributes to avoid overwriting user-defined methods

---

185-186: LGTM!

Correctly integrates the new add_operators call into the class extension flow.

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

7-16: LGTM!

Imports correctly updated to include new slot function types (SeqLenFunc, MapSubscriptFunc, etc.) and ToPyResult for the expanded SlotFunc::call implementation.

---

394-396: LGTM!

Correctly updates to use the renamed PyWrapper type in the init function.

---

400-444: Comprehensive SlotFunc enum expansion.

The enum now covers all slot categories (basic, attribute access, rich compare, descriptor, sequence, mapping, number) with appropriate function pointer types. The separation of SetAttro/DelAttro and DescrSet/DescrDel correctly enables distinct delete semantics.

---

484-647: SlotFunc::call correctly dispatches to all slot variants.

The implementation:

• Properly validates argument counts for no-arg methods
• Uses PySetterValue::Assign vs PySetterValue::Delete appropriately for setter/deleter variants
• Correctly binds arguments via args.bind(vm)?
• Uses sequence_unchecked() and mapping_unchecked() which is safe because PyWrapper::call validates the type before reaching here

One observation: the SeqAssItem and MapAssSubscript branches use OptionalArg for the value parameter, which correctly handles both __setitem__(key, value) and __delitem__(key) calling conventions.

---

650-739: PyWrapper (formerly PySlotWrapper) correctly implements wrapper_descriptor.

The rename is consistent throughout, and the implementations for GetDescriptor, Callable, and Representable are properly adapted. The type check in Callable::call (line 691) ensures the object is an instance of the expected type before delegating to SlotFunc::call.

---

743-785: LGTM!

PyMethodWrapper correctly updated to hold PyRef<PyWrapper> instead of the old type. The Callable implementation properly delegates to the wrapped slot function.

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

17-17: LGTM!

Import correctly added for the new SLOT_DEFS infrastructure.

---

292-305: LGTM!

New type aliases for sequence and mapping sub-slot functions match the protocol method signatures and enable type-safe slot storage.

---

549-612: Clean refactor to SLOT_DEFS-driven slot updates.

The new update_slot and update_one_slot methods provide a centralized, data-driven approach to slot management. The macros update_main_slot and update_sub_slot correctly:

1. Look up real implementations from the MRO via lookup_slot_in_mro
2. Fall back to wrapper functions when no native implementation exists
3. Delegate to inherit_from_mro when deleting (ADD = false)

---

618-644: Correct special handling for __hash__ = None.

The logic properly detects when a class explicitly sets __hash__ = None (making it unhashable) by checking if the method object is the context's None singleton, and correctly stores hash_not_implemented in that case.

---

659-689: Shared slot handling is correct.

TpSetattro correctly handles both __setattr__ and __delattr__ by accepting either SetAttro or DelAttro variants from the MRO lookup. Same pattern correctly applied for TpDescrSet handling __set__ and __delete__.

---

711-727: Binary op left/right dispatch is correct.

The code correctly distinguishes between __add__ and __radd__ by checking the method name string, then updates the appropriate slot field (add vs right_add). This pattern is consistently applied across all binary number operations.

---

1186-1226: LGTM!

lookup_slot_in_mro correctly searches for wrapper descriptors in the type's dict and MRO chain, returning None when a non-wrapper (real method) is found to trigger the wrapper fallback path.

crates/vm/src/types/slot_defs.rs (7)

1-48: LGTM!

SlotOp enum cleanly distinguishes comparison operations, binary op direction (left/right), and delete operations. The helper methods as_compare_op() and is_right() are correct.

---

50-167: LGTM!

SlotDef and SlotAccessor provide a clean, CPython-compatible slot definition system. The #[repr(u8)] enum values correctly match CPython's typeslots.h IDs.

---

407-515: LGTM!

The inherit_from_mro method correctly iterates the MRO (without skip, addressing the previous review comment) to find and inherit slot implementations.

---

785-865: Right-hand and delete variants are correctly wired.

get_slot_func_with_op properly handles:

• SlotOp::Delete → returns DelAttro/DescrDel variants
• SlotOp::Right → uses right_* slot fields for binary number operations

This addresses the previous review comment about mis-wired delete and right-hand variants.

---

868-874: LGTM!

find_slot_defs_by_name provides efficient lookup by name, correctly returning an iterator for cases like __len__ that appear in multiple protocols.

---

876-1385: Comprehensive SLOT_DEFS array covering all Python dunder methods.

The array correctly:

• Distinguishes left/right variants for binary ops via SlotOp::Left/Right
• Marks delete operations with SlotOp::Delete for __delattr__ and __delete__
• Includes both sequence (SqLength, SqItem) and mapping (MpLength, MpSubscript) variants for shared methods like __len__ and __getitem__
• Sets appropriate comparison ops for rich comparison methods

---

1387-1443: LGTM!

Tests validate CPython slot ID compatibility and find_slot_defs_by_name behavior. The test at line 1429 correctly expects 2 entries for __add__ (NbAdd with Left and SqConcat).

Actionable comments posted: 1

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

292-306: Slot update machinery is solid but doesn’t yet propagate right‑hand number slots

The new update_slot / update_one_slot implementation (with update_main_slot / update_sub_slot) correctly wires:

• main slots (repr/str/hash/call/iter/init/new/del/getattro/descr*/richcompare) via wrappers and MRO lookup, including __hash__ = None and shared slots for __setattr__/__delattr__ and __set__/__delete__;
• number, sequence, and mapping sub‑slots through the appropriate SlotFunc variants and wrapper closures;
• AsNumber traits into PyTypeSlots via slots.as_number.copy_from(Self::as_number()).

The only notable gap is consistency with the new right‑hand number fields:

• inherit_from_mro and copyslot_if_none in SlotAccessor (see crates/vm/src/types/slot_defs.rs) currently copy only left‑hand number slots; right_* fields are never inherited/copied from bases.
• Combined with copy_from not populating right_* (see the comment in protocol/number.rs), __r*__ wrappers created via SLOT_DEFS will have no underlying slot to bind for many types, and right‑side slots won’t be inherited.

Once copy_from is fixed to populate right_* slots, consider extending inherit_from_mro/copyslot_if_none to propagate the right_* counterparts as well so derived numeric types fully inherit both sides of the protocol.

Also applies to: 335-371, 547-782, 784-910, 1710-1716

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 520c965 and edd2713.

• .cspell.dict/cpython.txt
• crates/vm/src/builtins/bool.rs
• crates/vm/src/builtins/complex.rs
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/builtins/float.rs
• crates/vm/src/builtins/int.rs
• crates/vm/src/builtins/type.rs
• crates/vm/src/class.rs
• crates/vm/src/protocol/number.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/slot_defs.rs

• crates/vm/src/builtins/complex.rs
• crates/vm/src/builtins/float.rs

• .cspell.dict/cpython.txt
• crates/vm/src/builtins/type.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 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/bool.rs
• crates/vm/src/class.rs
• crates/vm/src/protocol/number.rs
• crates/vm/src/builtins/int.rs
• crates/vm/src/types/slot_defs.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/builtins/descriptor.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In RustPython's pattern matching implementation, only certain builtin types should have the SEQUENCE flag: list and tuple are confirmed sequences. The user youknowone indicated that bytes, bytearray are not considered sequences in this context, even though they implement sequence-like protocols.

Applied to files:

• crates/vm/src/types/slot.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In the RustPython codebase, only certain builtin types should be marked with the SEQUENCE flag for pattern matching. List and tuple are sequences, but bytes, bytearray, and range are not considered sequences in this context, even though they may implement sequence-like protocols.

Applied to files:

• crates/vm/src/types/slot.rs

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.011Z
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/builtins/int.rs (3)

• __or__ (332-334)
• __and__ (336-338)
• __xor__ (328-330)

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

• __or__ (960-962)

crates/vm/src/builtins/dict.rs (4)

• __or__ (298-306)
• __or__ (1069-1073)
• __and__ (1061-1065)
• __xor__ (1053-1057)

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

• __or__ (245-247)

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

• __or__ (598-608)
• __or__ (1043-1053)
• __and__ (612-626)
• __and__ (1057-1071)
• __xor__ (664-678)
• __xor__ (1110-1124)

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

• hash_not_implemented (422-424)

crates/vm/src/builtins/bool.rs (3)

• __xor__ (161-173)
• __or__ (133-145)
• __and__ (147-159)

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

• init (391-396)

crates/vm/src/protocol/number.rs (3)

• int (568-598)
• float (634-664)
• index (601-631)

crates/vm/src/protocol/sequence.rs (1)

• inplace_repeat (216-232)

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

• find_slot_defs_by_name (914-916)

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

• extend_slots (215-215)

• GitHub Check: Run snippets and cpython tests on wasm-wasi
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run tests under miri
• GitHub Check: Check the WASM package and demo
• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Check Rust code with clippy
• GitHub Check: Run snippets and cpython tests (macos-latest)

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

13-57: add_operators logic and integration into extend_class look sound

The SLOT_DEFS‑driven add_operators helper correctly:

• skips __new__,
• maps hash_not_implemented → __hash__ = None,
• respects existing attributes, and
• wraps only slots with an installed function.

Calling it once in extend_class before inherit_slots keeps builtin slot wiring coherent while avoiding double‑wrapping inherited Python methods. No issues from a correctness perspective.

Also applies to: 185-187

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

131-174: Bitwise helpers for bool moved to slot-backed functions are OK

The crate-private __or__, __and__, and __xor__ maintain the existing semantics (bool‑vs‑bool short‑circuit, otherwise delegate to PyInt or return NotImplemented) and are wired via AsNumber into slots. The refactor away from #[pymethod] looks behaviour‑preserving.

Also applies to: 176-186

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

322-339: Int arithmetic helpers and relocated __int__ are consistent with slot usage

The new crate-private __xor__, __or__, and __and__ simply reuse int_op, and moving __int__ onto PyRef<PyInt> aligns with its use in __trunc__, __floor__, real, numerator, etc. Together with PyInt::AS_NUMBER, this keeps number slots coherent without changing observable behaviour.

Also applies to: 382-387, 424-437, 530-543, 566-576

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

400-445: SlotFunc expansion and PyWrapper plumbing are coherent

The extended SlotFunc enum plus its call implementation cover all new slot kinds (attribute access, rich compare, descriptor get/set/del, sequence/mapping, number left/right/ternary) with correct argument binding and return handling. The PySlotWrapper→PyWrapper rename, along with updated GetDescriptor/Callable and PyMethodWrapper wiring, is internally consistent and matches how SLOT_DEFS and update_slot consume SlotFunc.

Also applies to: 486-653, 656-745, 747-829

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 edd2713 and 112b79a.

• Lib/test/test_descr.py is excluded by !Lib/**
• Lib/test/test_inspect/test_inspect.py is excluded by !Lib/**
• Lib/test/test_sqlite3/test_factory.py is excluded by !Lib/**
• Lib/test/test_unittest/testmock/testhelpers.py is excluded by !Lib/**

• .cspell.dict/cpython.txt
• crates/vm/src/builtins/bool.rs
• crates/vm/src/builtins/complex.rs
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/builtins/float.rs
• crates/vm/src/builtins/int.rs
• crates/vm/src/builtins/type.rs
• crates/vm/src/class.rs
• crates/vm/src/protocol/mod.rs
• crates/vm/src/protocol/number.rs
• crates/vm/src/types/mod.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/slot_defs.rs
• crates/vm/src/types/zoo.rs

• .cspell.dict/cpython.txt

• 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 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/mod.rs
• crates/vm/src/builtins/type.rs
• crates/vm/src/class.rs
• crates/vm/src/builtins/bool.rs
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/protocol/mod.rs
• crates/vm/src/builtins/int.rs
• crates/vm/src/protocol/number.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/slot_defs.rs
• crates/vm/src/builtins/complex.rs
• crates/vm/src/builtins/float.rs

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.011Z
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/builtins/complex.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In the RustPython codebase, only certain builtin types should be marked with the SEQUENCE flag for pattern matching. List and tuple are sequences, but bytes, bytearray, and range are not considered sequences in this context, even though they may implement sequence-like protocols.

Applied to files:

• crates/vm/src/protocol/mod.rs
• crates/vm/src/types/slot.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In RustPython's pattern matching implementation, only certain builtin types should have the SEQUENCE flag: list and tuple are confirmed sequences. The user youknowone indicated that bytes, bytearray are not considered sequences in this context, even though they implement sequence-like protocols.

Applied to files:

• crates/vm/src/protocol/mod.rs
• crates/vm/src/types/slot.rs

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

• slot (69-69)
• slot (93-93)

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

• slot (643-643)

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

• hash_not_implemented (422-424)

crates/vm/src/builtins/int.rs (3)

• __or__ (331-333)
• __and__ (335-337)
• __xor__ (327-329)

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

• __or__ (960-962)

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

• __or__ (201-203)

crates/vm/src/builtins/dict.rs (4)

• __or__ (298-306)
• __or__ (1069-1073)
• __and__ (1061-1065)
• __xor__ (1053-1057)

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

• __or__ (245-247)

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

• __or__ (598-608)
• __or__ (1043-1053)
• __and__ (612-626)
• __and__ (1057-1071)
• __xor__ (664-678)
• __xor__ (1110-1124)

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

• obj (1634-1635)

crates/vm/src/protocol/number.rs (1)

• handle_bytes_to_int_err (667-687)

crates/vm/src/builtins/bool.rs (3)

• __xor__ (161-173)
• __or__ (133-145)
• __and__ (147-159)

crates/vm/src/protocol/mapping.rs (1)

• copy_from (37-47)

crates/vm/src/protocol/sequence.rs (1)

• copy_from (44-69)

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

• find_slot_defs_by_name (918-920)

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

• extend_slots (215-215)

• GitHub Check: Check Rust code with clippy
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run rust tests (windows-latest)
• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Check the WASM package and demo
• GitHub Check: Ensure compilation on various targets
• GitHub Check: Run snippets and cpython tests on wasm-wasi

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

8-8: LGTM - Clean import update

The import has been focused to only include PyComparisonValue, aligning with the removal of arithmetic methods that are now handled through the AsNumber trait.

---

344-378: LGTM - Arithmetic operations properly centralized

The AsNumber implementation correctly provides all arithmetic operations (add, subtract, multiply, power, negative, positive, absolute, boolean, true_divide) through the slot system. This is consistent with the PR's goal of moving to a SLOT_DEFS-driven architecture.

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

389-432: LGTM - Consistent refactor pattern

The AsNumber implementation for PyFloat correctly provides all arithmetic operations through the slot system, consistent with the broader refactor to centralize numeric operations.

crates/vm/src/protocol/mod.rs (1)

13-16: LGTM - Export ordering update

The reordering of PyNumberTernaryFunc before PyNumberTernaryOp maintains consistency with the updated slot/wrapper infrastructure. No functional changes.

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

2-7: LGTM - New slot_defs module properly exposed

The new slot_defs module and its re-exported items (SLOT_DEFS, SlotAccessor, SlotDef) establish the public API for the data-driven slot system, which is central to this refactor.

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

467-471: LGTM - Clean migration to SLOT_DEFS-driven slot inheritance

The replacement of per-slot macros with a generic loop over SLOT_DEFS is a significant simplification. The comment correctly documents that as_buffer is handled separately in inherit_readonly_slots.

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

18-57: LGTM - Well-structured SLOT_DEFS iteration with appropriate special cases

The add_operators function correctly:

1. Iterates through SLOT_DEFS to create wrapper descriptors
2. Skips __new__ which has special handling in extend_class (lines 169-183)
3. Sets __hash__ to None when hash_not_implemented is set, matching CPython behavior
4. Avoids overwriting existing attributes in the type dict
5. Creates PyWrapper descriptors with proper documentation

The accessor-based slot function retrieval (def.accessor.get_slot_func_with_op) is the right approach for the new architecture.

---

186-186: LGTM - Proper integration of SLOT_DEFS-driven operator wrapping

The call to add_operators correctly replaces the previous macro-driven wrapper initialization, centralizing the logic for creating slot wrapper descriptors.

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

131-174: LGTM - Correct refactor to crate-internal methods with slot exposure

The bitwise operations (__or__, __and__, __xor__) are correctly changed to pub(crate) functions that are exposed via the AsNumber slots (lines 179-181) and wrapped as descriptors by the SLOT_DEFS system. The comment on line 132 helpfully clarifies this pattern.

crates/vm/src/builtins/int.rs (3)

326-337: LGTM - Bitwise operations correctly refactored for slot exposure

The bitwise operations (__xor__, __or__, __and__) are appropriately changed to pub(crate) and exposed via the AS_NUMBER slot table (lines 642-644). The comment clarifies this is called from both bool.rs and exposed as wrapper descriptors.

---

567-574: LGTM - __int__ properly exposed via slot system

The __int__ method is correctly moved to a pub(crate) function in the PyRef<PyInt> impl and wired through the AS_NUMBER.int slot (line 645). The comments on lines 567-568 helpfully document the multiple call sites and exposure mechanism.

---

618-654: LGTM - Comprehensive AS_NUMBER slot implementation

The AS_NUMBER constant correctly defines all numeric operations for PyInt, including:

• Basic arithmetic (add, subtract, multiply, remainder, divmod, power)
• Unary operations (negative, positive, absolute, boolean, invert)
• Bit operations (lshift, rshift, and, xor, or)
• Conversions (int, float, index)
• Division operations (floor_divide, true_divide)

This centralizes all numeric behavior in the slot system, consistent with the PR's architecture.

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

400-662: LGTM! SlotFunc expansion and dispatch logic are well-implemented.

The extensive SlotFunc enum now covers all major Python protocols (attributes, descriptors, rich comparison, sequences, mappings, and numbers) with proper variants for right-hand operations, delete operations, and sub-slots. The call method correctly handles each variant's calling convention:

• Right-hand binary/ternary operations properly swap arguments (lines 646, 658)
• Delete operations use PySetterValue::Delete (lines 554, 582)
• All argument binding, type conversions, and return value handling are appropriate for each protocol

This provides a solid foundation for the data-driven slot system.

---

394-394: Rename from PySlotWrapper to PyWrapper is clean and consistent.

The initialization, type references, and all related code have been updated to use PyWrapper throughout, maintaining consistency with the new naming.

crates/vm/src/protocol/number.rs (2)

355-462: LGTM! copy_from method correctly implements slot copying pattern.

The implementation follows the established pattern from PyMappingMethods and PySequenceMethods (as shown in relevant snippets), using if let Some(f) guards to conditionally copy each slot. This enables dynamic slot propagation from PyNumberMethods into PyNumberSlots, supporting the broader shift to SLOT_DEFS-driven slot management.

---

260-270: Right-hand slot additions are well-structured and documented.

The new right_* fields provide dedicated storage for reverse binary operations (__radd__, __rsub__, etc.), and the comment correctly notes these are internal. The initialization in From<&PyNumberMethods> (lines 319-349) appropriately reuses the same function pointers for native types, aligning with Python's semantics where builtin types typically implement commutative operations with a single underlying function.

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

350-371: Ternary operation wrapper macros correctly handle optional third argument.

The number_ternary_op_wrapper! and number_ternary_right_op_wrapper! macros properly check vm.is_none(c) to determine whether to pass the third argument, matching Python's pow(x, y) vs pow(x, y, z) semantics. The right variant correctly swaps the first two arguments for reverse operations.

---

1187-1224: lookup_slot_in_mro correctly searches for wrapper descriptors in the MRO.

The helper first checks the class's own attributes, then walks the MRO, extracting the slot function if a PyWrapper is found with a matching SlotFunc variant. Returning None when a non-wrapper method is found ensures user-defined methods take precedence over inherited slot functions, which is the correct Python behavior.

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

1-920: LGTM! Comprehensive slot system implementation with CPython compatibility.

This new module provides a data-driven slot system that:

• Defines SlotOp, SlotDef, and SlotAccessor with values matching CPython's typeslots.h (lines 15-167)
• Implements inheritance (inherit_from_mro, copyslot_if_none) correctly, with the comment at line 409 accurately noting that typ.mro does NOT include typ itself
• Provides get_slot_func_with_op (lines 785-914) that properly handles SlotOp::Delete for delete operations (lines 792-798), SlotOp::Right for reverse binary operations (lines 800-902), and comparison ops (lines 904-911)
• Exposes SLOT_DEFS and find_slot_defs_by_name for consumption by other modules

The implementation addresses all issues raised in past reviews and provides a solid foundation for the centralized slot dispatch system.

---

922-1434: SLOT_DEFS array is comprehensive and well-documented.

The static array covers all Python special methods across:

• Type slots (__init__, __new__, __del__, __repr__, __str__, __hash__, __call__, __iter__, __next__)
• Attribute access (__getattribute__, __setattr__, __delattr__)
• Rich comparisons (all six comparison operators)
• Descriptor protocol (__get__, __set__, __delete__)
• Sequence/mapping protocols (__len__, __getitem__, __setitem__, __delitem__, __contains__)
• Number protocol (binary ops with left/right variants, unary ops, in-place ops)

Each entry correctly specifies the accessor, operation type (where applicable), and documentation string. The use of SlotOp::Left/Right for binary operations and SlotOp::Delete for delete operations enables proper dispatch via get_slot_func_with_op.

Actionable comments posted: 0

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

324-404: Docstring mismatch: extract_from_slot_func returns bool, not a function pointer.

The docstring at line 324 says "Extract the raw function pointer from a SlotFunc" but the method only checks type compatibility via matches!() and returns a bool. Consider renaming to matches_slot_func or updating the docstring to clarify it performs type compatibility checking.

🔎 Suggested fix

-    /// Extract the raw function pointer from a SlotFunc if it matches this accessor's type
+    /// Check if a SlotFunc variant is compatible with this accessor's expected type
     pub fn extract_from_slot_func(&self, slot_func: &SlotFunc) -> bool {

Or rename the method:

-    pub fn extract_from_slot_func(&self, slot_func: &SlotFunc) -> bool {
+    pub fn matches_slot_func(&self, slot_func: &SlotFunc) -> bool {

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

366-371: Overflow detection logic is correct.

The check properly detects when norm() overflows for finite inputs. Minor style nit: .to_owned() on line 369 may be unnecessary for consistency with line 127 which passes a string literal directly to new_overflow_error.

Optional: remove unnecessary `.to_owned()`

 if result.is_infinite() && value.re.is_finite() && value.im.is_finite() {
-    return Err(vm.new_overflow_error("absolute value too large".to_owned()));
+    return Err(vm.new_overflow_error("absolute value too large"));
 }

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

744-760: Verify right-hand operation SlotFunc matching in update_sub_slot.

Looking at lines 744-760 for __radd__, the update_sub_slot! macro uses NumBinary as the match pattern. However, per past review comments, add_operators creates wrappers with SlotFunc::NumBinaryRight for right-hand operations.

The macro at line 595-601 matches against SlotFunc::$variant(f), so when looking up __radd__ in the MRO, it will try to match SlotFunc::NumBinary but the inherited wrapper contains SlotFunc::NumBinaryRight. This mismatch means lookup_slot_in_mro won't find inherited right-hand wrappers.

While the fallback wrapper will work correctly, this prevents optimization from reusing parent class slot functions.

🔎 Suggested fix direction

For right-hand operations, use the correct variant in the match:

 SlotAccessor::NbAdd => {
     if name.as_str() == "__radd__" {
         update_sub_slot!(
             as_number,
             right_add,
             number_binary_right_op_wrapper!(__radd__),
-            NumBinary
+            NumBinaryRight
         )

Apply similar changes to all __r*__ operations (__rsub__, __rmul__, etc.) and __rpow__ (use NumTernaryRight).

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between e7de2be and 330139a.

• Lib/test/test_descr.py is excluded by !Lib/**
• Lib/test/test_inspect/test_inspect.py is excluded by !Lib/**
• Lib/test/test_sqlite3/test_factory.py is excluded by !Lib/**
• Lib/test/test_unittest/testmock/testhelpers.py is excluded by !Lib/**

• .cspell.dict/cpython.txt
• crates/vm/src/builtins/bool.rs
• crates/vm/src/builtins/complex.rs
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/builtins/float.rs
• crates/vm/src/builtins/int.rs
• crates/vm/src/builtins/type.rs
• crates/vm/src/class.rs
• crates/vm/src/protocol/mod.rs
• crates/vm/src/protocol/number.rs
• crates/vm/src/types/mod.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/slot_defs.rs
• crates/vm/src/types/zoo.rs

• crates/vm/src/protocol/mod.rs
• crates/vm/src/builtins/type.rs
• crates/vm/src/types/zoo.rs
• crates/vm/src/types/mod.rs
• crates/vm/src/protocol/number.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 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/bool.rs
• crates/vm/src/builtins/descriptor.rs
• crates/vm/src/class.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/builtins/int.rs
• crates/vm/src/builtins/complex.rs
• crates/vm/src/builtins/float.rs
• crates/vm/src/types/slot_defs.rs

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.011Z
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/class.rs
• crates/vm/src/builtins/complex.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In RustPython's pattern matching implementation, only certain builtin types should have the SEQUENCE flag: list and tuple are confirmed sequences. The user youknowone indicated that bytes, bytearray are not considered sequences in this context, even though they implement sequence-like protocols.

Applied to files:

• crates/vm/src/types/slot.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6110
File: vm/src/frame.rs:1311-1316
Timestamp: 2025-08-26T05:20:54.540Z
Learning: In the RustPython codebase, only certain builtin types should be marked with the SEQUENCE flag for pattern matching. List and tuple are sequences, but bytes, bytearray, and range are not considered sequences in this context, even though they may implement sequence-like protocols.

Applied to files:

• crates/vm/src/types/slot.rs

crates/vm/src/builtins/int.rs (3)

• __or__ (332-334)
• __and__ (336-338)
• __xor__ (328-330)

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

• __or__ (965-967)

crates/vm/src/builtins/dict.rs (4)

• __or__ (298-306)
• __or__ (1069-1073)
• __and__ (1061-1065)
• __xor__ (1053-1057)

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

• __or__ (201-203)

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

• __or__ (245-247)

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

• __or__ (598-608)
• __or__ (1043-1053)
• __and__ (612-626)
• __and__ (1057-1071)
• __xor__ (664-678)
• __xor__ (1110-1124)

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

• find_slot_defs_by_name (917-919)

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

• extend_slots (216-216)

crates/vm/src/builtins/bool.rs (3)

• __xor__ (161-173)
• __or__ (133-145)
• __and__ (147-159)

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

• hash (832-836)
• call (105-107)
• call (490-661)
• call (701-715)
• call (776-778)
• new (43-53)

• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Run rust tests (windows-latest)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Check the WASM package and demo
• GitHub Check: Run snippets and cpython tests on wasm-wasi
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Check Rust code with clippy
• GitHub Check: Ensure compilation on various targets
• GitHub Check: Run tests under miri

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

8-8: LGTM!

Import cleanup aligns with the refactoring that moves arithmetic operations to the AsNumber trait implementation.

crates/vm/src/builtins/float.rs (3)

11-11: LGTM: Import inlining looks correct.

The inlined PyArithmeticValue::* import is properly used in the Comparable implementation (lines 376, 378).

---

389-432: No issues found. The number_downcast and number_downcast_exact methods are defined as part of the AsNumber trait in crates/vm/src/types/slot.rs (lines 1758 and 1763) and are available to all types implementing AsNumber, including PyFloat.

---

215-339: No breaking changes from removed operator methods.

Verification confirms that the removal of operator methods (__add__, __sub__, __mul__, __truediv__, __floordiv__, __mod__, __pow__, __divmod__, __abs__, __bool__, __int__, __float__, and reverse variants) from PyFloat does not break any Rust code. While similar method names exist on other types (PyDeque, PyInt, PySet, PyHkey), no direct calls to removed PyFloat methods were found in the codebase.

.cspell.dict/cpython.txt (1)

4-4: LGTM!

The dictionary additions (attro, releasebuffer) align with the new slot accessor terminology introduced in this PR (e.g., TpGetattro, TpSetattro, BfReleaseBuffer).

Also applies to: 54-54

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

131-174: LGTM!

The operator methods are correctly refactored to pub(crate) visibility for internal slot dispatch. The logic properly handles:

1. Fast-path for bool-bool operands returning Python bools
2. Delegation to PyInt operations for integer operands
3. NotImplemented fallback for unsupported types

This aligns well with the SLOT_DEFS-driven wrapper approach.

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

327-338: LGTM!

The bitwise operators are appropriately changed to pub(crate) visibility, aligning with the wrapper_descriptor exposure pattern. These are correctly called from PyBool's implementations (as shown in the relevant code snippets).

---

568-576: LGTM!

Moving __int__ to PyRef<PyInt> with pub(crate) visibility is appropriate. The method is correctly used by __trunc__, __floor__, __ceil__, conjugate, and the real/numerator properties, and exposed via AS_NUMBER.int slot.

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

18-57: LGTM!

The add_operators function is well-structured:

1. Correctly skips __new__ which has special handling via slot_new_wrapper
2. Properly handles __hash__ = None by checking for hash_not_implemented and setting the attribute to None
3. Uses get_slot_func_with_op to correctly resolve SlotFunc variants (including right-hand and delete operations)
4. Avoids overwriting existing attributes in the type dict

The function pointer comparison at line 31 (h as usize == hash_not_implemented as usize) is an acceptable pattern for comparing function pointers in Rust.

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

668-716: LGTM!

PyWrapper is correctly structured with:

• Static type and name references
• SlotFunc wrapped callable
• Optional documentation

The GetDescriptor and Callable implementations properly:

1. Return the descriptor itself when accessed from class (obj: None)
2. Create bound PyMethodWrapper when accessed from instance
3. Validate type compatibility in call() before delegating to SlotFunc::call

---

575-584: Argument binding for descriptor __set__ and __delete__ is correct.

The SlotFunc::DescrSet variant properly binds two required arguments (instance, value) and passes PySetterValue::Assign(value) to the underlying function. This correctly implements Python's __set__(self, instance, value) protocol. The related SlotFunc::DescrDel binds only (instance) and passes PySetterValue::Delete, correctly implementing __delete__(self, instance). The design of using the same DescrSetFunc type with different PySetterValue variants to distinguish between set and delete operations is sound.

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

690-741: Rich comparison slot update logic is well-designed.

The implementation correctly:

1. Checks if any Python-defined comparison method exists in self or MRO
2. Filters out wrapper_descriptor types to detect actual Python overrides
3. Falls back to inherited slot_richcompare if available
4. Uses richcompare_wrapper as the default

This ensures that Python-defined comparison methods take precedence over inherited C-level slot functions.

---

1219-1259: LGTM!

The lookup_slot_in_mro implementation correctly:

1. Checks self's dict first, then iterates the MRO
2. Only extracts slot functions from wrapper_descriptor types
3. Returns None when a Python method is found (allowing wrapper fallback)
4. Returns the extracted function when a compatible wrapper is found

This enables slot function reuse from parent classes while correctly falling back to Python method wrappers when needed.

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

784-913: LGTM!

get_slot_func_with_op correctly handles:

1. Delete operations (lines 791-797): Returns DelAttro/DescrDel variants for TpSetattro/TpDescrSet when op == Some(SlotOp::Delete)
2. Right-hand operations (lines 799-901): Returns NumBinaryRight/NumTernaryRight using right_* slot fields
3. Comparison operations (lines 903-910): Uses op.as_compare_op() to get the correct PyComparisonOp
4. Fallback (line 912): Delegates to get_slot_func for left/other operations

This addresses the issues raised in past reviews about right-hand and delete wrappers.

---

924-1433: LGTM!

The SLOT_DEFS array is comprehensive and well-structured:

• Covers all type slots (__init__, __repr__, __hash__, etc.)
• Correctly duplicates entries for shared slots (__len__ for both SqLength and MpLength)
• Uses appropriate SlotOp for comparison operators and binary left/right variants
• Includes descriptor protocol (__get__, __set__, __delete__)
• Covers sequence and mapping protocols with proper accessor assignments

The documentation strings match CPython's conventions.

---

1435-1490: LGTM!

Tests provide good coverage:

• Validates SlotAccessor enum values match CPython's slot IDs
• Verifies SLOT_DEFS count is within expected bounds
• Tests find_slot_defs_by_name for duplicate entries (__len__, __add__)
• Validates SlotOp comparison and right-hand operations