Actionable comments posted: 0

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

989-994: New __getattr__ and sequence __rmul__ slot defs are consistent with existing patterns

• __getattr__ mapped to TpGetattro mirrors how __getattribute__ is exposed in slots, and combined with the add_operators skip in class.rs keeps default object from gaining an unintended __getattr__.
• Adding a SqRepeat‑backed __rmul__ mirrors the existing dual definitions for __add__ (number + sequence), which should help right‑hand repetition for sequence types behave more like CPython.

Consider adding a small test alongside test_find_by_name that asserts the expected count for "__rmul__" now that it appears both as a number and sequence slot, to guard future refactors of SLOT_DEFS.

Also applies to: 1429-1433

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

15-17: PyCFuncPtr numeric/boolean behavior via AsNumber looks sound

Using AsNumber for CFuncPtr and defining boolean as Ok(zelf.get_func_ptr() != 0) aligns with ctypes’ expectation that a function pointer is falsy only when it’s NULL. The new #[pyclass(... with(Callable, Constructor, AsNumber, Representable, AsBuffer))] block correctly surfaces this behavior at the type level, and the implementation reuses the existing pointer‑extraction logic.

It may be worth adding/adjusting a small test to cover bool(CFuncPtr(0)) vs bool(CFuncPtr(<nonzero addr>)) to lock in this behavior, especially since the truthiness path has moved from an explicit __bool__ method into the numeric protocol.

Also applies to: 1755-1854, 1856-1867

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 37c47fc and 0aa1801.

• crates/vm/src/builtins/object.rs
• crates/vm/src/builtins/range.rs
• crates/vm/src/builtins/singletons.rs
• crates/vm/src/builtins/tuple.rs
• crates/vm/src/builtins/weakproxy.rs
• crates/vm/src/class.rs
• crates/vm/src/stdlib/collections.rs
• crates/vm/src/stdlib/ctypes/function.rs
• crates/vm/src/stdlib/ctypes/pointer.rs
• crates/vm/src/stdlib/ctypes/simple.rs
• crates/vm/src/stdlib/winreg.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/types/slot_defs.rs

• crates/vm/src/stdlib/ctypes/simple.rs
• crates/vm/src/builtins/object.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/class.rs
• crates/vm/src/types/slot_defs.rs
• crates/vm/src/stdlib/winreg.rs
• crates/vm/src/stdlib/ctypes/function.rs
• crates/vm/src/stdlib/ctypes/pointer.rs
• crates/vm/src/builtins/singletons.rs
• crates/vm/src/types/slot.rs
• crates/vm/src/builtins/range.rs
• crates/vm/src/stdlib/collections.rs
• crates/vm/src/builtins/weakproxy.rs
• crates/vm/src/builtins/tuple.rs

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

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

• a (195-196)
• zelf (73-74)
• zelf (168-168)

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

• a (503-504)
• zelf (250-251)

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

• a (1566-1566)

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

• zelf (278-282)

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

• zelf (1720-1720)

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

• zelf (3682-3682)

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

• zelf (212-213)

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

• as_number (422-431)
• number (425-425)
• zelf (278-282)

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

• as_number (192-208)
• as_number (777-786)
• number (780-780)
• zelf (73-74)
• zelf (168-168)

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

• as_number (409-425)
• zelf (136-137)

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

• as_number (604-607)

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

• as_number (176-184)

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

• zelf (305-306)

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

• as_number (68-74)
• as_number (110-119)

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

• as_number (422-431)
• number (425-425)
• zelf (278-282)

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

• as_number (1857-1866)
• number (1860-1860)

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

• as_number (255-288)

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

• as_number (409-425)
• zelf (136-137)

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

• as_number (604-607)

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

• zelf (305-306)

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

• as_number (1893-1893)

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

• as_number (171-180)

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

• as_number (604-607)

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

• as_number (1760-1766)

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

• find_slot_defs_by_name (918-920)

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

• as_number (68-74)
• as_number (110-119)

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

• as_number (422-431)
• number (425-425)
• zelf (278-282)

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

• as_number (171-180)
• number (174-174)

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

• as_number (192-208)
• as_number (777-786)
• number (780-780)
• zelf (73-74)
• zelf (168-168)

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

• as_number (255-288)

crates/vm/src/stdlib/ctypes/simple.rs (3)

• as_number (498-516)
• as_number (1455-1469)
• zelf (250-251)

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

• as_number (677-689)

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

• as_number (426-435)
• number (429-429)

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

• as_number (422-431)
• number (425-425)
• zelf (278-282)

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

• as_number (171-180)
• number (174-174)

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

• as_number (426-435)
• number (429-429)

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

• as_number (422-431)
• number (425-425)
• zelf (278-282)

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

• as_number (426-435)
• number (429-429)

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

• as_number (171-180)
• number (174-174)

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

• as_number (604-607)

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

• as_number (1760-1766)
• zelf (1718-1718)
• zelf (1720-1720)

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

37-42: Skipping __getattr__ wrapper from slots looks correct

