◐ Shell
clean mode source ↗

More slot impl by youknowone · Pull Request #6618 · RustPython/RustPython

youknowone

Warning

Rate limit exceeded

@youknowone has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 8 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between fbf02df and bb18d0c.

📒 Files selected for processing (11)
  • crates/vm/src/builtins/bool.rs
  • crates/vm/src/builtins/bytearray.rs
  • crates/vm/src/builtins/bytes.rs
  • crates/vm/src/builtins/dict.rs
  • crates/vm/src/builtins/list.rs
  • crates/vm/src/builtins/mappingproxy.rs
  • crates/vm/src/builtins/memory.rs
  • crates/vm/src/builtins/set.rs
  • crates/vm/src/builtins/tuple.rs
  • crates/vm/src/stdlib/collections.rs
  • crates/vm/src/stdlib/ctypes/array.rs
📝 Walkthrough

Walkthrough

This pull request removes Python method exposure for __len__ across multiple builtin classes via #[pymethod] attribute removal, refactors boolean truthiness evaluation to use protocol-based slot lookups instead of method fallbacks, updates binary operations for mixed set/frozenset types with type checking, and improves slot machinery for method resolution based on whether a type defines its own methods.

Changes

Cohort / File(s) Summary
Boolean Truthiness Refactor
crates/vm/src/builtins/bool.rs		 Replaced multi-path truthiness evaluation with simplified protocol-based approach: attempt nb_bool slot first, then mp_length, then sq_length, defaulting to truthy. Removed legacy __bool__ and __len__ fallback method lookups. Removed unused malachite_bigint::Sign import.
len Method Exposure Removal
crates/vm/src/builtins/{bytearray,bytes,dict,list,tuple}.rs, crates/vm/src/builtins/memory.rs, crates/vm/src/stdlib/ctypes/array.rs		 Removed #[pymethod] attribute from __len__ methods across eight builtin types, eliminating direct Python method exposure while preserving internal Rust implementations.
Mapping Proxy Protocol Wiring
crates/vm/src/builtins/mappingproxy.rs		 Removed #[pymethod] from __len__ and wired length exposure through sequence protocol via PySequenceMethods.length using sequence_downcast, maintaining external accessibility through protocol machinery instead of direct Python binding.
Set/Frozenset Binary Operations
crates/vm/src/builtins/set.rs		 Reworked binary operators (__sub__/__rsub__, __and__/__rand__, __xor__/__rxor__, __or__/__ror__) to validate operand types using new AnySet::check() helper, return NotImplemented for invalid type combinations, and wrap results to appropriate public types. Introduced unified __contains__(&self, needle: &PyObject, vm) signature for both PySet and PyFrozenSet; updated sequence containment to route through new __contains__ implementation.
Collections Deque API Changes
crates/vm/src/stdlib/collections.rs		 Removed #[pymethod] exposure for __len__ and __add__ methods from PyDeque, eliminating Python-facing concatenation and length methods while retaining internal concat and __iadd__ functionality.
Windows Registry Type Conversion
crates/vm/src/stdlib/winreg.rs		 Removed #[pymethod] attribute from PyHkey::__int__ method, eliminating Python-level integer conversion exposure while preserving internal Rust method for internal protocol hooks.
Number Protocol Slot Copying
crates/vm/src/protocol/number.rs		 Extended PyNumberSlots::copy_from to mirror left-hand operator slots to right-hand counterparts for binary operators (Add, Subtract, Multiply, Remainder, Divmod, Power, Lshift, Rshift, And, Xor, Or, FloorDivide, TrueDivide, MatrixMultiply).
Slot Update Machinery
crates/vm/src/types/slot.rs		 Refactored slot update logic to collect slot definitions into temporary Vec before iteration, added has_own detection to determine if a type defines its own method for a slot, and implemented conditional wrapper/fallback selection based on ownership (ass_item/ass_subscript slots consider paired __setitem__/__delitem__ methods).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 Protocol paths now guide our way, no more __len__ for the fray,
Sets and slots align just right, truthiness checked in protocol's light,
Simpler flows, fewer lookups dance, RustPython takes a cleaner stance! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 48.84% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'More slot impl' is vague and does not clearly convey the main changes, which involve refactoring truthiness evaluation, removing len method exposures, and restructuring set operations. Consider a more descriptive title that captures the primary change, such as 'Refactor protocol-driven truthiness and remove len Python method exposures' or 'Implement slot-based protocols for truthiness and method access'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.