From d9b1ae296b8952fdee9f163ff0ef69dd4e8bdeb7 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 20 Sep 2020 10:43:31 +0900 Subject: [PATCH 1/4] tp_del --- vm/src/macros.rs | 7 +++++-- vm/src/obj/objtype.rs | 7 +++++++ vm/src/pyobjectrc.rs | 5 +++-- vm/src/slots.rs | 21 +++++++++++++++++++++ vm/src/stdlib/io.rs | 9 +++++++-- 5 files changed, 43 insertions(+), 6 deletions(-) diff --git a/vm/src/macros.rs b/vm/src/macros.rs index 292dc674c..515ce12ad 100644 --- a/vm/src/macros.rs +++ b/vm/src/macros.rs @@ -47,11 +47,14 @@ macro_rules! py_class { py_class } }; - (@extract_slots($ctx:expr, $slots:expr, (slot $slot_name:ident), $value:expr)) => { - $slots.$slot_name = Some( + (@extract_slots($ctx:expr, $slots:expr, (slot new), $value:expr)) => { + $slots.new = Some( $crate::function::IntoPyNativeFunc::into_func($value) ); }; + (@extract_slots($ctx:expr, $slots:expr, (slot $slot_name:ident), $value:expr)) => { + $slots.$slot_name.store(Some($value)); + }; (@extract_slots($ctx:expr, $class:expr, $name:expr, $value:expr)) => {}; (@extract_attrs($ctx:expr, $slots:expr, (slot $slot_name:ident), $value:expr)) => {}; (@extract_attrs($ctx:expr, $class:expr, $name:expr, $value:expr)) => { diff --git a/vm/src/obj/objtype.rs b/vm/src/obj/objtype.rs index 0bd01e89b..5607bfcd9 100644 --- a/vm/src/obj/objtype.rs +++ b/vm/src/obj/objtype.rs @@ -150,6 +150,13 @@ impl PyClassRef { } as _; self.slots.hash.store(Some(func)); } + "__del__" => { + let func: slots::DelFunc = |zelf, vm| { + let magic = get_class_magic(&zelf, "__del__"); + vm.invoke(&magic, vec![zelf.clone()]).map(|_| ()) + } as _; + self.slots.del.store(Some(func)); + } _ => (), } } diff --git a/vm/src/pyobjectrc.rs b/vm/src/pyobjectrc.rs index 0e58b50cf..432dd90b0 100644 --- a/vm/src/pyobjectrc.rs +++ b/vm/src/pyobjectrc.rs @@ -146,11 +146,12 @@ where // CPython-compatible drop implementation let zelf = Self::into_ref(self.clone()); - if let Some(del_method) = zelf.inner.get_class_attr("__del__") { + if let Some(del_slot) = zelf.class().first_in_mro(|cls| cls.slots.del.load()) { crate::vm::thread::with_vm(&zelf, |vm| { - if let Err(e) = vm.invoke(&del_method, vec![zelf.clone()]) { + if let Err(e) = del_slot(&zelf, vm) { // exception in del will be ignored but printed print!("Exception ignored in: ",); + let del_method = zelf.get_class_attr("__del__").unwrap(); let repr = vm.to_repr(&del_method); match repr { Ok(v) => println!("{}", v.to_string()), diff --git a/vm/src/slots.rs b/vm/src/slots.rs index 68eb46829..10ec75137 100644 --- a/vm/src/slots.rs +++ b/vm/src/slots.rs @@ -41,6 +41,7 @@ impl Default for PyTpFlags { } pub(crate) type GenericMethod = fn(PyObjectRef, PyFuncArgs, &VirtualMachine) -> PyResult; +pub(crate) type DelFunc = fn(&PyObjectRef, &VirtualMachine) -> PyResult<()>; pub(crate) type DescrGetFunc = fn(PyObjectRef, Option, Option, &VirtualMachine) -> PyResult; pub(crate) type HashFunc = fn(&PyObjectRef, &VirtualMachine) -> PyResult; @@ -50,6 +51,7 @@ pub struct PyClassSlots { pub flags: PyTpFlags, pub name: PyRwLock>, // tp_name, not class name pub new: Option, + pub del: AtomicCell>, pub call: AtomicCell>, pub descr_get: AtomicCell>, pub hash: AtomicCell>, @@ -82,6 +84,25 @@ impl std::fmt::Debug for PyClassSlots { } } +#[pyimpl] +pub trait SlotDesctuctor: PyValue { + #[pyslot] + fn tp_del(zelf: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + if let Some(zelf) = zelf.downcast_ref() { + Self::del(zelf, vm) + } else { + Err(vm.new_type_error("unexpected payload for __del__".to_owned())) + } + } + + #[pymethod(magic)] + fn __del__(zelf: PyRef, vm: &VirtualMachine) -> PyResult<()> { + Self::del(&zelf, vm) + } + + fn del(zelf: &PyRef, vm: &VirtualMachine) -> PyResult<()>; +} + #[pyimpl] pub trait SlotCall: PyValue { #[pymethod(magic)] diff --git a/vm/src/stdlib/io.rs b/vm/src/stdlib/io.rs index 170f1bb35..121d6cefe 100644 --- a/vm/src/stdlib/io.rs +++ b/vm/src/stdlib/io.rs @@ -416,11 +416,15 @@ fn io_base_cm_enter(instance: PyObjectRef) -> PyObjectRef { instance } -fn io_base_cm_del(instance: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - vm.call_method(&instance, "close", vec![])?; +fn io_base_cm_del_slot(instance: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + vm.call_method(instance, "close", vec![])?; Ok(()) } +fn io_base_cm_del(instance: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + io_base_cm_del_slot(&instance, vm) +} + fn io_base_cm_exit(instance: PyObjectRef, _args: PyFuncArgs, vm: &VirtualMachine) -> PyResult<()> { vm.call_method(&instance, "close", vec![])?; Ok(()) @@ -1271,6 +1275,7 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { let io_base = py_class!(ctx, "_IOBase", &ctx.types.object_type, { "__enter__" => ctx.new_method(io_base_cm_enter), "__del__" => ctx.new_method(io_base_cm_del), + (slot del) => io_base_cm_del_slot, "__exit__" => ctx.new_method(io_base_cm_exit), "seekable" => ctx.new_method(io_base_seekable), "readable" => ctx.new_method(io_base_readable), From 991b968294821e8545e80b147a01a53b49050f67 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 24 Sep 2020 06:09:21 +0900 Subject: [PATCH 2/4] tp_call prevent clone --- derive/src/pyclass.rs | 15 +-------------- vm/src/obj/objbuiltinfunc.rs | 14 +++++++------- vm/src/obj/objfunction.rs | 14 +++++++------- vm/src/obj/objtype.rs | 8 ++++---- vm/src/obj/objweakref.rs | 8 ++++---- vm/src/slots.rs | 18 ++++++++++++++---- vm/src/stdlib/json.rs | 10 +++++----- vm/src/vm.rs | 2 +- 8 files changed, 43 insertions(+), 46 deletions(-) diff --git a/derive/src/pyclass.rs b/derive/src/pyclass.rs index 4c4356d10..ae3528937 100644 --- a/derive/src/pyclass.rs +++ b/derive/src/pyclass.rs @@ -349,21 +349,8 @@ where slots.#slot_ident = Some(#into_func); } } else { - let into_func = if slot_name == "call" { - quote_spanned! {ident.span() => - |zelf: ::rustpython_vm::pyobject::PyObjectRef, args: ::rustpython_vm::function::PyFuncArgs, vm: &::rustpython_vm::VirtualMachine| -> ::rustpython_vm::pyobject::PyResult { - use ::rustpython_vm::pyobject::TryFromObject; - let zelf = PyRef::::try_from_object(vm, zelf)?; - Self::#ident(zelf, args, vm) - } - } - } else { - quote_spanned! {ident.span() => - Self::#ident - } - }; quote! { - slots.#slot_ident.store(Some(#into_func as _)) + slots.#slot_ident.store(Some(Self::#ident as _)) } } }; diff --git a/vm/src/obj/objbuiltinfunc.rs b/vm/src/obj/objbuiltinfunc.rs index acc46aab9..4c9b6ee95 100644 --- a/vm/src/obj/objbuiltinfunc.rs +++ b/vm/src/obj/objbuiltinfunc.rs @@ -7,7 +7,7 @@ use crate::obj::objtype::PyClassRef; use crate::pyobject::{ PyClassImpl, PyContext, PyObject, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, }; -use crate::slots::{SlotCall, SlotDescriptor}; +use crate::slots::{Callable, SlotDescriptor}; use crate::vm::VirtualMachine; pub struct PyFuncDef { @@ -106,13 +106,13 @@ impl PyBuiltinFunction { } } -impl SlotCall for PyBuiltinFunction { - fn call(zelf: PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { +impl Callable for PyBuiltinFunction { + fn call(zelf: &PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { (zelf.value.func)(vm, args) } } -#[pyimpl(with(SlotCall), flags(HAS_DICT))] +#[pyimpl(with(Callable), flags(HAS_DICT))] impl PyBuiltinFunction { #[pyproperty(magic)] fn module(&self, vm: &VirtualMachine) -> PyObjectRef { @@ -186,13 +186,13 @@ impl SlotDescriptor for PyBuiltinMethod { } } -impl SlotCall for PyBuiltinMethod { - fn call(zelf: PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { +impl Callable for PyBuiltinMethod { + fn call(zelf: &PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { (zelf.value.func)(vm, args) } } -#[pyimpl(with(SlotDescriptor, SlotCall))] +#[pyimpl(with(SlotDescriptor, Callable))] impl PyBuiltinMethod { #[pyproperty(magic)] fn name(&self) -> Option { diff --git a/vm/src/obj/objfunction.rs b/vm/src/obj/objfunction.rs index 646f2d411..1f5e1765b 100644 --- a/vm/src/obj/objfunction.rs +++ b/vm/src/obj/objfunction.rs @@ -19,7 +19,7 @@ use crate::pyobject::{ PyRef, PyResult, PyValue, TypeProtocol, }; use crate::scope::Scope; -use crate::slots::{SlotCall, SlotDescriptor}; +use crate::slots::{Callable, SlotDescriptor}; use crate::VirtualMachine; use itertools::Itertools; #[cfg(feature = "jit")] @@ -269,7 +269,7 @@ impl PyValue for PyFunction { } } -#[pyimpl(with(SlotDescriptor, SlotCall), flags(HAS_DICT))] +#[pyimpl(with(SlotDescriptor, Callable), flags(HAS_DICT))] impl PyFunction { #[pyproperty(magic)] fn code(&self) -> PyCodeRef { @@ -320,8 +320,8 @@ impl SlotDescriptor for PyFunction { } } -impl SlotCall for PyFunction { - fn call(zelf: PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { +impl Callable for PyFunction { + fn call(zelf: &PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { zelf.invoke(args, vm) } } @@ -334,14 +334,14 @@ pub struct PyBoundMethod { pub function: PyObjectRef, } -impl SlotCall for PyBoundMethod { - fn call(zelf: PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { +impl Callable for PyBoundMethod { + fn call(zelf: &PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { let args = args.insert(zelf.object.clone()); vm.invoke(&zelf.function, args) } } -#[pyimpl(with(SlotCall), flags(HAS_DICT))] +#[pyimpl(with(Callable), flags(HAS_DICT))] impl PyBoundMethod { pub fn new(object: PyObjectRef, function: PyObjectRef) -> Self { PyBoundMethod { object, function } diff --git a/vm/src/obj/objtype.rs b/vm/src/obj/objtype.rs index 5607bfcd9..3361b2f29 100644 --- a/vm/src/obj/objtype.rs +++ b/vm/src/obj/objtype.rs @@ -17,7 +17,7 @@ use crate::pyobject::{ BorrowValue, IdProtocol, PyAttributes, PyClassImpl, PyContext, PyIterable, PyLease, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; -use crate::slots::{self, PyClassSlots, PyTpFlags, SlotCall}; +use crate::slots::{self, Callable, PyClassSlots, PyTpFlags}; use crate::vm::VirtualMachine; use itertools::Itertools; use std::ops::Deref; @@ -172,7 +172,7 @@ fn get_class_magic(zelf: &PyObjectRef, name: &str) -> PyObjectRef { // attrs.get(name).unwrap().clone() } -#[pyimpl(with(SlotCall), flags(BASETYPE))] +#[pyimpl(with(Callable), flags(BASETYPE))] impl PyClass { #[pyproperty(name = "__mro__")] fn get_mro(zelf: PyRef) -> PyTuple { @@ -502,8 +502,8 @@ impl PyClass { } } -impl SlotCall for PyClass { - fn call(zelf: PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { +impl Callable for PyClass { + fn call(zelf: &PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { vm_trace!("type_call: {:?}", zelf); let obj = call_tp_new(zelf.clone(), zelf.clone(), args.clone(), vm)?; diff --git a/vm/src/obj/objweakref.rs b/vm/src/obj/objweakref.rs index 3da31b05c..e5a262d67 100644 --- a/vm/src/obj/objweakref.rs +++ b/vm/src/obj/objweakref.rs @@ -5,7 +5,7 @@ use crate::pyobject::{ IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, }; use crate::pyobjectrc::{PyObjectRc, PyObjectWeak}; -use crate::slots::{Comparable, Hashable, PyComparisonOp, SlotCall}; +use crate::slots::{Callable, Comparable, Hashable, PyComparisonOp}; use crate::vm::VirtualMachine; use crossbeam_utils::atomic::AtomicCell; @@ -38,14 +38,14 @@ impl PyValue for PyWeak { pub type PyWeakRef = PyRef; -impl SlotCall for PyWeak { - fn call(zelf: PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { +impl Callable for PyWeak { + fn call(zelf: &PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { args.bind::<()>(vm)?; Ok(vm.unwrap_or_none(zelf.upgrade())) } } -#[pyimpl(with(SlotCall, Hashable, Comparable), flags(BASETYPE))] +#[pyimpl(with(Callable, Hashable, Comparable), flags(BASETYPE))] impl PyWeak { // TODO callbacks #[pyslot] diff --git a/vm/src/slots.rs b/vm/src/slots.rs index 10ec75137..2fb21ffa4 100644 --- a/vm/src/slots.rs +++ b/vm/src/slots.rs @@ -40,7 +40,7 @@ impl Default for PyTpFlags { } } -pub(crate) type GenericMethod = fn(PyObjectRef, PyFuncArgs, &VirtualMachine) -> PyResult; +pub(crate) type GenericMethod = fn(&PyObjectRef, PyFuncArgs, &VirtualMachine) -> PyResult; pub(crate) type DelFunc = fn(&PyObjectRef, &VirtualMachine) -> PyResult<()>; pub(crate) type DescrGetFunc = fn(PyObjectRef, Option, Option, &VirtualMachine) -> PyResult; @@ -104,10 +104,20 @@ pub trait SlotDesctuctor: PyValue { } #[pyimpl] -pub trait SlotCall: PyValue { - #[pymethod(magic)] +pub trait Callable: PyValue { #[pyslot] - fn call(zelf: PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult; + fn tp_call(zelf: &PyObjectRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { + if let Some(zelf) = zelf.downcast_ref() { + Self::call(zelf, args, vm) + } else { + Err(vm.new_type_error("unexpected payload for __call__".to_owned())) + } + } + #[pymethod] + fn __call__(zelf: PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { + Self::call(&zelf, args, vm) + } + fn call(zelf: &PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult; } #[pyimpl] diff --git a/vm/src/stdlib/json.rs b/vm/src/stdlib/json.rs index 7018fec74..f7a67b05c 100644 --- a/vm/src/stdlib/json.rs +++ b/vm/src/stdlib/json.rs @@ -10,7 +10,7 @@ mod _json { use crate::pyobject::{ BorrowValue, IdProtocol, PyClassImpl, PyObjectRef, PyRef, PyResult, PyValue, }; - use crate::slots::SlotCall; + use crate::slots::Callable; use crate::VirtualMachine; use num_bigint::BigInt; @@ -35,7 +35,7 @@ mod _json { } } - #[pyimpl(with(SlotCall))] + #[pyimpl(with(Callable))] impl JsonScanner { #[pyslot] fn tp_new(cls: PyClassRef, ctx: PyObjectRef, vm: &VirtualMachine) -> PyResult> { @@ -197,7 +197,7 @@ mod _json { } fn call( - zelf: PyRef, + zelf: &PyRef, pystr: PyStringRef, idx: isize, vm: &VirtualMachine, @@ -222,8 +222,8 @@ mod _json { } } - impl SlotCall for JsonScanner { - fn call(zelf: PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { + impl Callable for JsonScanner { + fn call(zelf: &PyRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { let (pystr, idx) = args.bind::<(PyStringRef, isize)>(vm)?; JsonScanner::call(zelf, pystr, idx, vm) } diff --git a/vm/src/vm.rs b/vm/src/vm.rs index f401f59d8..c4cb06280 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -886,7 +886,7 @@ impl VirtualMachine { match slot_call { Some(slot_call) => { self.trace_event(TraceEvent::Call)?; - let result = slot_call(callable.clone(), args, self); + let result = slot_call(callable, args, self); self.trace_event(TraceEvent::Return)?; result } From c76126f951ee582ad4e5dada3ea22f568103752c Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 24 Sep 2020 03:07:43 +0900 Subject: [PATCH 3/4] tp_cmp optimization --- derive/src/pyclass.rs | 7 ------- vm/src/obj/objfunction.rs | 33 ++++++++++++++++++--------------- vm/src/obj/objobject.rs | 26 ++++++++++++++++++-------- vm/src/obj/objtype.rs | 9 ++++++++- vm/src/slots.rs | 25 ++++++++++--------------- vm/src/vm.rs | 10 +++++----- 6 files changed, 59 insertions(+), 51 deletions(-) diff --git a/derive/src/pyclass.rs b/derive/src/pyclass.rs index ae3528937..cd149cddd 100644 --- a/derive/src/pyclass.rs +++ b/derive/src/pyclass.rs @@ -341,13 +341,6 @@ where quote! { slots.#slot_ident = Some(#into_func); } - } else if slot_name == "cmp" { - let into_func = quote_spanned! {ident.span() => - ::std::boxed::Box::new(Self::#ident) - }; - quote! { - slots.#slot_ident = Some(#into_func); - } } else { quote! { slots.#slot_ident.store(Some(Self::#ident as _)) diff --git a/vm/src/obj/objfunction.rs b/vm/src/obj/objfunction.rs index 1f5e1765b..4e1be30c9 100644 --- a/vm/src/obj/objfunction.rs +++ b/vm/src/obj/objfunction.rs @@ -19,7 +19,7 @@ use crate::pyobject::{ PyRef, PyResult, PyValue, TypeProtocol, }; use crate::scope::Scope; -use crate::slots::{Callable, SlotDescriptor}; +use crate::slots::{Callable, Comparable, PyComparisonOp, SlotDescriptor}; use crate::VirtualMachine; use itertools::Itertools; #[cfg(feature = "jit")] @@ -341,7 +341,23 @@ impl Callable for PyBoundMethod { } } -#[pyimpl(with(Callable), flags(HAS_DICT))] +impl Comparable for PyBoundMethod { + fn cmp( + zelf: PyRef, + other: PyObjectRef, + op: PyComparisonOp, + _vm: &VirtualMachine, + ) -> PyResult { + op.eq_only(|| { + let other = class_or_notimplemented!(Self, other); + Ok(PyComparisonValue::Implemented( + zelf.function.is(&other.function) && zelf.object.is(&other.object), + )) + }) + } +} + +#[pyimpl(with(Callable, Comparable), flags(HAS_DICT))] impl PyBoundMethod { pub fn new(object: PyObjectRef, function: PyObjectRef) -> Self { PyBoundMethod { object, function } @@ -377,19 +393,6 @@ impl PyBoundMethod { fn module(&self, vm: &VirtualMachine) -> Option { vm.get_attribute(self.function.clone(), "__module__").ok() } - - #[pymethod(magic)] - fn eq(&self, other: PyObjectRef) -> PyResult { - let other = class_or_notimplemented!(Self, other); - Ok(PyComparisonValue::Implemented( - self.function.is(&other.function) && self.object.is(&other.object), - )) - } - - #[pymethod(magic)] - fn ne(&self, other: PyObjectRef) -> PyResult { - Ok(self.eq(other)?.map(|v| !v)) - } } impl PyValue for PyBoundMethod { diff --git a/vm/src/obj/objobject.rs b/vm/src/obj/objobject.rs index 6b6283cae..f0d33d818 100644 --- a/vm/src/obj/objobject.rs +++ b/vm/src/obj/objobject.rs @@ -7,8 +7,8 @@ use crate::common::hash::PyHash; use crate::function::{OptionalArg, PyFuncArgs}; use crate::obj::objtype::PyClass; use crate::pyobject::{ - BorrowValue, IdProtocol, ItemProtocol, PyAttributes, PyClassImpl, PyComparisonValue, PyContext, - PyObject, PyObjectRef, PyResult, PyValue, TryFromObject, TypeProtocol, + BorrowValue, Either, IdProtocol, ItemProtocol, PyAttributes, PyClassImpl, PyComparisonValue, + PyContext, PyObject, PyObjectRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; use crate::slots::PyComparisonOp; use crate::vm::VirtualMachine; @@ -44,6 +44,16 @@ impl PyBaseObject { other: PyObjectRef, op: PyComparisonOp, vm: &VirtualMachine, + ) -> PyResult> { + Self::cmp(zelf, other, op, vm).map(Either::B) + } + + #[inline(always)] + fn cmp( + zelf: PyObjectRef, + other: PyObjectRef, + op: PyComparisonOp, + vm: &VirtualMachine, ) -> PyResult { let res = match op { PyComparisonOp::Eq => { @@ -76,7 +86,7 @@ impl PyBaseObject { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult { - Self::tp_cmp(zelf, other, PyComparisonOp::Eq, vm) + Self::cmp(zelf, other, PyComparisonOp::Eq, vm) } #[pymethod(magic)] fn ne( @@ -84,7 +94,7 @@ impl PyBaseObject { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult { - Self::tp_cmp(zelf, other, PyComparisonOp::Ne, vm) + Self::cmp(zelf, other, PyComparisonOp::Ne, vm) } #[pymethod(magic)] fn lt( @@ -92,7 +102,7 @@ impl PyBaseObject { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult { - Self::tp_cmp(zelf, other, PyComparisonOp::Lt, vm) + Self::cmp(zelf, other, PyComparisonOp::Lt, vm) } #[pymethod(magic)] fn le( @@ -100,7 +110,7 @@ impl PyBaseObject { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult { - Self::tp_cmp(zelf, other, PyComparisonOp::Le, vm) + Self::cmp(zelf, other, PyComparisonOp::Le, vm) } #[pymethod(magic)] fn ge( @@ -108,7 +118,7 @@ impl PyBaseObject { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult { - Self::tp_cmp(zelf, other, PyComparisonOp::Ge, vm) + Self::cmp(zelf, other, PyComparisonOp::Ge, vm) } #[pymethod(magic)] fn gt( @@ -116,7 +126,7 @@ impl PyBaseObject { other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult { - Self::tp_cmp(zelf, other, PyComparisonOp::Gt, vm) + Self::cmp(zelf, other, PyComparisonOp::Gt, vm) } #[pymethod(magic)] diff --git a/vm/src/obj/objtype.rs b/vm/src/obj/objtype.rs index 3361b2f29..daacc2744 100644 --- a/vm/src/obj/objtype.rs +++ b/vm/src/obj/objtype.rs @@ -14,7 +14,7 @@ use super::objtuple::PyTuple; use super::objweakref::PyWeak; use crate::function::{KwArgs, PyFuncArgs}; use crate::pyobject::{ - BorrowValue, IdProtocol, PyAttributes, PyClassImpl, PyContext, PyIterable, PyLease, + BorrowValue, Either, IdProtocol, PyAttributes, PyClassImpl, PyContext, PyIterable, PyLease, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; use crate::slots::{self, Callable, PyClassSlots, PyTpFlags}; @@ -157,6 +157,13 @@ impl PyClassRef { } as _; self.slots.del.store(Some(func)); } + "__eq__" | "__ne__" | "__le__" | "__lt__" | "__ge__" | "__gt__" => { + let func: slots::CmpFunc = |zelf, other, op, vm| { + let magic = get_class_magic(&zelf, op.method_name()); + vm.invoke(&magic, vec![zelf, other]).map(Either::A) + } as _; + self.slots.cmp.store(Some(func)) + } _ => (), } } diff --git a/vm/src/slots.rs b/vm/src/slots.rs index 2fb21ffa4..4055bb8b0 100644 --- a/vm/src/slots.rs +++ b/vm/src/slots.rs @@ -4,7 +4,7 @@ use crate::common::cell::PyRwLock; use crate::common::hash::PyHash; use crate::function::{OptionalArg, PyFuncArgs, PyNativeFunc}; use crate::pyobject::{ - IdProtocol, PyComparisonValue, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, + Either, IdProtocol, PyComparisonValue, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, }; use crate::VirtualMachine; use crossbeam_utils::atomic::AtomicCell; @@ -45,6 +45,12 @@ pub(crate) type DelFunc = fn(&PyObjectRef, &VirtualMachine) -> PyResult<()>; pub(crate) type DescrGetFunc = fn(PyObjectRef, Option, Option, &VirtualMachine) -> PyResult; pub(crate) type HashFunc = fn(&PyObjectRef, &VirtualMachine) -> PyResult; +pub(crate) type CmpFunc = fn( + PyObjectRef, + PyObjectRef, + PyComparisonOp, + &VirtualMachine, +) -> PyResult>; #[derive(Default)] pub struct PyClassSlots { @@ -55,20 +61,9 @@ pub struct PyClassSlots { pub call: AtomicCell>, pub descr_get: AtomicCell>, pub hash: AtomicCell>, - pub cmp: Option, + pub cmp: AtomicCell>, } -type CmpFunc = Box< - py_dyn_fn!( - dyn Fn( - PyObjectRef, - PyObjectRef, - PyComparisonOp, - &VirtualMachine, - ) -> PyResult - ), ->; - impl PyClassSlots { pub fn from_flags(flags: PyTpFlags) -> Self { Self { @@ -224,9 +219,9 @@ pub trait Comparable: PyValue { other: PyObjectRef, op: PyComparisonOp, vm: &VirtualMachine, - ) -> PyResult { + ) -> PyResult> { let zelf = PyRef::try_from_object(vm, zelf)?; - Self::cmp(zelf, other, op, vm) + Self::cmp(zelf, other, op, vm).map(Either::B) } fn cmp( diff --git a/vm/src/vm.rs b/vm/src/vm.rs index c4cb06280..b42ec6cf5 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -1399,11 +1399,11 @@ impl VirtualMachine { // TODO: _Py_EnterRecursiveCall(tstate, " in comparison") let call_cmp = |obj: PyObjectRef, other, op| { - let ret = if let Some(ref cmp) = obj.class().slots.cmp { - Some(cmp(obj, other, op, self)?.map(Either::B)) - } else if let Some(method) = self.get_method(obj, op.method_name()).transpose()? { - let ret = self.invoke(&method, vec![other])?; - Some(PyArithmaticValue::from_object(self, ret).map(Either::A)) + let ret = if let Some(ref cmp) = obj.class().first_in_mro(|cls| cls.slots.cmp.load()) { + Some(match cmp(obj, other, op, self)? { + Either::A(obj) => PyArithmaticValue::from_object(self, obj).map(Either::A), + Either::B(arithmatic) => arithmatic.map(Either::B), + }) } else { None }; From bebfa4b4f1b0c34963098fa1ea5bc5796dedf0b7 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 24 Sep 2020 03:10:48 +0900 Subject: [PATCH 4/4] tp_cmp always exists --- vm/src/vm.rs | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/vm/src/vm.rs b/vm/src/vm.rs index b42ec6cf5..844d3661b 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -1399,15 +1399,14 @@ impl VirtualMachine { // TODO: _Py_EnterRecursiveCall(tstate, " in comparison") let call_cmp = |obj: PyObjectRef, other, op| { - let ret = if let Some(ref cmp) = obj.class().first_in_mro(|cls| cls.slots.cmp.load()) { - Some(match cmp(obj, other, op, self)? { - Either::A(obj) => PyArithmaticValue::from_object(self, obj).map(Either::A), - Either::B(arithmatic) => arithmatic.map(Either::B), - }) - } else { - None - }; - Ok(ret) + let cmp = obj + .class() + .first_in_mro(|cls| cls.slots.cmp.load()) + .unwrap(); + Ok(match cmp(obj, other, op, self)? { + Either::A(obj) => PyArithmaticValue::from_object(self, obj).map(Either::A), + Either::B(arithmatic) => arithmatic.map(Either::B), + }) }; let mut checked_reverse_op = false; @@ -1417,19 +1416,18 @@ impl VirtualMachine { !v_class.is(&w_class) && objtype::issubclass(&w_class, &v_class) }; if is_strict_subclass { - if let Some(res) = call_cmp(w.clone(), v.clone(), swapped)? { - checked_reverse_op = true; - if let PyArithmaticValue::Implemented(x) = res { - return Ok(x); - } + let res = call_cmp(w.clone(), v.clone(), swapped)?; + checked_reverse_op = true; + if let PyArithmaticValue::Implemented(x) = res { + return Ok(x); } } - if let Some(PyArithmaticValue::Implemented(x)) = call_cmp(v.clone(), w.clone(), op)? { + if let PyArithmaticValue::Implemented(x) = call_cmp(v.clone(), w.clone(), op)? { return Ok(x); } if !checked_reverse_op { let res = call_cmp(w.clone(), v.clone(), swapped)?; - if let Some(PyArithmaticValue::Implemented(x)) = res { + if let PyArithmaticValue::Implemented(x) = res { return Ok(x); } }