The special-casing here avoids giving object (and other types that only define tp_getattro) a synthetic __getattr__, which matches CPython’s behavior that only __getattribute__ is present by default. This aligns with the comment and the new slot def for __getattr__.

If you haven’t already, it’s worth double‑checking that types which explicitly define __getattr__ still get their method via the normal method/slot plumbing and don’t rely on this wrapper path.

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

266-269: Truthiness for PyHkey via AsNumber looks correct

Using !zelf.hkey.load().is_null() matches the expected bool(HKEY) semantics (non‑null handle is truthy, closed/detached is falsy), and the downcast is safe given this is PyHkey’s AsNumber implementation.

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

11-15: PyRange truthiness via AsNumber is correctly wired

Exposing AsNumber on range and implementing boolean as Ok(!zelf.is_empty()) is semantically correct (bool(range) is false only when empty) and consistent with the container pattern used for tuple/deque. The static PyNumberMethods setup also follows the existing idiom in this crate.

Also applies to: 175-185, 425-435

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

15-22: Deque AsNumber implementation aligns with container truthiness

Hooking PyDeque into AsNumber and implementing boolean as Ok(!zelf.borrow_deque().is_empty()) matches Python’s bool(deque) semantics and mirrors the existing pattern for other containers. The use of a read lock keeps it non‑intrusive w.r.t. existing mutation/state tracking.

Also applies to: 63-68, 495-506

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

740-777: TpGetattro getattr detection logic is correct.

The enhanced logic properly handles the case where __getattr__ may be defined anywhere in the MRO:

• Line 745-757: Correctly checks both self's attributes and all MRO classes (skipping index 0 which is self)
• Line 759-761: When __getattr__ is found, forces the wrapper to ensure the fallback behavior works
• Lines 762-776: Standard slot inheritance when no __getattr__ is present

This aligns with CPython's behavior where native tp_getattro implementations don't call __getattr__, requiring the wrapper when __getattr__ is defined.

---

618-643: Lock implementation is safe; no deadlock risk from lock ordering.

The method properly handles weak references and skips subclasses with their own definitions. The hierarchical lock acquisition pattern (parent → child only) is safe. RustPython's single-threaded Cell-based lock implementation detects potential re-entrancy issues at runtime rather than causing deadlocks, making the original concern about cross-thread lock ordering inapplicable. Recursion depth is bounded by class hierarchy depth and poses no practical risk.

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

776-787: AsNumber implementation for PyCPointer is correct.

The boolean semantics (non-NULL pointer = true, NULL = false) correctly matches Python's ctypes behavior where pointer types are truthy when non-NULL. The implementation pattern is consistent with other types in the PR (e.g., PyCFuncPtr at crates/vm/src/stdlib/ctypes/function.rs lines 1856-1865).

---

262-265: Pyclass declaration correctly includes AsNumber.

The addition of AsNumber to the with(...) clause properly wires the AsNumber implementation into the type's slots. The ordering (Constructor, Initializer, AsNumber, AsBuffer) follows the existing pattern in the codebase.

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

67-75: AsNumber for PyNone correctly implements falsy semantics.

None is always falsy in Python, and returning Ok(false) is the correct implementation. The pattern matches other singleton types in the codebase.

---

52-53: Empty impl block after bool removal is fine.

The empty impl PyNone {} block is acceptable - it's required for the #[pyclass(with(...))] attribute to attach to. Rust allows empty impl blocks and this pattern is common when methods are provided through traits rather than direct implementations.

---

109-120: AsNumber for PyNotImplemented with deprecation TODO.

The implementation correctly returns true for current Python semantics. The TODO comment accurately documents the breaking change from bpo-35712:

• Python 3.9: DeprecationWarning when using NotImplemented in boolean context
• Python 3.14: TypeError

Consider tracking this with an issue to implement the deprecation warning before Python 3.14 compatibility is targeted.

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

421-432: AsNumber implementation for PyTuple is correct and consistent.

The boolean semantics (!zelf.elements.is_empty()) correctly implements Python's tuple truthiness where empty tuples are falsy.

The use of static AS_NUMBER: PyNumberMethods (without LazyLock) is appropriate here since no runtime initialization is needed - this is a const-initializable struct. This matches the pattern in PyRange (lines 425-434 in range.rs).

---

260-264: Pyclass declaration correctly adds AsNumber to protocol list.

The addition maintains alphabetical ordering of the traits (AsMapping, AsNumber, AsSequence, ...) which aids readability and consistency with other type declarations.

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

170-181: AsNumber for PyWeakProxy correctly delegates to referent's truthiness.

The implementation appropriately:

1. Uses LazyLock since the closure captures runtime behavior (unlike static const patterns used for simpler types)
2. Calls try_upgrade(vm)? which will raise ReferenceError if the referent is dead
3. Delegates to is_true(vm) on the referent, correctly proxying the truthiness

This maintains the proxy semantics where the weak proxy behaves like the underlying object for boolean contexts.

---

66-76: Pyclass with clause properly includes AsNumber.

The ordering of traits in the with(...) clause is sensible, grouping related functionality together. AsNumber is placed after Comparable following an approximate pattern of core attributes → protocols.