From 131e5aa495051d6b534553e3264b601aad271295 Mon Sep 17 00:00:00 2001 From: rlaisqls Date: Fri, 13 Mar 2026 11:25:35 +0900 Subject: [PATCH 1/4] Fix subclass right-op dispatch for Python classes --- Lib/test/test_descr.py | 1 - crates/vm/src/protocol/number.rs | 51 ++++++++++++++++++++++++++++++++ crates/vm/src/vm/vm_ops.rs | 45 +++++++++++++++++++++++++--- 3 files changed, 92 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index 1f7c5452c4d..3a66cdcdead 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -4318,7 +4318,6 @@ class C: C.__name__ = Nasty("abc") C.__name__ = "normal" - @unittest.expectedFailure # TODO: RUSTPYTHON def test_subclass_right_op(self): # Testing correct dispatch of subclass overloading __r__... diff --git a/crates/vm/src/protocol/number.rs b/crates/vm/src/protocol/number.rs index 36dbd5b8843..77e57293f15 100644 --- a/crates/vm/src/protocol/number.rs +++ b/crates/vm/src/protocol/number.rs @@ -229,12 +229,63 @@ pub enum PyNumberBinaryOp { InplaceMatrixMultiply, } +impl PyNumberBinaryOp { + /// Returns `None` for in-place ops which don't have right-side variants. + pub fn right_method_name( + self, + vm: &VirtualMachine, + ) -> Option<&'static crate::builtins::PyStrInterned> { + use PyNumberBinaryOp::*; + Some(match self { + Add => identifier!(vm, __radd__), + Subtract => identifier!(vm, __rsub__), + Multiply => identifier!(vm, __rmul__), + Remainder => identifier!(vm, __rmod__), + Divmod => identifier!(vm, __rdivmod__), + Lshift => identifier!(vm, __rlshift__), + Rshift => identifier!(vm, __rrshift__), + And => identifier!(vm, __rand__), + Xor => identifier!(vm, __rxor__), + Or => identifier!(vm, __ror__), + FloorDivide => identifier!(vm, __rfloordiv__), + TrueDivide => identifier!(vm, __rtruediv__), + MatrixMultiply => identifier!(vm, __rmatmul__), + // In-place ops don't have right-side variants + InplaceAdd + | InplaceSubtract + | InplaceMultiply + | InplaceRemainder + | InplaceLshift + | InplaceRshift + | InplaceAnd + | InplaceXor + | InplaceOr + | InplaceFloorDivide + | InplaceTrueDivide + | InplaceMatrixMultiply => return None, + }) + } +} + #[derive(Copy, Clone)] pub enum PyNumberTernaryOp { Power, InplacePower, } +impl PyNumberTernaryOp { + /// Returns `None` for in-place ops which don't have right-side variants. + pub fn right_method_name( + self, + vm: &VirtualMachine, + ) -> Option<&'static crate::builtins::PyStrInterned> { + Some(match self { + PyNumberTernaryOp::Power => identifier!(vm, __rpow__), + PyNumberTernaryOp::InplacePower => return None, + }) + } +} + #[derive(Default)] pub struct PyNumberSlots { pub add: AtomicCell>, diff --git a/crates/vm/src/vm/vm_ops.rs b/crates/vm/src/vm/vm_ops.rs index d5c70f87386..5a4744fc9f1 100644 --- a/crates/vm/src/vm/vm_ops.rs +++ b/crates/vm/src/vm/vm_ops.rs @@ -1,14 +1,29 @@ use super::VirtualMachine; use crate::stdlib::_warnings; use crate::{ - PyRef, - builtins::{PyInt, PyStr, PyStrRef, PyUtf8Str}, + Py, PyRef, + builtins::{PyInt, PyStr, PyStrInterned, PyStrRef, PyType, PyUtf8Str}, object::{AsObject, PyObject, PyObjectRef, PyResult}, protocol::{PyNumberBinaryOp, PyNumberTernaryOp}, types::PyComparisonOp, }; use num_traits::ToPrimitive; +/// Similar to `method_is_overloaded` in CPython typeobject.c +fn method_is_overloaded( + class_a: &Py, + class_b: &Py, + rop_name: &'static PyStrInterned, + vm: &VirtualMachine, +) -> bool { + let Some(method_b) = class_b.get_attr(rop_name) else { + return false; + }; + class_a + .get_attr(rop_name) + .is_none_or(|method_a| !vm.identical_or_equal(&method_a, &method_b).unwrap_or(false)) +} + macro_rules! binary_func { ($fn:ident, $op_slot:ident, $op:expr) => { pub fn $fn(&self, a: &PyObject, b: &PyObject) -> PyResult { @@ -166,7 +181,18 @@ impl VirtualMachine { if !class_a.is(class_b) { let slot_bb = class_b.slots.as_number.right_binary_op(op_slot); - if slot_bb.map(|x| x as usize) != slot_a.map(|x| x as usize) { + if slot_a.map(|x| x as usize) + != class_b + .slots + .as_number + .left_binary_op(op_slot) + .map(|x| x as usize) + { + slot_b = slot_bb; + } else if class_b.fast_issubclass(class_a) + && let Some(rop_name) = op_slot.right_method_name(self) + && method_is_overloaded(class_a, class_b, rop_name, self) + { slot_b = slot_bb; } } @@ -273,7 +299,18 @@ impl VirtualMachine { if !class_a.is(class_b) { let slot_bb = class_b.slots.as_number.right_ternary_op(op_slot); - if slot_bb.map(|x| x as usize) != slot_a.map(|x| x as usize) { + if slot_a.map(|x| x as usize) + != class_b + .slots + .as_number + .left_ternary_op(op_slot) + .map(|x| x as usize) + { + slot_b = slot_bb; + } else if class_b.fast_issubclass(class_a) + && let Some(rop_name) = op_slot.right_method_name(self) + && method_is_overloaded(class_a, class_b, rop_name, self) + { slot_b = slot_bb; } } From b24d50e127bdf54b6dc50c38b2eee82eed2f38d7 Mon Sep 17 00:00:00 2001 From: rlaisqls Date: Wed, 18 Mar 2026 22:21:55 +0900 Subject: [PATCH 2/4] Separate fallback queueing from subclass priority in op dispatch --- crates/vm/src/vm/vm_ops.rs | 73 ++++++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 30 deletions(-) diff --git a/crates/vm/src/vm/vm_ops.rs b/crates/vm/src/vm/vm_ops.rs index 5a4744fc9f1..ecd6068b0a1 100644 --- a/crates/vm/src/vm/vm_ops.rs +++ b/crates/vm/src/vm/vm_ops.rs @@ -13,15 +13,18 @@ use num_traits::ToPrimitive; fn method_is_overloaded( class_a: &Py, class_b: &Py, - rop_name: &'static PyStrInterned, + rop_name: Option<&'static PyStrInterned>, vm: &VirtualMachine, -) -> bool { +) -> PyResult { + let Some(rop_name) = rop_name else { + return Ok(false); + }; let Some(method_b) = class_b.get_attr(rop_name) else { - return false; + return Ok(false); }; - class_a - .get_attr(rop_name) - .is_none_or(|method_a| !vm.identical_or_equal(&method_a, &method_b).unwrap_or(false)) + class_a.get_attr(rop_name).map_or(Ok(true), |method_a| { + vm.identical_or_equal(&method_a, &method_b).map(|eq| !eq) + }) } macro_rules! binary_func { @@ -177,29 +180,34 @@ impl VirtualMachine { // Number slots are inherited, direct access is O(1) let slot_a = class_a.slots.as_number.left_binary_op(op_slot); + let slot_a_addr = slot_a.map(|x| x as usize); let mut slot_b = None; + let left_b_addr; if !class_a.is(class_b) { let slot_bb = class_b.slots.as_number.right_binary_op(op_slot); - if slot_a.map(|x| x as usize) - != class_b - .slots - .as_number - .left_binary_op(op_slot) - .map(|x| x as usize) - { - slot_b = slot_bb; - } else if class_b.fast_issubclass(class_a) - && let Some(rop_name) = op_slot.right_method_name(self) - && method_is_overloaded(class_a, class_b, rop_name, self) - { + if slot_bb.map(|x| x as usize) != slot_a_addr { slot_b = slot_bb; } + left_b_addr = class_b + .slots + .as_number + .left_binary_op(op_slot) + .map(|x| x as usize); + } else { + left_b_addr = slot_a_addr; } if let Some(slot_a) = slot_a { if let Some(slot_bb) = slot_b && class_b.fast_issubclass(class_a) + && (slot_a_addr != left_b_addr + || method_is_overloaded( + class_a, + class_b, + op_slot.right_method_name(self), + self, + )?) { let ret = slot_bb(a, b, self)?; if !ret.is(&self.ctx.not_implemented) { @@ -295,29 +303,34 @@ impl VirtualMachine { // Number slots are inherited, direct access is O(1) let slot_a = class_a.slots.as_number.left_ternary_op(op_slot); + let slot_a_addr = slot_a.map(|x| x as usize); let mut slot_b = None; + let left_b_addr; if !class_a.is(class_b) { let slot_bb = class_b.slots.as_number.right_ternary_op(op_slot); - if slot_a.map(|x| x as usize) - != class_b - .slots - .as_number - .left_ternary_op(op_slot) - .map(|x| x as usize) - { - slot_b = slot_bb; - } else if class_b.fast_issubclass(class_a) - && let Some(rop_name) = op_slot.right_method_name(self) - && method_is_overloaded(class_a, class_b, rop_name, self) - { + if slot_bb.map(|x| x as usize) != slot_a_addr { slot_b = slot_bb; } + left_b_addr = class_b + .slots + .as_number + .left_ternary_op(op_slot) + .map(|x| x as usize); + } else { + left_b_addr = slot_a_addr; } if let Some(slot_a) = slot_a { if let Some(slot_bb) = slot_b && class_b.fast_issubclass(class_a) + && (slot_a_addr != left_b_addr + || method_is_overloaded( + class_a, + class_b, + op_slot.right_method_name(self), + self, + )?) { let ret = slot_bb(a, b, c, self)?; if !ret.is(&self.ctx.not_implemented) { From b26a464ebab4634bdef2caa9bc426dc55be943d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=9D=80=EB=B9=88?= Date: Wed, 18 Mar 2026 22:27:34 +0900 Subject: [PATCH 3/4] Add CPython source link to `method_is_overloaded` doc comment Co-authored-by: Shahar Naveh <50263213+ShaharNaveh@users.noreply.github.com> --- crates/vm/src/vm/vm_ops.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vm/src/vm/vm_ops.rs b/crates/vm/src/vm/vm_ops.rs index ecd6068b0a1..abce0ea678b 100644 --- a/crates/vm/src/vm/vm_ops.rs +++ b/crates/vm/src/vm/vm_ops.rs @@ -9,7 +9,7 @@ use crate::{ }; use num_traits::ToPrimitive; -/// Similar to `method_is_overloaded` in CPython typeobject.c +/// [CPython `method_is_overloaded`](https://github.com/python/cpython/blob/v3.14.3/Objects/typeobject.c#L9849-L9879) fn method_is_overloaded( class_a: &Py, class_b: &Py, From eeb8d144f3af6ca6d82dbfb791cd56e8cd405fa6 Mon Sep 17 00:00:00 2001 From: rlaisqls Date: Wed, 18 Mar 2026 22:40:29 +0900 Subject: [PATCH 4/4] Remove expectedFailure from test_union_operators as it now passes --- Lib/test/test_collections.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_collections.py b/Lib/test/test_collections.py index 26d0dcb654d..7a57ba722e8 100644 --- a/Lib/test/test_collections.py +++ b/Lib/test/test_collections.py @@ -262,7 +262,6 @@ def __contains__(self, key): d = c.new_child(b=20, c=30) self.assertEqual(d.maps, [{'b': 20, 'c': 30}, {'a': 1, 'b': 2}]) - @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: .Subclass'> is not def test_union_operators(self): cm1 = ChainMap(dict(a=1, b=2), dict(c=3, d=4)) cm2 = ChainMap(dict(a=10, e=5), dict(b=20, d=4))