Resolve number slots via MRO in PyNumber and operator, ensure inherit… by YashSuthar983 · Pull Request #6222 · RustPython/RustPython

coderabbitai

Caution

Review failed

The pull request is closed.

Walkthrough

Replaced direct class slot access with MRO-aware lookups for number protocol methods and VM operation slots, so slot retrieval now traverses the method resolution order (using mro_find_map) before invoking or falling back to previous behavior.

Changes

Cohort / File(s)	Summary
Number Protocol MRO Lookups `vm/src/protocol/number.rs`	Replaced direct per-class slot reads with `mro_find_map`-based lookups in `check`, `is_index`, `int`, `index`, and `float`. Invocation path and result handling preserved; only slot retrieval now searches the MRO.
VM Operations MRO Lookups `vm/src/vm/vm_ops.rs`	Replaced direct operation-slot lookups with `mro_find_map` in binary/ternary op resolution (`binary_op1`, `binary_iop1`, `ternary_op`, `ternary_iop`). Added comments clarifying MRO-based resolution; fallbacks unchanged.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Protocol as Number/VM Ops
    participant MRO as mro_find_map
    participant ClassSlot as Class Slots

    rect rgb(245,245,255)
    note over Caller,ClassSlot: Old: direct class slot access
    Caller->>Protocol: request slot
    Protocol->>ClassSlot: read obj.class().slots[slot]
    ClassSlot-->>Protocol: slot or None
    Protocol-->>Caller: call slot or fallback
    end

    rect rgb(245,255,245)
    note over Caller,MRO: New: MRO-aware lookup
    Caller->>Protocol: request slot
    Protocol->>MRO: mro_find_map(class, find_slot)
    MRO->>ClassSlot: iterate classes -> check slots
    alt found
        ClassSlot-->>MRO: slot
        MRO-->>Protocol: slot
        Protocol-->>Caller: call slot
    else not found
        MRO-->>Protocol: None
        Protocol-->>Caller: fallback
    end
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Review focus:
- Consistency of error and deprecation messages in int, index, float
- Correctness of mro_find_map usage and closure/map key for each slot kind
- Binary/ternary op resolution paths and fallback ordering in vm_ops.rs

Possibly related PRs

Resolve number slots via MRO in PyNumber and operator, ensure inherit… #6222 — Applies the same pattern: replacing direct class slot access with mro_find_map for number protocol and operator slots.

Poem

🐰
I hop through classes, nose to the sky,
Now slots I find by walking MRO high.
No sudden leaps — I follow the chain,
Parents guide methods, nothing in vain.
A tiny hop, a system made spry.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "Resolve number slots via MRO in PyNumber and operator, ensure inherit..." directly and accurately reflects the main changes in the changeset. The modifications replace direct slot access with MRO-aware lookups in both `vm/src/protocol/number.rs` and `vm/src/vm/vm_ops.rs`, which enables inherited and dynamically added number methods to be properly discovered during numeric operations. The title is clear, specific, and sufficiently concise to convey the primary purpose without vague or generic terminology.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 21c7e15 and 914bd74.

⛔ Files ignored due to path filters (1)

Lib/ctypes/__init__.py is excluded by !Lib/**

📒 Files selected for processing (1)

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

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

❤️ Share

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