From 2d179f076544760b35aa9075d03a3373bfcabdf6 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Thu, 11 Mar 2021 21:00:24 -0600 Subject: [PATCH 1/7] Optimize generic_getattribute_opt --- vm/src/vm.rs | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 7ab652421..b62d0950a 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -1259,15 +1259,23 @@ impl VirtualMachine { dict: Option, ) -> PyResult> { let name = name_str.borrow_value(); - let cls_attr = obj.class().get_attr(name); - - if let Some(ref attr) = cls_attr { - if attr.class().has_attr("__set__") { - if let Ok(r) = self.call_get_descriptor(attr.clone(), obj.clone()) { - return r.map(Some); + let obj_cls = obj.class(); + let cls_attr = match obj_cls.get_attr(name) { + Some(descr) => { + let descr_cls = descr.class(); + let descr_get = descr_cls.mro_find_map(|cls| cls.slots.descr_get.load()); + if let Some(descr_get) = descr_get { + if descr_cls.has_attr("__set__") { + drop(descr_cls); + let cls = PyLease::into_pyref(obj_cls).into_object(); + return descr_get(descr, Some(obj), Some(cls), self).map(Some); + } } + drop(descr_cls); + Some((descr, descr_get)) } - } + None => None, + }; let dict = dict.or_else(|| obj.dict()); @@ -1279,9 +1287,16 @@ impl VirtualMachine { if let Some(obj_attr) = attr { Ok(Some(obj_attr)) - } else if let Some(attr) = cls_attr { - self.call_if_get_descriptor(attr, obj).map(Some) - } else if let Some(getter) = obj.clone_class().get_attr("__getattr__") { + } else if let Some((attr, descr_get)) = cls_attr { + match descr_get { + Some(descr_get) => { + let cls = PyLease::into_pyref(obj_cls).into_object(); + descr_get(attr, Some(obj), Some(cls), self).map(Some) + } + None => Ok(Some(attr)), + } + } else if let Some(getter) = obj_cls.get_attr("__getattr__") { + drop(obj_cls); self.invoke(&getter, (obj, name_str)).map(Some) } else { Ok(None) From 79fa1b104f20195686b5c04a3cf84c3e19b939a1 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Sun, 14 Mar 2021 12:46:47 -0500 Subject: [PATCH 2/7] WIP getmethod --- vm/src/builtins/builtinfunc.rs | 4 +- vm/src/builtins/function.rs | 2 +- vm/src/builtins/object.rs | 2 +- vm/src/function.rs | 22 ++++---- vm/src/pyobject.rs | 94 ++++++++++++++++++++++++++++++++++ vm/src/slots.rs | 1 + vm/src/vm.rs | 16 ++---- 7 files changed, 117 insertions(+), 24 deletions(-) diff --git a/vm/src/builtins/builtinfunc.rs b/vm/src/builtins/builtinfunc.rs index 9092b8787..e7dd5d2d5 100644 --- a/vm/src/builtins/builtinfunc.rs +++ b/vm/src/builtins/builtinfunc.rs @@ -137,7 +137,7 @@ impl PyValue for PyBuiltinMethod { impl fmt::Debug for PyBuiltinMethod { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "method descriptor") + write!(f, "method descriptor for '{}'", self.value.name) } } @@ -172,7 +172,7 @@ impl Callable for PyBuiltinMethod { } } -#[pyimpl(with(SlotDescriptor, Callable))] +#[pyimpl(with(SlotDescriptor, Callable), flags(METHOD_DESCR))] impl PyBuiltinMethod { #[pyproperty(magic)] fn name(&self) -> PyStrRef { diff --git a/vm/src/builtins/function.rs b/vm/src/builtins/function.rs index 2c2a7aaed..914d494fd 100644 --- a/vm/src/builtins/function.rs +++ b/vm/src/builtins/function.rs @@ -302,7 +302,7 @@ impl PyValue for PyFunction { } } -#[pyimpl(with(SlotDescriptor, Callable), flags(HAS_DICT))] +#[pyimpl(with(SlotDescriptor, Callable), flags(HAS_DICT, METHOD_DESCR))] impl PyFunction { #[pyproperty(magic)] fn code(&self) -> PyCodeRef { diff --git a/vm/src/builtins/object.rs b/vm/src/builtins/object.rs index 213749139..09722d6a6 100644 --- a/vm/src/builtins/object.rs +++ b/vm/src/builtins/object.rs @@ -240,7 +240,7 @@ impl PyBaseObject { #[pymethod(name = "__getattribute__")] #[pyslot] - fn getattro(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult { + pub(crate) fn getattro(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult { vm_trace!("object.__getattribute__({:?}, {:?})", obj, name); vm.generic_getattribute(obj, name) } diff --git a/vm/src/function.rs b/vm/src/function.rs index 42626593f..bfef80c7c 100644 --- a/vm/src/function.rs +++ b/vm/src/function.rs @@ -13,8 +13,13 @@ use result_like::impl_option_like; use std::marker::PhantomData; use std::ops::RangeInclusive; -pub trait IntoFuncArgs { +pub trait IntoFuncArgs: Sized { fn into_args(self, vm: &VirtualMachine) -> FuncArgs; + fn into_method_args(self, obj: PyObjectRef, vm: &VirtualMachine) -> FuncArgs { + let mut args = self.into_args(vm); + args.prepend_arg(obj); + args + } } impl IntoFuncArgs for T @@ -26,11 +31,6 @@ where } } -macro_rules! count { - () => (0usize); - ( $x:tt $($xs:tt)* ) => (1usize + count!($($xs)*)); -} - // A tuple of values that each implement `IntoPyObject` represents a sequence of // arguments that can be bound and passed to a built-in function. macro_rules! into_func_args_from_tuple { @@ -42,9 +42,13 @@ macro_rules! into_func_args_from_tuple { #[inline] fn into_args(self, vm: &VirtualMachine) -> FuncArgs { let ($($n,)*) = self; - let mut result = Vec::with_capacity(count!($($n,)*) + 1); - $(result.push($n.into_pyobject(vm), );)* - result.into() + vec![$($n.into_pyobject(vm),)*].into() + } + + #[inline] + fn into_method_args(self, obj: PyObjectRef, vm: &VirtualMachine) -> FuncArgs { + let ($($n,)*) = self; + vec![obj, $($n.into_pyobject(vm),)*].into() } } }; diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index e8f688b93..8df2d812c 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -1251,3 +1251,97 @@ pub fn hash_iter_unordered<'a, I: IntoIterator>( ) -> PyResult { rustpython_common::hash::hash_iter_unordered(iter, |obj| vm._hash(obj)) } + +#[derive(Debug)] +pub enum PyMethod { + Function { + target: PyObjectRef, + func: PyObjectRef, + }, + Attribute(PyObjectRef), +} + +impl PyMethod { + pub fn get(obj: PyObjectRef, name: pystr::PyStrRef, vm: &VirtualMachine) -> PyResult { + let cls = obj.class(); + let getattro = cls.mro_find_map(|cls| cls.slots.getattro.load()).unwrap(); + if getattro as usize != object::PyBaseObject::getattro as usize { + drop(cls); + return getattro(obj, name, vm).map(Self::Attribute); + } + + let mut is_method = false; + + let cls_attr = match cls.get_attr(name.borrow_value()) { + Some(descr) => { + let descr_cls = descr.class(); + let descr_get = if descr_cls.slots.flags.has_feature(PyTpFlags::METHOD_DESCR) { + is_method = true; + None + } else { + let descr_get = descr_cls.mro_find_map(|cls| cls.slots.descr_get.load()); + if let Some(descr_get) = descr_get { + if descr_cls.has_attr("__set__") { + drop(descr_cls); + let cls = PyLease::into_pyref(cls).into_object(); + return descr_get(descr, Some(obj), Some(cls), vm).map(Self::Attribute); + } + } + descr_get + }; + drop(descr_cls); + Some((descr, descr_get)) + } + None => None, + }; + + if let Some(dict) = obj.dict() { + if let Some(attr) = dict.get_item_option(name.clone(), vm)? { + return Ok(Self::Attribute(attr)); + } + } + + if let Some((attr, descr_get)) = cls_attr { + match descr_get { + None if is_method => { + drop(cls); + Ok(Self::Function { + target: obj, + func: attr, + }) + } + Some(descr_get) => { + let cls = PyLease::into_pyref(cls).into_object(); + descr_get(attr, Some(obj), Some(cls), vm).map(Self::Attribute) + } + None => Ok(Self::Attribute(attr)), + } + } else if let Some(getter) = cls.get_attr("__getattr__") { + drop(cls); + vm.invoke(&getter, (obj, name)).map(Self::Attribute) + } else { + Err(vm.new_attribute_error(format!( + "'{}' object has not attribute '{}'", + cls.name, name + ))) + } + } + + pub fn invoke(self, args: impl IntoFuncArgs, vm: &VirtualMachine) -> PyResult { + let (func, args) = match self { + PyMethod::Function { target, func } => (func, args.into_method_args(target, vm)), + PyMethod::Attribute(func) => (func, args.into_args(vm)), + }; + vm.invoke(&func, args) + } + + pub fn invoke_ref(&self, args: impl IntoFuncArgs, vm: &VirtualMachine) -> PyResult { + let (func, args) = match self { + PyMethod::Function { target, func } => { + (func, args.into_method_args(target.clone(), vm)) + } + PyMethod::Attribute(func) => (func, args.into_args(vm)), + }; + vm.invoke(func, args) + } +} diff --git a/vm/src/slots.rs b/vm/src/slots.rs index 833f271d9..1d9dadfe9 100644 --- a/vm/src/slots.rs +++ b/vm/src/slots.rs @@ -15,6 +15,7 @@ bitflags! { pub struct PyTpFlags: u64 { const HEAPTYPE = 1 << 9; const BASETYPE = 1 << 10; + const METHOD_DESCR = 1 << 17; const HAS_DICT = 1 << 40; #[cfg(debug_assertions)] diff --git a/vm/src/vm.rs b/vm/src/vm.rs index b62d0950a..e5594011b 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -29,8 +29,8 @@ use crate::frame::{ExecutionResult, Frame, FrameRef}; use crate::function::{FuncArgs, IntoFuncArgs}; use crate::pyobject::{ BorrowValue, Either, IdProtocol, IntoPyObject, ItemProtocol, PyArithmaticValue, PyContext, - PyObject, PyObjectRef, PyRef, PyRefExact, PyResult, PyValue, TryFromObject, TryIntoRef, - TypeProtocol, + PyLease, PyMethod, PyObject, PyObjectRef, PyRef, PyRefExact, PyResult, PyValue, TryFromObject, + TryIntoRef, TypeProtocol, }; use crate::scope::Scope; use crate::slots::PyComparisonOp; @@ -965,21 +965,15 @@ impl VirtualMachine { self.call_get_descriptor(attr, obj).unwrap_or_else(Ok) } + #[inline] pub fn call_method(&self, obj: &PyObjectRef, method_name: &str, args: T) -> PyResult where T: IntoFuncArgs, { flame_guard!(format!("call_method({:?})", method_name)); - // This is only used in the vm for magic methods, which use a greatly simplified attribute lookup. - match obj.get_class_attr(method_name) { - Some(func) => { - vm_trace!("vm.call_method {:?} {:?} -> {:?}", obj, method_name, func); - let wrapped = self.call_if_get_descriptor(func, obj.clone())?; - self.invoke(&wrapped, args) - } - None => Err(self.new_type_error(format!("Unsupported method: {}", method_name))), - } + PyMethod::get(obj.clone(), PyStr::from(method_name).into_ref(self), self)? + .invoke(args, self) } fn _invoke(&self, callable: &PyObjectRef, args: FuncArgs) -> PyResult { From ac320f572d02ed847fd9b9be09d8d33efc1a1bfd Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Wed, 3 Feb 2021 09:07:32 -0600 Subject: [PATCH 3/7] wip descr_set --- vm/src/builtins/getset.rs | 54 +++++++++++++++++++++---------------- vm/src/builtins/object.rs | 10 ++++--- vm/src/builtins/property.rs | 40 ++++++++++++++++----------- vm/src/builtins/pytype.rs | 51 ++++++++++++++++++++++------------- vm/src/macros.rs | 4 +-- vm/src/pyobject.rs | 4 +-- vm/src/slots.rs | 3 +++ vm/src/vm.rs | 5 +++- 8 files changed, 105 insertions(+), 66 deletions(-) diff --git a/vm/src/builtins/getset.rs b/vm/src/builtins/getset.rs index 5a84c5724..8b91ca62d 100644 --- a/vm/src/builtins/getset.rs +++ b/vm/src/builtins/getset.rs @@ -303,29 +303,37 @@ impl PyGetSet { impl PyGetSet { // Descriptor methods - #[pymethod(magic)] - fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - if let Some(ref f) = self.setter { - f(vm, obj, value) - } else { - Err(vm.new_attribute_error(format!( - "attribute '{}' of '{}' objects is not writable", - self.name, - obj.class().name - ))) - } - } - - #[pymethod(magic)] - fn delete(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - if let Some(ref f) = self.deleter { - f(vm, obj) - } else { - Err(vm.new_attribute_error(format!( - "attribute '{}' of '{}' objects is not writable", - self.name, - obj.class().name - ))) + #[pyslot] + fn descr_set( + zelf: PyObjectRef, + obj: PyObjectRef, + value: Option, + vm: &VirtualMachine, + ) -> PyResult<()> { + let zelf = PyRef::::try_from_object(vm, zelf)?; + match value { + Some(value) => { + if let Some(ref f) = zelf.setter { + f(vm, obj, value) + } else { + Err(vm.new_attribute_error(format!( + "attribute '{}' of '{}' objects is not writable", + zelf.name, + obj.class().name + ))) + } + } + None => { + if let Some(ref f) = zelf.deleter { + f(vm, obj) + } else { + Err(vm.new_attribute_error(format!( + "attribute '{}' of '{}' objects is not writable", + zelf.name, + obj.class().name + ))) + } + } } } diff --git a/vm/src/builtins/object.rs b/vm/src/builtins/object.rs index 09722d6a6..487b2337b 100644 --- a/vm/src/builtins/object.rs +++ b/vm/src/builtins/object.rs @@ -144,8 +144,9 @@ impl PyBaseObject { #[pymethod(magic)] fn delattr(obj: PyObjectRef, attr_name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> { if let Some(attr) = obj.get_class_attr(attr_name.borrow_value()) { - if let Some(descriptor) = attr.get_class_attr("__delete__") { - return vm.invoke(&descriptor, vec![attr, obj]).map(|_| ()); + let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load()); + if let Some(descriptor) = descr_set { + return descriptor(attr, obj, None, vm); } } @@ -292,8 +293,9 @@ pub(crate) fn setattr( vm_trace!("object.__setattr__({:?}, {}, {:?})", obj, attr_name, value); if let Some(attr) = obj.get_class_attr(attr_name.borrow_value()) { - if let Some(descriptor) = attr.get_class_attr("__set__") { - return vm.invoke(&descriptor, vec![attr, obj, value]).map(|_| ()); + let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load()); + if let Some(descriptor) = descr_set { + return descriptor(attr, obj, Some(value), vm); } } diff --git a/vm/src/builtins/property.rs b/vm/src/builtins/property.rs index cf18ac968..291743481 100644 --- a/vm/src/builtins/property.rs +++ b/vm/src/builtins/property.rs @@ -6,7 +6,7 @@ use crate::common::lock::PyRwLock; use super::pytype::PyTypeRef; use crate::function::FuncArgs; use crate::pyobject::{ - PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, + PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; use crate::slots::SlotDescriptor; use crate::vm::VirtualMachine; @@ -111,21 +111,29 @@ impl PyProperty { // Descriptor methods - #[pymethod(name = "__set__")] - fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(ref setter) = self.setter.read().as_ref() { - vm.invoke(setter, vec![obj, value]) - } else { - Err(vm.new_attribute_error("can't set attribute".to_owned())) - } - } - - #[pymethod(name = "__delete__")] - fn delete(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(ref deleter) = self.deleter.read().as_ref() { - vm.invoke(deleter, (obj,)) - } else { - Err(vm.new_attribute_error("can't delete attribute".to_owned())) + #[pyslot] + fn descr_set( + zelf: PyObjectRef, + obj: PyObjectRef, + value: Option, + vm: &VirtualMachine, + ) -> PyResult<()> { + let zelf = PyRef::::try_from_object(vm, zelf)?; + match value { + Some(value) => { + if let Some(ref setter) = zelf.setter.read().as_ref() { + vm.invoke(setter, vec![obj, value]).map(drop) + } else { + Err(vm.new_attribute_error("can't set attribute".to_owned())) + } + } + None => { + if let Some(ref deleter) = zelf.deleter.read().as_ref() { + vm.invoke(deleter, (obj,)).map(drop) + } else { + Err(vm.new_attribute_error("can't delete attribute".to_owned())) + } + } } } diff --git a/vm/src/builtins/pytype.rs b/vm/src/builtins/pytype.rs index cc6565447..34b2e9224 100644 --- a/vm/src/builtins/pytype.rs +++ b/vm/src/builtins/pytype.rs @@ -167,6 +167,16 @@ impl PyType { } as _; update_slot!(descr_get, func); } + "__set__" | "__delete__" => { + let func: slots::DescrSetFunc = |zelf, obj, value, vm| { + match value { + Some(val) => vm.call_method(&zelf, "__set__", (obj, val)), + None => vm.call_method(&zelf, "__delete__", (obj,)), + } + .map(drop) + } as _; + update_slot!(descr_set, func); + } "__hash__" => { let func: slots::HashFunc = |zelf, vm| { let magic = get_class_magic(&zelf, "__hash__"); @@ -347,9 +357,9 @@ impl PyType { vm: &VirtualMachine, ) -> PyResult<()> { if let Some(attr) = zelf.get_class_attr(attr_name.borrow_value()) { - if let Some(ref descriptor) = attr.get_class_attr("__set__") { - vm.invoke(descriptor, (attr, zelf, value))?; - return Ok(()); + let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load()); + if let Some(descriptor) = descr_set { + return descriptor(attr, zelf.into_object(), Some(value), vm); } } let attr_name = attr_name.borrow_value(); @@ -364,8 +374,9 @@ impl PyType { #[pymethod(magic)] fn delattr(zelf: PyRef, attr_name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> { if let Some(attr) = zelf.get_class_attr(attr_name.borrow_value()) { - if let Some(ref descriptor) = attr.get_class_attr("__delete__") { - return vm.invoke(descriptor, (attr, zelf)).map(|_| ()); + let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load()); + if let Some(set) = descr_set { + return set(attr, zelf.into_object(), None, vm); } } @@ -494,7 +505,7 @@ impl PyType { ) .map_err(|e| vm.new_type_error(e))?; - vm.ctx.add_tp_new_wrapper(&typ); + vm.ctx.add_slot_wrappers(&typ); // avoid deadlock let attributes = typ @@ -558,10 +569,11 @@ impl SlotGetattro for PyType { if let Some(ref attr) = mcl_attr { let attr_class = attr.class(); - if attr_class.has_attr("__set__") { - if let Some(ref descr_get) = - attr_class.mro_find_map(|cls| cls.slots.descr_get.load()) - { + if attr_class + .mro_find_map(|cls| cls.slots.descr_set.load()) + .is_some() + { + if let Some(descr_get) = attr_class.mro_find_map(|cls| cls.slots.descr_get.load()) { let mcl = PyLease::into_pyref(mcl).into_object(); return descr_get(attr.clone(), Some(zelf.into_object()), Some(mcl), vm); } @@ -644,19 +656,22 @@ fn subtype_set_dict(obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) - let cls = obj.clone_class(); match find_base_dict_descr(&cls, vm) { Some(descr) => { - descr - .get_class_attr("__set__") - .map(|set| vm.invoke(&set, vec![descr, obj, value])) - .unwrap_or_else(|| { - Err(vm.new_type_error(format!( + let descr_set = descr + .class() + .mro_find_map(|cls| cls.slots.descr_set.load()) + .ok_or_else(|| { + vm.new_type_error(format!( "this __dict__ descriptor does not support '{}' objects", cls.name - ))) + )) })?; + descr_set(descr, obj, Some(value), vm) + } + None => { + object::object_set_dict(obj, PyDictRef::try_from_object(vm, value)?, vm)?; + Ok(()) } - None => object::object_set_dict(obj, PyDictRef::try_from_object(vm, value)?, vm)?, } - Ok(()) } /* diff --git a/vm/src/macros.rs b/vm/src/macros.rs index d4317dc17..847aef3ba 100644 --- a/vm/src/macros.rs +++ b/vm/src/macros.rs @@ -30,7 +30,7 @@ macro_rules! py_class { $($crate::py_class!(@extract_slots($ctx, &mut slots, $name, $value));)* let py_class = $ctx.new_class($class_name, $class_base, slots); $($crate::py_class!(@extract_attrs($ctx, &py_class, $name, $value));)* - $ctx.add_tp_new_wrapper(&py_class); + $ctx.add_slot_wrappers(&py_class); py_class } }; @@ -55,7 +55,7 @@ macro_rules! extend_class { $( $class.set_str_attr($name, $value); )* - $ctx.add_tp_new_wrapper(&$class); + $ctx.add_slot_wrappers(&$class); }; } diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 8df2d812c..06ad503d9 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -359,7 +359,7 @@ impl PyContext { PyObject::new(object::PyBaseObject, class, dict) } - pub fn add_tp_new_wrapper(&self, ty: &PyTypeRef) { + pub fn add_slot_wrappers(&self, ty: &PyTypeRef) { if !ty.attributes.read().contains_key("__new__") { let new_wrapper = self.new_bound_method(self.tp_new_wrapper.clone(), ty.clone().into_object()); @@ -1075,7 +1075,7 @@ pub trait PyClassImpl: PyClassDef { ); } Self::impl_extend_class(ctx, class); - ctx.add_tp_new_wrapper(&class); + ctx.add_slot_wrappers(&class); if let Some(doc) = Self::DOC { class.set_str_attr("__doc__", ctx.new_str(doc)); } diff --git a/vm/src/slots.rs b/vm/src/slots.rs index 1d9dadfe9..be0aa937d 100644 --- a/vm/src/slots.rs +++ b/vm/src/slots.rs @@ -47,6 +47,8 @@ pub(crate) type GenericMethod = fn(&PyObjectRef, FuncArgs, &VirtualMachine) -> P pub(crate) type DelFunc = fn(&PyObjectRef, &VirtualMachine) -> PyResult<()>; pub(crate) type DescrGetFunc = fn(PyObjectRef, Option, Option, &VirtualMachine) -> PyResult; +pub(crate) type DescrSetFunc = + fn(PyObjectRef, PyObjectRef, Option, &VirtualMachine) -> PyResult<()>; pub(crate) type HashFunc = fn(&PyObjectRef, &VirtualMachine) -> PyResult; pub(crate) type CmpFunc = fn( &PyObjectRef, @@ -67,6 +69,7 @@ pub struct PyTypeSlots { pub del: AtomicCell>, pub call: AtomicCell>, pub descr_get: AtomicCell>, + pub descr_set: AtomicCell>, pub hash: AtomicCell>, pub cmp: AtomicCell>, pub getattro: AtomicCell>, diff --git a/vm/src/vm.rs b/vm/src/vm.rs index e5594011b..6743f4872 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -1259,7 +1259,10 @@ impl VirtualMachine { let descr_cls = descr.class(); let descr_get = descr_cls.mro_find_map(|cls| cls.slots.descr_get.load()); if let Some(descr_get) = descr_get { - if descr_cls.has_attr("__set__") { + if descr_cls + .mro_find_map(|cls| cls.slots.descr_set.load()) + .is_some() + { drop(descr_cls); let cls = PyLease::into_pyref(obj_cls).into_object(); return descr_get(descr, Some(obj), Some(cls), self).map(Some); From f50f930255e8514d226531c741a9d061cfa5f612 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Sun, 14 Mar 2021 18:22:15 -0500 Subject: [PATCH 4/7] Add setattro slot, split call_method and call_special_method --- Lib/test/test_fileio.py | 7 +- Lib/test/test_with.py | 2 - vm/src/builtins/iter.rs | 5 +- vm/src/builtins/make_module.rs | 29 +++--- vm/src/builtins/object.rs | 56 ++++++------ vm/src/builtins/pytype.rs | 155 ++++++++++++++------------------- vm/src/builtins/weakproxy.rs | 18 ++-- vm/src/cformat.rs | 11 ++- vm/src/format.rs | 10 +-- vm/src/frame.rs | 42 +++++++-- vm/src/pyobject.rs | 71 +++++++++++++-- vm/src/slots.rs | 43 +++++++++ vm/src/stdlib/thread.rs | 59 ++++++------- vm/src/vm.rs | 82 +++++++++++++---- 14 files changed, 372 insertions(+), 218 deletions(-) diff --git a/Lib/test/test_fileio.py b/Lib/test/test_fileio.py index cc5f16a88..5a0c31df8 100644 --- a/Lib/test/test_fileio.py +++ b/Lib/test/test_fileio.py @@ -427,6 +427,7 @@ class PyAutoFileTests(AutoFileTests, unittest.TestCase): class OtherFileTests: + @unittest.skip("TODO: non-deterministic failures, FileIO.seekable()?") def testAbles(self): try: f = self.FileIO(TESTFN, "w") @@ -673,12 +674,6 @@ class PyOtherFileTests(OtherFileTests, unittest.TestCase): FileIO = _pyio.FileIO modulename = '_pyio' - # TODO: RUSTPYTHON - if sys.platform == "win32": - @unittest.expectedFailure - def testAbles(self): - super().testAbles() - # TODO: RUSTPYTHON if sys.platform == "win32": @unittest.expectedFailure diff --git a/Lib/test/test_with.py b/Lib/test/test_with.py index 3ace8f48b..117fb4630 100644 --- a/Lib/test/test_with.py +++ b/Lib/test/test_with.py @@ -109,8 +109,6 @@ class FailureTestCase(unittest.TestCase): with foo: pass self.assertRaises(NameError, fooNotDeclared) - # TODO: RUSTPYTHON - @unittest.expectedFailure def testEnterAttributeError1(self): class LacksEnter(object): def __exit__(self, type, value, traceback): diff --git a/vm/src/builtins/iter.rs b/vm/src/builtins/iter.rs index 6b77321a6..99ffe1f7d 100644 --- a/vm/src/builtins/iter.rs +++ b/vm/src/builtins/iter.rs @@ -6,7 +6,8 @@ use crossbeam_utils::atomic::AtomicCell; use super::pytype::PyTypeRef; use crate::pyobject::{ - PyCallable, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, + ItemProtocol, PyCallable, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, + TypeProtocol, }; use crate::slots::PyIter; use crate::vm::VirtualMachine; @@ -61,7 +62,7 @@ impl PyIter for PySequenceIterator { let step: isize = if zelf.reversed { -1 } else { 1 }; let pos = zelf.position.fetch_add(step); if pos >= 0 { - match vm.call_method(&zelf.obj, "__getitem__", (pos,)) { + match zelf.obj.get_item(pos, vm) { Err(ref e) if e.isinstance(&vm.ctx.exceptions.index_error) => { Err(vm.new_stop_iteration()) } diff --git a/vm/src/builtins/make_module.rs b/vm/src/builtins/make_module.rs index 87fdf5559..04657d251 100644 --- a/vm/src/builtins/make_module.rs +++ b/vm/src/builtins/make_module.rs @@ -13,7 +13,7 @@ mod decl { use crate::builtins::code::PyCodeRef; use crate::builtins::dict::PyDictRef; use crate::builtins::function::{PyCellRef, PyFunctionRef}; - use crate::builtins::int::{self, PyIntRef}; + use crate::builtins::int::PyIntRef; use crate::builtins::iter::{PyCallableIterator, PySequenceIterator}; use crate::builtins::list::{PyList, SortOptions}; use crate::builtins::pybool::IntoPyBool; @@ -37,7 +37,7 @@ mod decl { use crate::slots::PyComparisonOp; use crate::vm::VirtualMachine; use crate::{py_io, sysmodule}; - use num_traits::{Signed, ToPrimitive, Zero}; + use num_traits::{Signed, Zero}; #[pyfunction] fn abs(x: PyObjectRef, vm: &VirtualMachine) -> PyResult { @@ -197,7 +197,10 @@ mod decl { #[pyfunction] fn dir(obj: OptionalArg, vm: &VirtualMachine) -> PyResult { let seq = match obj { - OptionalArg::Present(obj) => vm.call_method(&obj, "__dir__", ())?, + OptionalArg::Present(obj) => vm + .get_special_method(obj, "__dir__")? + .map_err(|_obj| vm.new_type_error("object does not provide __dir__".to_owned()))? + .invoke((), vm)?, OptionalArg::Missing => vm.call_method(vm.current_locals()?.as_object(), "keys", ())?, }; let sorted = sorted(seq, Default::default(), vm)?; @@ -717,8 +720,7 @@ mod decl { vm.get_method_or_type_error(obj.clone(), "__getitem__", || { "argument to reversed() must be a sequence".to_owned() })?; - let len = vm.call_method(&obj, "__len__", ())?; - let len = int::get_value(&len).to_isize().unwrap(); + let len = vm.obj_len(&obj)? as isize; let obj_iterator = PySequenceIterator::new_reversed(obj, len); Ok(obj_iterator.into_object(vm)) } @@ -734,17 +736,24 @@ mod decl { #[pyfunction] fn round(RoundArgs { number, ndigits }: RoundArgs, vm: &VirtualMachine) -> PyResult { - let rounded = match ndigits.flatten() { + let meth = vm + .get_special_method(number, "__round__")? + .map_err(|number| { + vm.new_type_error(format!( + "type {} doesn't define __round__", + number.class().name + )) + })?; + match ndigits.flatten() { Some(obj) => { let ndigits = vm.to_index(&obj)?; - vm.call_method(&number, "__round__", (ndigits,))? + meth.invoke((ndigits,), vm) } None => { // without a parameter, the result type is coerced to int - vm.call_method(&number, "__round__", ())? + meth.invoke((), vm) } - }; - Ok(rounded) + } } #[pyfunction] diff --git a/vm/src/builtins/object.rs b/vm/src/builtins/object.rs index 487b2337b..d4362ae9e 100644 --- a/vm/src/builtins/object.rs +++ b/vm/src/builtins/object.rs @@ -131,40 +131,34 @@ impl PyBaseObject { Self::cmp(&zelf, &other, PyComparisonOp::Gt, vm) } - #[pymethod(magic)] - pub(crate) fn setattr( + #[pymethod] + fn __setattr__( obj: PyObjectRef, attr_name: PyStrRef, value: PyObjectRef, vm: &VirtualMachine, + ) -> PyResult<()> { + setattr(&obj, attr_name, Some(value), vm) + } + + #[pymethod] + fn __delattr__(obj: PyObjectRef, attr_name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> { + setattr(&obj, attr_name, None, vm) + } + + #[pyslot] + fn tp_setattro( + obj: &PyObjectRef, + attr_name: PyStrRef, + value: Option, + vm: &VirtualMachine, ) -> PyResult<()> { setattr(obj, attr_name, value, vm) } #[pymethod(magic)] - fn delattr(obj: PyObjectRef, attr_name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> { - if let Some(attr) = obj.get_class_attr(attr_name.borrow_value()) { - let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load()); - if let Some(descriptor) = descr_set { - return descriptor(attr, obj, None, vm); - } - } - - if let Some(dict) = obj.dict() { - dict.del_item(attr_name.borrow_value(), vm)?; - Ok(()) - } else { - Err(vm.new_attribute_error(format!( - "'{}' object has no attribute '{}'", - obj.class().name, - attr_name.borrow_value() - ))) - } - } - - #[pymethod(magic)] - fn str(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult { - vm.call_method(&zelf, "__repr__", ()) + fn str(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult { + vm.to_repr(&zelf) } #[pymethod(magic)] @@ -285,9 +279,9 @@ pub fn object_set_dict(obj: PyObjectRef, dict: PyDictRef, vm: &VirtualMachine) - #[cfg_attr(feature = "flame-it", flame)] pub(crate) fn setattr( - obj: PyObjectRef, + obj: &PyObjectRef, attr_name: PyStrRef, - value: PyObjectRef, + value: Option, vm: &VirtualMachine, ) -> PyResult<()> { vm_trace!("object.__setattr__({:?}, {}, {:?})", obj, attr_name, value); @@ -295,12 +289,16 @@ pub(crate) fn setattr( if let Some(attr) = obj.get_class_attr(attr_name.borrow_value()) { let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load()); if let Some(descriptor) = descr_set { - return descriptor(attr, obj, Some(value), vm); + return descriptor(attr, obj.clone(), value, vm); } } if let Some(dict) = obj.dict() { - dict.set_item(attr_name.borrow_value(), value, vm)?; + if let Some(value) = value { + dict.set_item(attr_name, value, vm)?; + } else { + dict.del_item(attr_name, vm)?; + } Ok(()) } else { Err(vm.new_attribute_error(format!( diff --git a/vm/src/builtins/pytype.rs b/vm/src/builtins/pytype.rs index 34b2e9224..adfe80b60 100644 --- a/vm/src/builtins/pytype.rs +++ b/vm/src/builtins/pytype.rs @@ -21,7 +21,7 @@ use crate::pyobject::{ BorrowValue, Either, IdProtocol, PyAttributes, PyClassImpl, PyContext, PyLease, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; -use crate::slots::{self, Callable, PyTpFlags, PyTypeSlots, SlotGetattro}; +use crate::slots::{self, Callable, PyTpFlags, PyTypeSlots, SlotGetattro, SlotSetattro}; use crate::vm::VirtualMachine; use itertools::Itertools; use std::ops::Deref; @@ -145,7 +145,6 @@ impl PyType { } pub(crate) fn update_slot(&self, name: &str, add: bool) { - // self is the resolved class in get_class_magic macro_rules! update_slot { ($name:ident, $func:expr) => {{ self.slots.$name.store(if add { Some($func) } else { None }); @@ -153,34 +152,28 @@ impl PyType { } match name { "__call__" => { - let func: slots::GenericMethod = |zelf, args, vm| { - let magic = get_class_magic(&zelf, "__call__"); - let magic = vm.call_if_get_descriptor(magic, zelf.clone())?; - vm.invoke(&magic, args) - } as _; + let func: slots::GenericMethod = + |zelf, args, vm| vm.call_special_method(zelf.clone(), "__call__", args); update_slot!(call, func); } "__get__" => { - let func: slots::DescrGetFunc = |zelf, obj, cls, vm| { - let magic = get_class_magic(&zelf, "__get__"); - vm.invoke(&magic, (zelf, obj, cls)) - } as _; + let func: slots::DescrGetFunc = + |zelf, obj, cls, vm| vm.call_special_method(zelf, "__get__", (obj, cls)); update_slot!(descr_get, func); } "__set__" | "__delete__" => { let func: slots::DescrSetFunc = |zelf, obj, value, vm| { match value { - Some(val) => vm.call_method(&zelf, "__set__", (obj, val)), - None => vm.call_method(&zelf, "__delete__", (obj,)), + Some(val) => vm.call_special_method(zelf, "__set__", (obj, val)), + None => vm.call_special_method(zelf, "__delete__", (obj,)), } .map(drop) - } as _; + }; update_slot!(descr_set, func); } "__hash__" => { let func: slots::HashFunc = |zelf, vm| { - let magic = get_class_magic(&zelf, "__hash__"); - let hash_obj = vm.invoke(&magic, vec![zelf.clone()])?; + let hash_obj = vm.call_special_method(zelf.clone(), "__hash__", ())?; match hash_obj.payload_if_subclass::(vm) { Some(py_int) => { Ok(rustpython_common::hash::hash_bigint(py_int.borrow_value())) @@ -193,40 +186,44 @@ impl PyType { } "__del__" => { let func: slots::DelFunc = |zelf, vm| { - let magic = get_class_magic(&zelf, "__del__"); - let _ = vm.invoke(&magic, vec![zelf.clone()])?; + vm.call_special_method(zelf.clone(), "__del__", ())?; Ok(()) } as _; update_slot!(del, func); } "__eq__" | "__ne__" | "__le__" | "__lt__" | "__ge__" | "__gt__" => { let func: slots::CmpFunc = |zelf, other, op, vm| { - // TODO: differentiate between op types; get_class_magic would panic - let magic = get_class_magic(&zelf, op.method_name()); - vm.invoke(&magic, vec![zelf.clone(), other.clone()]) + vm.call_special_method(zelf.clone(), op.method_name(), (other.clone(),)) .map(Either::A) } as _; update_slot!(cmp, func); } "__getattribute__" => { - let func: slots::GetattroFunc = |zelf, name, vm| { - let magic = get_class_magic(&zelf, "__getattribute__"); - vm.invoke(&magic, (zelf, name)) - }; + let func: slots::GetattroFunc = + |zelf, name, vm| vm.call_special_method(zelf, "__getattribute__", (name,)); update_slot!(getattro, func); } - "__iter__" => { - let func: slots::IterFunc = |zelf, vm| { - let magic = get_class_magic(&zelf, "__iter__"); - vm.invoke(&magic, (zelf,)) + "__setattr__" => { + let func: slots::SetattroFunc = |zelf, name, value, vm| { + match value { + Some(value) => { + vm.call_special_method(zelf.clone(), "__setattr__", (name, value))?; + } + None => { + vm.call_special_method(zelf.clone(), "__delattr__", (name,))?; + } + }; + Ok(()) }; + update_slot!(setattro, func); + } + "__iter__" => { + let func: slots::IterFunc = |zelf, vm| vm.call_special_method(zelf, "__iter__", ()); update_slot!(iter, func); } "__next__" => { - let func: slots::IterNextFunc = |zelf, vm| { - let magic = get_class_magic(zelf, "__next__"); - vm.invoke(&magic, (zelf.clone(),)) - }; + let func: slots::IterNextFunc = + |zelf, vm| vm.call_special_method(zelf.clone(), "__next__", ()); update_slot!(iternext, func); } _ => {} @@ -248,17 +245,7 @@ impl PyTypeRef { } } -#[inline] -fn get_class_magic(zelf: &PyObjectRef, name: &str) -> PyObjectRef { - zelf.get_class_attr(name).unwrap() - - // TODO: we already looked up the matching class but lost the information here - // let cls = zelf.class(); - // let attrs = cls.attributes.read(); - // attrs.get(name).unwrap().clone() -} - -#[pyimpl(with(SlotGetattro, Callable), flags(BASETYPE))] +#[pyimpl(with(SlotGetattro, SlotSetattro, Callable), flags(BASETYPE))] impl PyType { #[pyproperty(name = "__mro__")] fn get_mro(zelf: PyRef) -> PyTuple { @@ -349,51 +336,6 @@ impl PyType { vm.ctx.new_dict() } - #[pymethod(magic)] - fn setattr( - zelf: PyRef, - attr_name: PyStrRef, - value: PyObjectRef, - vm: &VirtualMachine, - ) -> PyResult<()> { - if let Some(attr) = zelf.get_class_attr(attr_name.borrow_value()) { - let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load()); - if let Some(descriptor) = descr_set { - return descriptor(attr, zelf.into_object(), Some(value), vm); - } - } - let attr_name = attr_name.borrow_value(); - let mut attributes = zelf.attributes.write(); - attributes.insert(attr_name.to_owned(), value); - if attr_name.starts_with("__") && attr_name.ends_with("__") { - zelf.update_slot(attr_name, true); - } - Ok(()) - } - - #[pymethod(magic)] - fn delattr(zelf: PyRef, attr_name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> { - if let Some(attr) = zelf.get_class_attr(attr_name.borrow_value()) { - let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load()); - if let Some(set) = descr_set { - return set(attr, zelf.into_object(), None, vm); - } - } - - let mut attributes = zelf.attributes.write(); - if attributes.remove(attr_name.borrow_value()).is_none() { - return Err(vm.new_exception( - vm.ctx.exceptions.attribute_error.clone(), - vec![attr_name.into_object()], - )); - }; - let attr_name = attr_name.borrow_value(); - if attr_name.starts_with("__") && attr_name.ends_with("__") { - zelf.update_slot(attr_name, false); - } - Ok(()) - } - #[pymethod(magic)] fn subclasses(&self) -> PyList { let mut subclasses = self.subclasses.write(); @@ -605,6 +547,41 @@ impl SlotGetattro for PyType { } } +impl SlotSetattro for PyType { + fn setattro( + zelf: &PyRef, + attr_name: PyStrRef, + value: Option, + vm: &VirtualMachine, + ) -> PyResult<()> { + if let Some(attr) = zelf.get_class_attr(attr_name.borrow_value()) { + let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load()); + if let Some(descriptor) = descr_set { + return descriptor(attr, zelf.clone().into_object(), value, vm); + } + } + let assign = value.is_some(); + + let mut attributes = zelf.attributes.write(); + if let Some(value) = value { + attributes.insert(attr_name.borrow_value().to_owned(), value); + } else { + let prev_value = attributes.remove(attr_name.borrow_value()); + if prev_value.is_none() { + return Err(vm.new_exception( + vm.ctx.exceptions.attribute_error.clone(), + vec![attr_name.into_object()], + )); + } + } + let attr_name = attr_name.borrow_value(); + if attr_name.starts_with("__") && attr_name.ends_with("__") { + zelf.update_slot(attr_name, assign); + } + Ok(()) + } +} + impl Callable for PyType { fn call(zelf: &PyRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { vm_trace!("type_call: {:?}", zelf); diff --git a/vm/src/builtins/weakproxy.rs b/vm/src/builtins/weakproxy.rs index 18932628e..45737efbd 100644 --- a/vm/src/builtins/weakproxy.rs +++ b/vm/src/builtins/weakproxy.rs @@ -1,7 +1,9 @@ use super::pytype::PyTypeRef; use super::weakref::PyWeak; +use super::PyStrRef; use crate::function::OptionalArg; use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue}; +use crate::slots::SlotSetattro; use crate::vm::VirtualMachine; #[pyclass(module = false, name = "weakproxy")] @@ -16,7 +18,7 @@ impl PyValue for PyWeakProxy { } } -#[pyimpl] +#[pyimpl(with(SlotSetattro))] impl PyWeakProxy { // TODO: callbacks #[pyslot] @@ -45,11 +47,17 @@ impl PyWeakProxy { )), } } +} - #[pymethod(name = "__setattr__")] - fn setattr(&self, attr_name: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult { - match self.weak.upgrade() { - Some(obj) => vm.set_attr(&obj, attr_name, value), +impl SlotSetattro for PyWeakProxy { + fn setattro( + zelf: &PyRef, + attr_name: PyStrRef, + value: Option, + vm: &VirtualMachine, + ) -> PyResult<()> { + match zelf.weak.upgrade() { + Some(obj) => vm.call_set_attr(&obj, attr_name, value), None => Err(vm.new_exception_msg( vm.ctx.exceptions.reference_error.clone(), "weakly-referenced object no longer exists".to_owned(), diff --git a/vm/src/cformat.rs b/vm/src/cformat.rs index 6f2b8a24f..b3538e643 100644 --- a/vm/src/cformat.rs +++ b/vm/src/cformat.rs @@ -380,13 +380,16 @@ impl CFormatSpec { }; Ok(self.format_bytes(bytes)) } else { - let bytes = vm.call_method(&obj, "__bytes__", ()) - .map_err(|_| { + let bytes = vm + .get_special_method(obj, "__bytes__")? + .map_err(|obj| { vm.new_type_error(format!( - "%b requires a bytes-like object, or an object that implements __bytes__, not '{}'", + "%b requires a bytes-like object, or an object that \ + implements __bytes__, not '{}'", obj.class().name )) - })?; + })? + .invoke((), vm)?; let bytes = PyBytes::try_from_object(vm, bytes)?; Ok(self.format_bytes(bytes.borrow_value())) } diff --git a/vm/src/format.rs b/vm/src/format.rs index c13e3b968..779ddb6ad 100644 --- a/vm/src/format.rs +++ b/vm/src/format.rs @@ -1,4 +1,4 @@ -use crate::builtins::pystr; +use crate::builtins::{self, pystr}; use crate::common::float_ops; use crate::exceptions::{IntoPyException, PyBaseExceptionRef}; use crate::function::FuncArgs; @@ -891,13 +891,13 @@ fn call_object_format( format_spec: &str, ) -> PyResult { let argument = match preconversion_spec.and_then(FormatPreconversor::from_char) { - Some(FormatPreconversor::Str) => vm.call_method(&argument, "__str__", ())?, - Some(FormatPreconversor::Repr) => vm.call_method(&argument, "__repr__", ())?, - Some(FormatPreconversor::Ascii) => vm.call_method(&argument, "__repr__", ())?, + Some(FormatPreconversor::Str) => vm.to_str(&argument)?.into_object(), + Some(FormatPreconversor::Repr) => vm.to_repr(&argument)?.into_object(), + Some(FormatPreconversor::Ascii) => vm.ctx.new_str(builtins::ascii(argument, vm)?), Some(FormatPreconversor::Bytes) => vm.call_method(&argument, "decode", ())?, None => argument, }; - let result = vm.call_method(&argument, "__format__", (format_spec,))?; + let result = vm.call_special_method(argument, "__format__", (format_spec,))?; if !result.isinstance(&vm.ctx.types.str_type) { return Err(vm.new_type_error(format!( "__format__ must return a str, not {}", diff --git a/vm/src/frame.rs b/vm/src/frame.rs index f0ea038ac..d04a25a60 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -792,7 +792,7 @@ impl ExecutingFrame<'_> { let exit = vm.get_attribute(context_manager.clone(), "__exit__")?; self.push_value(exit); // Call enter: - let enter_res = vm.call_method(&context_manager, "__enter__", ())?; + let enter_res = vm.call_special_method(context_manager, "__enter__", ())?; self.push_block(BlockType::Finally { handler: *end }); self.push_value(enter_res); Ok(None) @@ -801,7 +801,7 @@ impl ExecutingFrame<'_> { let mgr = self.pop_value(); let aexit = vm.get_attribute(mgr.clone(), "__aexit__")?; self.push_value(aexit); - let aenter_res = vm.call_method(&mgr, "__aenter__", ())?; + let aenter_res = vm.call_special_method(mgr, "__aenter__", ())?; self.push_value(aenter_res); Ok(None) @@ -883,17 +883,17 @@ impl ExecutingFrame<'_> { } bytecode::Instruction::GetAIter => { let aiterable = self.pop_value(); - let aiter = vm.call_method(&aiterable, "__aiter__", ())?; + let aiter = vm.call_special_method(aiterable, "__aiter__", ())?; self.push_value(aiter); Ok(None) } bytecode::Instruction::GetANext => { let aiter = self.last_value(); - let awaitable = vm.call_method(&aiter, "__anext__", ())?; + let awaitable = vm.call_special_method(aiter, "__anext__", ())?; let awaitable = if awaitable.payload_is::() { awaitable } else { - vm.call_method(&awaitable, "__await__", ())? + vm.call_special_method(awaitable, "__await__", ())? }; self.push_value(awaitable); Ok(None) @@ -1030,7 +1030,7 @@ impl ExecutingFrame<'_> { }; let spec = self.pop_value(); - let formatted = vm.call_method(&value, "__format__", (spec,))?; + let formatted = vm.call_special_method(value, "__format__", (spec,))?; self.push_value(formatted); Ok(None) } @@ -1630,9 +1630,33 @@ impl ExecutingFrame<'_> { fn execute_unop(&mut self, vm: &VirtualMachine, op: &bytecode::UnaryOperator) -> FrameResult { let a = self.pop_value(); let value = match *op { - bytecode::UnaryOperator::Minus => vm.call_method(&a, "__neg__", ())?, - bytecode::UnaryOperator::Plus => vm.call_method(&a, "__pos__", ())?, - bytecode::UnaryOperator::Invert => vm.call_method(&a, "__invert__", ())?, + bytecode::UnaryOperator::Minus => vm + .get_special_method(a, "__neg__")? + .map_err(|a| { + vm.new_type_error(format!( + "bad operand type for unary -: '{}'", + a.class().name + )) + })? + .invoke((), vm)?, + bytecode::UnaryOperator::Plus => vm + .get_special_method(a, "__pos__")? + .map_err(|a| { + vm.new_type_error(format!( + "bad operand type for unary +: '{}'", + a.class().name + )) + })? + .invoke((), vm)?, + bytecode::UnaryOperator::Invert => vm + .get_special_method(a, "__invert__")? + .map_err(|a| { + vm.new_type_error(format!( + "bad operand type for unary ~: '{}'", + a.class().name + )) + })? + .invoke((), vm)?, bytecode::UnaryOperator::Not => { let value = pybool::boolval(vm, a)?; vm.ctx.new_bool(!value) diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 06ad503d9..937a76f58 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -643,15 +643,38 @@ where T: IntoPyObject, { fn get_item(&self, key: T, vm: &VirtualMachine) -> PyResult { - vm.call_method(self, "__getitem__", (key,)) + vm.get_special_method(self.clone(), "__getitem__")? + .map_err(|obj| { + vm.new_type_error(format!( + "'{}' object is not subscriptable", + obj.class().name + )) + })? + .invoke((key,), vm) } fn set_item(&self, key: T, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - vm.call_method(self, "__setitem__", (key, value)).map(drop) + vm.get_special_method(self.clone(), "__setitem__")? + .map_err(|obj| { + vm.new_type_error(format!( + "'{}' does not support item assignment", + obj.class().name + )) + })? + .invoke((key, value), vm)?; + Ok(()) } fn del_item(&self, key: T, vm: &VirtualMachine) -> PyResult<()> { - vm.call_method(self, "__delitem__", (key,)).map(drop) + vm.get_special_method(self.clone(), "__delitem__")? + .map_err(|obj| { + vm.new_type_error(format!( + "'{}' does not support item deletion", + obj.class().name + )) + })? + .invoke((key,), vm)?; + Ok(()) } } @@ -1281,7 +1304,10 @@ impl PyMethod { } else { let descr_get = descr_cls.mro_find_map(|cls| cls.slots.descr_get.load()); if let Some(descr_get) = descr_get { - if descr_cls.has_attr("__set__") { + if descr_cls + .mro_find_map(|cls| cls.slots.descr_set.load()) + .is_some() + { drop(descr_cls); let cls = PyLease::into_pyref(cls).into_object(); return descr_get(descr, Some(obj), Some(cls), vm).map(Self::Attribute); @@ -1320,13 +1346,42 @@ impl PyMethod { drop(cls); vm.invoke(&getter, (obj, name)).map(Self::Attribute) } else { - Err(vm.new_attribute_error(format!( - "'{}' object has not attribute '{}'", - cls.name, name - ))) + Err(vm + .new_attribute_error(format!("'{}' object has no attribute '{}'", cls.name, name))) } } + pub(crate) fn get_special( + obj: PyObjectRef, + name: &str, + vm: &VirtualMachine, + ) -> PyResult> { + let obj_cls = obj.class(); + let func = match obj_cls.get_attr(name) { + Some(f) => f, + None => { + drop(obj_cls); + return Ok(Err(obj)); + } + }; + let meth = if func + .class() + .slots + .flags + .has_feature(PyTpFlags::METHOD_DESCR) + { + drop(obj_cls); + Self::Function { target: obj, func } + } else { + let obj_cls = PyLease::into_pyref(obj_cls).into_object(); + let attr = vm + .call_get_descriptor_specific(func, Some(obj), Some(obj_cls)) + .unwrap_or_else(Ok)?; + Self::Attribute(attr) + }; + Ok(Ok(meth)) + } + pub fn invoke(self, args: impl IntoFuncArgs, vm: &VirtualMachine) -> PyResult { let (func, args) = match self { PyMethod::Function { target, func } => (func, args.into_method_args(target, vm)), diff --git a/vm/src/slots.rs b/vm/src/slots.rs index be0aa937d..1a0a66a1f 100644 --- a/vm/src/slots.rs +++ b/vm/src/slots.rs @@ -57,6 +57,8 @@ pub(crate) type CmpFunc = fn( &VirtualMachine, ) -> PyResult>; pub(crate) type GetattroFunc = fn(PyObjectRef, PyStrRef, &VirtualMachine) -> PyResult; +pub(crate) type SetattroFunc = + fn(&PyObjectRef, PyStrRef, Option, &VirtualMachine) -> PyResult<()>; pub(crate) type BufferFunc = fn(&PyObjectRef, &VirtualMachine) -> PyResult>; pub(crate) type IterFunc = fn(PyObjectRef, &VirtualMachine) -> PyResult; pub(crate) type IterNextFunc = fn(&PyObjectRef, &VirtualMachine) -> PyResult; @@ -73,6 +75,7 @@ pub struct PyTypeSlots { pub hash: AtomicCell>, pub cmp: AtomicCell>, pub getattro: AtomicCell>, + pub setattro: AtomicCell>, pub buffer: Option, pub iter: AtomicCell>, pub iternext: AtomicCell>, @@ -413,6 +416,46 @@ pub trait SlotGetattro: PyValue { Self::getattro(zelf, name, vm) } } + +#[pyimpl] +pub trait SlotSetattro: PyValue { + #[pyslot] + fn tp_setattro( + obj: &PyObjectRef, + name: PyStrRef, + value: Option, + vm: &VirtualMachine, + ) -> PyResult<()> { + if let Some(zelf) = obj.downcast_ref::() { + Self::setattro(zelf, name, value, vm) + } else { + Err(vm.new_type_error("unexpected payload for __setattr__".to_owned())) + } + } + + fn setattro( + zelf: &PyRef, + name: PyStrRef, + value: Option, + vm: &VirtualMachine, + ) -> PyResult<()>; + + #[pymethod] + fn __setattr__( + zelf: PyRef, + name: PyStrRef, + value: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult<()> { + Self::setattro(&zelf, name, Some(value), vm) + } + + #[pymethod] + fn __delattr__(zelf: PyRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> { + Self::setattro(&zelf, name, None, vm) + } +} + #[pyimpl] pub trait BufferProtocol: PyValue { #[pyslot] diff --git a/vm/src/stdlib/thread.rs b/vm/src/stdlib/thread.rs index 58da8e0d2..ccd14f386 100644 --- a/vm/src/stdlib/thread.rs +++ b/vm/src/stdlib/thread.rs @@ -9,7 +9,7 @@ use crate::pyobject::{ BorrowValue, Either, IdProtocol, ItemProtocol, PyCallable, PyClassImpl, PyObjectRef, PyRef, PyResult, PyValue, StaticType, TypeProtocol, }; -use crate::slots::SlotGetattro; +use crate::slots::{SlotGetattro, SlotSetattro}; use crate::vm::VirtualMachine; use parking_lot::{ @@ -304,7 +304,7 @@ impl PyValue for PyLocal { } } -#[pyimpl(with(SlotGetattro), flags(BASETYPE))] +#[pyimpl(with(SlotGetattro, SlotSetattro), flags(BASETYPE))] impl PyLocal { fn ldict(&self, vm: &VirtualMachine) -> PyDictRef { self.data.get_or(|| vm.ctx.new_dict()).clone() @@ -317,37 +317,6 @@ impl PyLocal { } .into_ref_with_type(vm, cls) } - - #[pymethod(magic)] - fn setattr( - zelf: PyRef, - attr: PyStrRef, - value: PyObjectRef, - vm: &VirtualMachine, - ) -> PyResult<()> { - if attr.borrow_value() == "__dict__" { - Err(vm.new_attribute_error(format!( - "{} attribute '__dict__' is read-only", - zelf.as_object() - ))) - } else { - zelf.ldict(vm).set_item(attr.into_object(), value, vm)?; - Ok(()) - } - } - - #[pymethod(magic)] - fn delattr(zelf: PyRef, attr: PyStrRef, vm: &VirtualMachine) -> PyResult<()> { - if attr.borrow_value() == "__dict__" { - Err(vm.new_attribute_error(format!( - "{} attribute '__dict__' is read-only", - zelf.as_object() - ))) - } else { - zelf.ldict(vm).del_item(attr.into_object(), vm)?; - Ok(()) - } - } } impl SlotGetattro for PyLocal { @@ -365,6 +334,30 @@ impl SlotGetattro for PyLocal { } } +impl SlotSetattro for PyLocal { + fn setattro( + zelf: &PyRef, + attr: PyStrRef, + value: Option, + vm: &VirtualMachine, + ) -> PyResult<()> { + if attr.borrow_value() == "__dict__" { + Err(vm.new_attribute_error(format!( + "{} attribute '__dict__' is read-only", + zelf.as_object() + ))) + } else { + let dict = zelf.ldict(vm); + if let Some(value) = value { + dict.set_item(attr, value, vm)?; + } else { + dict.del_item(attr, vm)?; + } + Ok(()) + } + } +} + pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { let ctx = &vm.ctx; diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 6743f4872..4e311ce27 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -817,7 +817,7 @@ impl VirtualMachine { if obj.class().is(&self.ctx.types.str_type) { Ok(obj.clone().downcast().unwrap()) } else { - let s = self.call_method(&obj, "__str__", ())?; + let s = self.call_special_method(obj.clone(), "__str__", ())?; PyStrRef::try_from_object(self, s) } } @@ -828,8 +828,8 @@ impl VirtualMachine { } pub fn to_repr(&self, obj: &PyObjectRef) -> PyResult { - let repr = self.call_method(obj, "__repr__", ())?; - TryFromObject::try_from_object(self, repr) + let repr = self.call_special_method(obj.clone(), "__repr__", ())?; + PyStrRef::try_from_object(self, repr) } pub fn to_index_opt(&self, obj: PyObjectRef) -> Option> { @@ -927,7 +927,11 @@ impl VirtualMachine { if obj.class().is(cls) { Ok(true) } else { - let ret = self.call_method(cls.as_object(), "__instancecheck__", (obj.clone(),))?; + let ret = self.call_special_method( + cls.as_object().clone(), + "__instancecheck__", + (obj.clone(),), + )?; pybool::boolval(self, ret) } } @@ -935,7 +939,11 @@ impl VirtualMachine { /// Determines if `subclass` is a subclass of `cls`, either directly, indirectly or virtually /// via the __subclasscheck__ magic method. pub fn issubclass(&self, subclass: &PyTypeRef, cls: &PyTypeRef) -> PyResult { - let ret = self.call_method(cls.as_object(), "__subclasscheck__", (subclass.clone(),))?; + let ret = self.call_special_method( + cls.as_object().clone(), + "__subclasscheck__", + (subclass.clone(),), + )?; pybool::boolval(self, ret) } @@ -976,6 +984,26 @@ impl VirtualMachine { .invoke(args, self) } + #[inline] + pub(crate) fn get_special_method( + &self, + obj: PyObjectRef, + method: &str, + ) -> PyResult> { + PyMethod::get_special(obj, method, self) + } + + pub(crate) fn call_special_method( + &self, + obj: PyObjectRef, + method: &str, + args: impl IntoFuncArgs, + ) -> PyResult { + self.get_special_method(obj, method)? + .map_err(|_obj| self.new_attribute_error(method.to_owned()))? + .invoke(args, self) + } + fn _invoke(&self, callable: &PyObjectRef, args: FuncArgs) -> PyResult { vm_trace!("Invoke: {:?} {:?}", callable, args); let slot_call = callable.class().mro_find_map(|cls| cls.slots.call.load()); @@ -1145,18 +1173,42 @@ impl VirtualMachine { } } - pub fn set_attr(&self, obj: &PyObjectRef, attr_name: K, attr_value: V) -> PyResult + pub fn call_set_attr( + &self, + obj: &PyObjectRef, + attr_name: PyStrRef, + attr_value: Option, + ) -> PyResult<()> { + let setattro = { + let cls = obj.class(); + cls.mro_find_map(|cls| cls.slots.setattro.load()) + .ok_or_else(|| { + let assign = attr_value.is_some(); + let has_getattr = cls.mro_find_map(|cls| cls.slots.getattro.load()).is_some(); + self.new_type_error(format!( + "'{}' object has {} attributes ({} {})", + cls.name, + if has_getattr { "only read-only" } else { "no" }, + if assign { "assign to" } else { "del" }, + attr_name + )) + })? + }; + setattro(obj, attr_name, attr_value, self) + } + + pub fn set_attr(&self, obj: &PyObjectRef, attr_name: K, attr_value: V) -> PyResult<()> where K: TryIntoRef, V: Into, { let attr_name = attr_name.try_into_ref(self)?; - self.call_method(obj, "__setattr__", (attr_name, attr_value.into())) + self.call_set_attr(obj, attr_name, Some(attr_value.into())) } - pub fn del_attr(&self, obj: &PyObjectRef, attr_name: PyObjectRef) -> PyResult<()> { - self.call_method(&obj, "__delattr__", (attr_name,))?; - Ok(()) + pub fn del_attr(&self, obj: &PyObjectRef, attr_name: impl TryIntoRef) -> PyResult<()> { + let attr_name = attr_name.try_into_ref(self)?; + self.call_set_attr(obj, attr_name, None) } // get_method should be used for internal access to magic methods (by-passing @@ -1693,11 +1745,9 @@ impl VirtualMachine { } pub fn _membership(&self, haystack: PyObjectRef, needle: PyObjectRef) -> PyResult { - if let Some(method_or_err) = self.get_method(haystack.clone(), "__contains__") { - let method = method_or_err?; - self.invoke(&method, vec![needle]) - } else { - self._membership_iter_search(haystack, needle) + match PyMethod::get_special(haystack, "__contains__", self)? { + Ok(method) => method.invoke((needle,), self), + Err(haystack) => self._membership_iter_search(haystack, needle), } } @@ -1812,7 +1862,7 @@ impl VirtualMachine { attr_value: impl Into, ) -> PyResult<()> { let val = attr_value.into(); - object::setattr(module.clone(), attr_name.try_into_ref(self)?, val, self) + object::setattr(module, attr_name.try_into_ref(self)?, Some(val), self) } } From 7d813761c58df2ad9c53b14e50ba548bfacc5d92 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Sun, 14 Mar 2021 18:35:28 -0500 Subject: [PATCH 5/7] Move io.rs over to vm.call_method --- vm/src/stdlib/io.rs | 95 +++++++++++++++++++-------------------------- 1 file changed, 39 insertions(+), 56 deletions(-) diff --git a/vm/src/stdlib/io.rs b/vm/src/stdlib/io.rs index 7f13ce168..c33c3c882 100644 --- a/vm/src/stdlib/io.rs +++ b/vm/src/stdlib/io.rs @@ -259,7 +259,7 @@ mod _io { } fn check_readable(file: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - if pybool::boolval(vm, call_method(vm, file, "readable", ())?)? { + if pybool::boolval(vm, vm.call_method(file, "readable", ())?)? { Ok(()) } else { Err(new_unsupported_operation( @@ -270,7 +270,7 @@ mod _io { } fn check_writable(file: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - if pybool::boolval(vm, call_method(vm, file, "writable", ())?)? { + if pybool::boolval(vm, vm.call_method(file, "writable", ())?)? { Ok(()) } else { Err(new_unsupported_operation( @@ -281,7 +281,7 @@ mod _io { } fn check_seekable(file: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - if pybool::boolval(vm, call_method(vm, file, "seekable", ())?)? { + if pybool::boolval(vm, vm.call_method(file, "seekable", ())?)? { Ok(()) } else { Err(new_unsupported_operation( @@ -308,7 +308,7 @@ mod _io { } #[pymethod] fn tell(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult { - call_method(vm, &zelf, "seek", (0, 1)) + vm.call_method(&zelf, "seek", (0, 1)) } #[pymethod] fn truncate(zelf: PyObjectRef, _pos: OptionalArg, vm: &VirtualMachine) -> PyResult { @@ -328,7 +328,7 @@ mod _io { #[pyslot] fn tp_del(instance: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - let _ = call_method(vm, instance, "close", ()); + let _ = vm.call_method(instance, "close", ()); Ok(()) } @@ -339,7 +339,7 @@ mod _io { #[pymethod(magic)] fn exit(instance: PyObjectRef, _args: FuncArgs, vm: &VirtualMachine) -> PyResult<()> { - call_method(vm, &instance, "close", ())?; + vm.call_method(&instance, "close", ())?; Ok(()) } @@ -428,7 +428,7 @@ mod _io { ) -> PyResult<()> { check_closed(&instance, vm)?; for line in lines.iter(vm)? { - call_method(vm, &instance, "write", (line?,))?; + vm.call_method(&instance, "write", (line?,))?; } Ok(()) } @@ -461,7 +461,7 @@ mod _io { } #[pyslot] fn tp_iternext(instance: &PyObjectRef, vm: &VirtualMachine) -> PyResult { - let line = call_method(vm, &instance, "readline", ())?; + let line = vm.call_method(&instance, "readline", ())?; if !pybool::boolval(vm, line.clone())? { Err(vm.new_stop_iteration()) } else { @@ -476,7 +476,7 @@ mod _io { pub(super) fn iobase_close(file: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { if !file_closed(file, vm)? { - let res = call_method(vm, file, "flush", ()); + let res = vm.call_method(file, "flush", ()); vm.set_attr(file, "__closed", vm.ctx.new_bool(true))?; res?; } @@ -496,7 +496,7 @@ mod _io { let b = PyByteArray::from(vec![0; size]).into_ref(vm); let n = >::try_from_object( vm, - call_method(vm, &instance, "readinto", (b.clone(),))?, + vm.call_method(&instance, "readinto", (b.clone(),))?, )?; Ok(n.map(|n| { let bytes = &mut b.borrow_value_mut().elements; @@ -505,7 +505,7 @@ mod _io { }) .into_pyobject(vm)) } else { - call_method(vm, &instance, "readall", ()) + vm.call_method(&instance, "readall", ()) } } @@ -514,7 +514,7 @@ mod _io { let mut chunks = Vec::new(); let mut total_len = 0; loop { - let data = call_method(vm, &instance, "read", (DEFAULT_BUFFER_SIZE,))?; + let data = vm.call_method(&instance, "read", (DEFAULT_BUFFER_SIZE,))?; let data = >::try_from_object(vm, data)?; match data { None => { @@ -562,7 +562,7 @@ mod _io { ) -> PyResult { let b = PyRwBytesLike::new(vm, &bufobj)?; let l = b.len(); - let data = call_method(vm, &zelf, method, (l,))?; + let data = vm.call_method(&zelf, method, (l,))?; if data.is(&bufobj) { return Ok(l); } @@ -732,7 +732,7 @@ mod _io { } fn raw_seek(&mut self, pos: Offset, whence: i32, vm: &VirtualMachine) -> PyResult { - let ret = call_method(vm, self.check_init(vm)?, "seek", (pos, whence))?; + let ret = vm.call_method(self.check_init(vm)?, "seek", (pos, whence))?; let offset = get_offset(ret, vm)?; if offset < 0 { return Err( @@ -777,7 +777,7 @@ mod _io { } fn raw_tell(&mut self, vm: &VirtualMachine) -> PyResult { - let ret = call_method(vm, self.check_init(vm)?, "tell", ())?; + let ret = vm.call_method(self.check_init(vm)?, "tell", ())?; let offset = get_offset(ret, vm)?; if offset < 0 { return Err( @@ -809,7 +809,7 @@ mod _io { .into_pyobject(vm); // TODO: loop if write() raises an interrupt - call_method(vm, self.raw.as_ref().unwrap(), "write", (memobj,))? + vm.call_method(self.raw.as_ref().unwrap(), "write", (memobj,))? } else { let options = BufferOptions { len, @@ -827,7 +827,7 @@ mod _io { .into_ref(vm); // TODO: loop if write() raises an interrupt - let res = call_method(vm, self.raw.as_ref().unwrap(), "write", (memobj.clone(),)); + let res = vm.call_method(self.raw.as_ref().unwrap(), "write", (memobj.clone(),)); memobj.released.store(true); self.buffer = std::mem::take(&mut writebuf.data.lock()); @@ -1064,12 +1064,8 @@ mod _io { .into_ref(vm); // TODO: loop if readinto() raises an interrupt - let res = call_method( - vm, - self.raw.as_ref().unwrap(), - "readinto", - (memobj.clone(),), - ); + let res = + vm.call_method(self.raw.as_ref().unwrap(), "readinto", (memobj.clone(),)); memobj.released.store(true); std::mem::swap(v, &mut readbuf.data.lock()); @@ -1080,7 +1076,7 @@ mod _io { let memobj = PyMemoryView::from_buffer_range(vm.ctx.none(), buf, buf_range, vm)?; // TODO: loop if readinto() raises an interrupt - call_method(vm, self.raw.as_ref().unwrap(), "readinto", (memobj,))? + vm.call_method(self.raw.as_ref().unwrap(), "readinto", (memobj,))? } }; @@ -1135,7 +1131,7 @@ mod _io { let mut read_size = 0; loop { - let read_data = call_method(vm, self.raw.as_ref().unwrap(), "read", ())?; + let read_data = vm.call_method(self.raw.as_ref().unwrap(), "read", ())?; let read_data = >::try_from_object(vm, read_data)?; match read_data { @@ -1388,13 +1384,13 @@ mod _io { if data.writable() { data.flush_rewind(vm)?; } - let res = call_method(vm, data.raw.as_ref().unwrap(), "truncate", (pos,))?; + let res = vm.call_method(data.raw.as_ref().unwrap(), "truncate", (pos,))?; let _ = data.raw_tell(vm); Ok(res) } #[pymethod] fn detach(zelf: PyRef, vm: &VirtualMachine) -> PyResult { - call_method(vm, zelf.as_object(), "flush", ())?; + vm.call_method(zelf.as_object(), "flush", ())?; let mut data = zelf.lock(vm)?; data.flags.insert(BufferedFlags::DETACHED); data.raw @@ -1403,7 +1399,7 @@ mod _io { } #[pymethod] fn seekable(&self, vm: &VirtualMachine) -> PyResult { - call_method(vm, self.lock(vm)?.check_init(vm)?, "seekable", ()) + vm.call_method(self.lock(vm)?.check_init(vm)?, "seekable", ()) } #[pyproperty] fn raw(&self, vm: &VirtualMachine) -> PyResult> { @@ -1423,11 +1419,11 @@ mod _io { } #[pymethod] fn fileno(&self, vm: &VirtualMachine) -> PyResult { - call_method(vm, self.lock(vm)?.check_init(vm)?, "fileno", ()) + vm.call_method(self.lock(vm)?.check_init(vm)?, "fileno", ()) } #[pymethod] fn isatty(&self, vm: &VirtualMachine) -> PyResult { - call_method(vm, self.lock(vm)?.check_init(vm)?, "isatty", ()) + vm.call_method(self.lock(vm)?.check_init(vm)?, "isatty", ()) } #[pymethod(magic)] @@ -1464,7 +1460,7 @@ mod _io { return Ok(vm.ctx.none()); } let flush_res = data.flush(vm); - let close_res = call_method(vm, data.raw.as_ref().unwrap(), "close", ()); + let close_res = vm.call_method(data.raw.as_ref().unwrap(), "close", ()); exceptions::chain(flush_res, close_res) } @@ -1477,10 +1473,10 @@ mod _io { return Ok(vm.ctx.none()); } } - let flush_res = call_method(vm, zelf.as_object(), "flush", ()).map(drop); + let flush_res = vm.call_method(zelf.as_object(), "flush", ()).map(drop); let data = zelf.lock(vm)?; let raw = data.raw.as_ref().unwrap(); - let close_res = call_method(vm, raw, "close", ()); + let close_res = vm.call_method(raw, "close", ()); exceptions::chain(flush_res, close_res) } @@ -1500,19 +1496,6 @@ mod _io { } } - // vm.call_method() only calls class attributes - // TODO: have this be the implementation of vm.call_method() once the current implementation isn't needed - // anymore because of slots - pub fn call_method( - vm: &VirtualMachine, - obj: &PyObjectRef, - name: impl crate::pyobject::TryIntoRef, - args: impl crate::function::IntoFuncArgs, - ) -> PyResult { - let meth = vm.get_attribute(obj.clone(), name)?; - vm.invoke(&meth, args) - } - #[pyimpl] trait BufferedReadable: PyValue { type Reader: BufferedMixin; @@ -1921,13 +1904,13 @@ mod _io { "can't do nonzero end-relative seeks".to_owned(), )); } - call_method(vm, &buffer, "seek", (offset, how)) + vm.call_method(&buffer, "seek", (offset, how)) } #[pymethod] fn tell(&self, vm: &VirtualMachine) -> PyResult { let buffer = self.lock(vm)?.buffer.clone(); - call_method(vm, &buffer, "tell", ()) + vm.call_method(&buffer, "tell", ()) } #[pyproperty] @@ -1947,7 +1930,7 @@ mod _io { #[pymethod] fn fileno(&self, vm: &VirtualMachine) -> PyResult { let buffer = self.lock(vm)?.buffer.clone(); - call_method(vm, &buffer, "fileno", ()) + vm.call_method(&buffer, "fileno", ()) } #[pymethod] @@ -1955,7 +1938,7 @@ mod _io { let buffer = self.lock(vm)?.buffer.clone(); check_readable(&buffer, vm)?; - let bytes = call_method(vm, &buffer, "read", (size.flatten(),))?; + let bytes = vm.call_method(&buffer, "read", (size.flatten(),))?; let bytes = PyBytesLike::try_from_object(vm, bytes)?; //format bytes into string let rust_string = String::from_utf8(bytes.to_cow().into_owned()).map_err(|e| { @@ -1976,9 +1959,9 @@ mod _io { let bytes = obj.borrow_value().as_bytes(); - let len = call_method(vm, &buffer, "write", (bytes.to_owned(),)); + let len = vm.call_method(&buffer, "write", (bytes.to_owned(),)); if obj.borrow_value().contains('\n') { - let _ = call_method(vm, &buffer, "flush", ()); + let _ = vm.call_method(&buffer, "flush", ()); } let len = usize::try_from_object(vm, len?)?; @@ -1994,14 +1977,14 @@ mod _io { fn flush(&self, vm: &VirtualMachine) -> PyResult { let buffer = self.lock(vm)?.buffer.clone(); check_closed(&buffer, vm)?; - call_method(vm, &buffer, "flush", ()) + vm.call_method(&buffer, "flush", ()) } #[pymethod] fn isatty(&self, vm: &VirtualMachine) -> PyResult { let buffer = self.lock(vm)?.buffer.clone(); check_closed(&buffer, vm)?; - call_method(vm, &buffer, "isatty", ()) + vm.call_method(&buffer, "isatty", ()) } #[pymethod] @@ -2013,7 +1996,7 @@ mod _io { let buffer = self.lock(vm)?.buffer.clone(); check_readable(&buffer, vm)?; - let bytes = call_method(vm, &buffer, "readline", (size.flatten(),))?; + let bytes = vm.call_method(&buffer, "readline", (size.flatten(),))?; let bytes = PyBytesLike::try_from_object(vm, bytes)?; //format bytes into string let rust_string = String::from_utf8(bytes.borrow_value().to_vec()).map_err(|e| { @@ -2028,7 +2011,7 @@ mod _io { #[pymethod] fn close(&self, vm: &VirtualMachine) -> PyResult { let buffer = self.lock(vm)?.buffer.clone(); - call_method(vm, &buffer, "close", ()) + vm.call_method(&buffer, "close", ()) } #[pyproperty] fn closed(&self, vm: &VirtualMachine) -> PyResult { From 324462e3f507e90ac6b0ccb6bfb7942cb1da996e Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Sun, 14 Mar 2021 18:43:55 -0500 Subject: [PATCH 6/7] Make get_method crate-private --- vm/src/vm.rs | 47 ++++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 4e311ce27..9e8467889 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -1228,8 +1228,8 @@ impl VirtualMachine { } } - /// May return exception, if `__get__` descriptor raises one - pub fn get_method(&self, obj: PyObjectRef, method_name: &str) -> Option { + // TODO: remove + transfer over to get_special_method + pub(crate) fn get_method(&self, obj: PyObjectRef, method_name: &str) -> Option { let method = obj.get_class_attr(method_name)?; Some(self.call_if_get_descriptor(method, obj)) } @@ -1698,25 +1698,30 @@ impl VirtualMachine { } pub fn obj_len_opt(&self, obj: &PyObjectRef) -> Option> { - self.get_method(obj.clone(), "__len__").map(|len| { - let len = self.invoke(&len?, ())?; - let len = len - .payload_if_subclass::(self) - .ok_or_else(|| { - self.new_type_error(format!( - "'{}' object cannot be interpreted as an integer", - len.class().name - )) - })? - .borrow_value(); - if len.is_negative() { - return Err(self.new_value_error("__len__() should return >= 0".to_owned())); - } - let len = len.to_isize().ok_or_else(|| { - self.new_overflow_error("cannot fit 'int' into an index-sized integer".to_owned()) - })?; - Ok(len as usize) - }) + self.get_special_method(obj.clone(), "__len__") + .map(Result::ok) + .transpose() + .map(|meth| { + let len = meth?.invoke((), self)?; + let len = len + .payload_if_subclass::(self) + .ok_or_else(|| { + self.new_type_error(format!( + "'{}' object cannot be interpreted as an integer", + len.class().name + )) + })? + .borrow_value(); + if len.is_negative() { + return Err(self.new_value_error("__len__() should return >= 0".to_owned())); + } + let len = len.to_isize().ok_or_else(|| { + self.new_overflow_error( + "cannot fit 'int' into an index-sized integer".to_owned(), + ) + })?; + Ok(len as usize) + }) } pub fn obj_len(&self, obj: &PyObjectRef) -> PyResult { From 54ab9cb54a0a9a637e1013a405dccdd8c52442a1 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Mon, 15 Mar 2021 09:21:20 -0500 Subject: [PATCH 7/7] Add LoadMethod and CallMethod* opcodes --- bytecode/src/lib.rs | 52 ++++++++++++++------ compiler/src/compile.rs | 98 ++++++++++++++++++++++++++++--------- compiler/src/ir.rs | 22 +++++---- vm/src/frame.rs | 106 ++++++++++++++++++++++++++++------------ vm/src/pyobjectrc.rs | 7 ++- 5 files changed, 205 insertions(+), 80 deletions(-) diff --git a/bytecode/src/lib.rs b/bytecode/src/lib.rs index 73e2e173d..111480054 100644 --- a/bytecode/src/lib.rs +++ b/bytecode/src/lib.rs @@ -247,9 +247,7 @@ pub enum Instruction { Continue { target: Label, }, - Break { - target: Label, - }, + Break, Jump { target: Label, }, @@ -281,6 +279,18 @@ pub enum Instruction { CallFunctionEx { has_kwargs: bool, }, + LoadMethod { + idx: NameIdx, + }, + CallMethodPositional { + nargs: u32, + }, + CallMethodKeyword { + nargs: u32, + }, + CallMethodEx { + has_kwargs: bool, + }, ForIter { target: Label, }, @@ -288,7 +298,9 @@ pub enum Instruction { YieldValue, YieldFrom, SetupAnnotation, - SetupLoop, + SetupLoop { + break_target: Label, + }, /// Setup a finally handler, which will be called whenever one of this events occurs: /// - the block is popped @@ -824,7 +836,7 @@ impl Instruction { | SetupExcept { handler: l } | SetupWith { end: l } | SetupAsyncWith { end: l } - | Break { target: l } + | SetupLoop { break_target: l } | Continue { target: l } => Some(l), _ => None, } @@ -844,7 +856,7 @@ impl Instruction { | SetupExcept { handler: l } | SetupWith { end: l } | SetupAsyncWith { end: l } - | Break { target: l } + | SetupLoop { break_target: l } | Continue { target: l } => Some(l), _ => None, } @@ -853,7 +865,7 @@ impl Instruction { pub fn unconditional_branch(&self) -> bool { matches!( self, - Jump { .. } | Continue { .. } | Break { .. } | ReturnValue | Raise { .. } + Jump { .. } | Continue { .. } | Break | ReturnValue | Raise { .. } ) } @@ -880,7 +892,7 @@ impl Instruction { Duplicate => 1, GetIter => 0, Continue { .. } => 0, - Break { .. } => 0, + Break => 0, Jump { .. } => 0, JumpIfTrue { .. } | JumpIfFalse { .. } => -1, JumpIfTrueOrPop { .. } | JumpIfFalseOrPop { .. } => { @@ -897,9 +909,13 @@ impl Instruction { - flags.contains(MakeFunctionFlags::DEFAULTS) as i32 + 1 } - CallFunctionPositional { nargs } => -1 - (*nargs as i32) + 1, - CallFunctionKeyword { nargs } => -2 - (*nargs as i32) + 1, - CallFunctionEx { has_kwargs } => -2 - *has_kwargs as i32 + 1, + CallFunctionPositional { nargs } => -(*nargs as i32) - 1 + 1, + CallMethodPositional { nargs } => -(*nargs as i32) - 3 + 1, + CallFunctionKeyword { nargs } => -1 - (*nargs as i32) - 1 + 1, + CallMethodKeyword { nargs } => -1 - (*nargs as i32) - 3 + 1, + CallFunctionEx { has_kwargs } => -1 - (*has_kwargs as i32) - 1 + 1, + CallMethodEx { has_kwargs } => -1 - (*has_kwargs as i32) - 3 + 1, + LoadMethod { .. } => -1 + 3, ForIter { .. } => { if jump { -1 @@ -910,7 +926,11 @@ impl Instruction { ReturnValue => -1, YieldValue => 0, YieldFrom => -1, - SetupAnnotation | SetupLoop | SetupFinally { .. } | EnterFinally | EndFinally => 0, + SetupAnnotation + | SetupLoop { .. } + | SetupFinally { .. } + | EnterFinally + | EndFinally => 0, SetupExcept { .. } => { if jump { 1 @@ -1056,7 +1076,7 @@ impl Instruction { Duplicate => w!(Duplicate), GetIter => w!(GetIter), Continue { target } => w!(Continue, target), - Break { target } => w!(Break, target), + Break => w!(Break), Jump { target } => w!(Jump, target), JumpIfTrue { target } => w!(JumpIfTrue, target), JumpIfFalse { target } => w!(JumpIfFalse, target), @@ -1066,12 +1086,16 @@ impl Instruction { CallFunctionPositional { nargs } => w!(CallFunctionPositional, nargs), CallFunctionKeyword { nargs } => w!(CallFunctionKeyword, nargs), CallFunctionEx { has_kwargs } => w!(CallFunctionEx, has_kwargs), + LoadMethod { idx } => w!(LoadMethod, name(*idx)), + CallMethodPositional { nargs } => w!(CallMethodPositional, nargs), + CallMethodKeyword { nargs } => w!(CallMethodKeyword, nargs), + CallMethodEx { has_kwargs } => w!(CallMethodEx, has_kwargs), ForIter { target } => w!(ForIter, target), ReturnValue => w!(ReturnValue), YieldValue => w!(YieldValue), YieldFrom => w!(YieldFrom), SetupAnnotation => w!(SetupAnnotation), - SetupLoop => w!(SetupLoop), + SetupLoop { break_target } => w!(SetupLoop, break_target), SetupExcept { handler } => w!(SetupExcept, handler), SetupFinally { handler } => w!(SetupFinally, handler), EnterFinally => w!(EnterFinally), diff --git a/compiler/src/compile.rs b/compiler/src/compile.rs index 3dfd7731a..8b2c7cf3c 100644 --- a/compiler/src/compile.rs +++ b/compiler/src/compile.rs @@ -28,6 +28,28 @@ enum NameUsage { Delete, } +enum CallType { + Positional { nargs: u32 }, + Keyword { nargs: u32 }, + Ex { has_kwargs: bool }, +} +impl CallType { + fn normal_call(self) -> Instruction { + match self { + CallType::Positional { nargs } => Instruction::CallFunctionPositional { nargs }, + CallType::Keyword { nargs } => Instruction::CallFunctionKeyword { nargs }, + CallType::Ex { has_kwargs } => Instruction::CallFunctionEx { has_kwargs }, + } + } + fn method_call(self) -> Instruction { + match self { + CallType::Positional { nargs } => Instruction::CallMethodPositional { nargs }, + CallType::Keyword { nargs } => Instruction::CallMethodKeyword { nargs }, + CallType::Ex { has_kwargs } => Instruction::CallMethodEx { has_kwargs }, + } + } +} + /// Main structure holding the state of compilation. struct Compiler { code_stack: Vec, @@ -667,14 +689,13 @@ impl Compiler { self.switch_to_block(after_block); } } - Break => match self.ctx.loop_data { - Some((_, end)) => { - self.emit(Instruction::Break { target: end }); + Break => { + if self.ctx.loop_data.is_some() { + self.emit(Instruction::Break); + } else { + return Err(self.error_loc(CompileErrorType::InvalidBreak, statement.location)); } - None => { - return Err(self.error_loc(CompileErrorType::InvalidBreak, statement.location)) - } - }, + } Continue => match self.ctx.loop_data { Some((start, _)) => { self.emit(Instruction::Continue { target: start }); @@ -1242,7 +1263,8 @@ impl Compiler { value: qualified_name, }); - self.compile_call(2, bases, keywords)?; + let call = self.compile_call_inner(2, bases, keywords)?; + self.emit(call.normal_call()); self.apply_decorators(decorator_list); @@ -1271,7 +1293,9 @@ impl Compiler { let else_block = self.new_block(); let after_block = self.new_block(); - self.emit(Instruction::SetupLoop); + self.emit(Instruction::SetupLoop { + break_target: after_block, + }); self.switch_to_block(while_block); self.compile_jump_if(test, false, else_block)?; @@ -1360,7 +1384,9 @@ impl Compiler { let else_block = self.new_block(); let after_block = self.new_block(); - self.emit(Instruction::SetupLoop); + self.emit(Instruction::SetupLoop { + break_target: after_block, + }); // The thing iterated: self.compile_expression(iter)?; @@ -1794,10 +1820,7 @@ impl Compiler { func, args, keywords, - } => { - self.compile_expression(func)?; - self.compile_call(0, args, keywords)?; - } + } => self.compile_call(func, args, keywords)?, BoolOp { op, values } => self.compile_bool_op(op, values)?, BinOp { left, op, right } => { self.compile_expression(left)?; @@ -2103,17 +2126,41 @@ impl Compiler { fn compile_call( &mut self, - additional_positional: u32, + func: &ast::Expr, args: &[ast::Expr], keywords: &[ast::Keyword], ) -> CompileResult<()> { + let method = if let ast::ExprKind::Attribute { value, attr, .. } = &func.node { + self.compile_expression(value)?; + let idx = self.name(attr); + self.emit(Instruction::LoadMethod { idx }); + true + } else { + self.compile_expression(func)?; + false + }; + let call = self.compile_call_inner(0, args, keywords)?; + self.emit(if method { + call.method_call() + } else { + call.normal_call() + }); + Ok(()) + } + + fn compile_call_inner( + &mut self, + additional_positional: u32, + args: &[ast::Expr], + keywords: &[ast::Keyword], + ) -> CompileResult { let count = (args.len() + keywords.len()) as u32 + additional_positional; // Normal arguments: let (size, unpack) = self.gather_elements(additional_positional, args)?; let has_double_star = keywords.iter().any(|k| k.node.arg.is_none()); - if unpack || has_double_star { + let call = if unpack || has_double_star { // Create a tuple with positional args: self.emit(Instruction::BuildTuple { size, unpack }); @@ -2122,7 +2169,7 @@ impl Compiler { if has_kwargs { self.compile_keywords(keywords)?; } - self.emit(Instruction::CallFunctionEx { has_kwargs }); + CallType::Ex { has_kwargs } } else if !keywords.is_empty() { let mut kwarg_names = vec![]; for keyword in keywords { @@ -2140,12 +2187,12 @@ impl Compiler { self.emit_constant(ConstantData::Tuple { elements: kwarg_names, }); - self.emit(Instruction::CallFunctionKeyword { nargs: count }); + CallType::Keyword { nargs: count } } else { - self.emit(Instruction::CallFunctionPositional { nargs: count }); - } + CallType::Positional { nargs: count } + }; - Ok(()) + Ok(call) } // Given a vector of expr / star expr generate code which gives either @@ -2262,8 +2309,13 @@ impl Compiler { unimplemented!("async for comprehensions"); } + let loop_block = self.new_block(); + let after_block = self.new_block(); + // Setup for loop: - self.emit(Instruction::SetupLoop); + self.emit(Instruction::SetupLoop { + break_target: after_block, + }); if loop_labels.is_empty() { // Load iterator onto stack (passed as first argument): @@ -2276,8 +2328,6 @@ impl Compiler { self.emit(Instruction::GetIter); } - let loop_block = self.new_block(); - let after_block = self.new_block(); loop_labels.push((loop_block, after_block)); self.switch_to_block(loop_block); diff --git a/compiler/src/ir.rs b/compiler/src/ir.rs index d2926bf6e..3c7ba9ffe 100644 --- a/compiler/src/ir.rs +++ b/compiler/src/ir.rs @@ -167,25 +167,29 @@ impl CodeInfo { let mut startdepths = vec![u32::MAX; self.blocks.len()]; startdepths[0] = 0; stack.push(Label(0)); + let debug = false; 'process_blocks: while let Some(block) = stack.pop() { let mut depth = startdepths[block.0 as usize]; + if debug { + eprintln!("===BLOCK {}===", block.0); + } let block = &self.blocks[block.0 as usize]; for i in &block.instructions { let instr = &i.instr; let effect = instr.stack_effect(false); let new_depth = add_ui(depth, effect); + if debug { + eprintln!("{:?}: {:+}, {:+} = {}", instr, effect, depth, new_depth); + } if new_depth > maxdepth { maxdepth = new_depth } - // we don't want to worry about Continue or Break, they use unwinding to jump to - // their targets and as such the stack size is taken care of in frame.rs by setting + // we don't want to worry about Continue, it uses unwinding to jump to + // its targets and as such the stack size is taken care of in frame.rs by setting // it back to the level it was at when SetupLoop was run - let jump_label = instr.label_arg().filter(|_| { - !matches!( - instr, - Instruction::Continue { .. } | Instruction::Break { .. } - ) - }); + let jump_label = instr + .label_arg() + .filter(|_| !matches!(instr, Instruction::Continue { .. })); if let Some(&target_block) = jump_label { let effect = instr.stack_effect(true); let target_depth = add_ui(depth, effect); @@ -215,7 +219,7 @@ fn stackdepth_push(stack: &mut Vec