From 5abc674737c6fb154a7cdb3623ef37d4c1af5240 Mon Sep 17 00:00:00 2001 From: ben Date: Sat, 9 Mar 2019 12:11:07 +1300 Subject: [PATCH 1/4] Change property implementation to have rust payload and to use new function style. --- vm/src/obj/objproperty.rs | 196 +++++++++++++++++++++++++++++--------- vm/src/pyobject.rs | 17 ++++ vm/src/vm.rs | 5 + 3 files changed, 172 insertions(+), 46 deletions(-) diff --git a/vm/src/obj/objproperty.rs b/vm/src/obj/objproperty.rs index 0a952375d6..e1e6bfa3c6 100644 --- a/vm/src/obj/objproperty.rs +++ b/vm/src/obj/objproperty.rs @@ -2,11 +2,151 @@ */ -use crate::pyobject::{PyContext, PyFuncArgs, PyResult, TypeProtocol}; -use crate::vm::VirtualMachine; +use crate::pyobject::{PyContext, PyFuncArgs, PyObject, PyObjectRef, PyObjectPayload, PyObjectPayload2, PyResult, TypeProtocol}; +use crate::pyobject::IntoPyNativeFunc; +use crate::obj::objnone::PyNone; +use crate::VirtualMachine; +use crate::function::PyRef; +use std::marker::PhantomData; + +/// Read-only property, doesn't have __set__ or __delete__ +#[derive(Debug)] +pub struct PyReadOnlyProperty { + getter: PyObjectRef +} + +impl PyObjectPayload2 for PyReadOnlyProperty { + fn required_type(ctx: &PyContext) -> PyObjectRef { + ctx.readonly_property_type() + } +} + +pub type PyReadOnlyPropertyRef = PyRef; + +impl PyReadOnlyPropertyRef { + fn get(self, obj: PyObjectRef, _owner: PyObjectRef, vm: &mut VirtualMachine) -> PyResult { + vm.invoke(self.getter.clone(), obj) + } +} + +/// Fully fledged property +#[derive(Debug)] +pub struct PyProperty { + getter: Option, + setter: Option, + deleter: Option +} + +impl PyObjectPayload2 for PyProperty { + fn required_type(ctx: &PyContext) -> PyObjectRef { + ctx.property_type() + } +} + +pub type PyPropertyRef = PyRef; + +impl PyPropertyRef { + fn get(self, obj: PyObjectRef, _owner: PyObjectRef, vm: &mut VirtualMachine) -> PyResult { + if let Some(getter) = self.getter.as_ref() { + vm.invoke(getter.clone(), obj) + } else { + Err(vm.new_attribute_error("unreadable attribute".to_string())) + } + } + + fn set(self, obj: PyObjectRef, value: PyObjectRef, vm: &mut VirtualMachine) -> PyResult { + if let Some(setter) = self.setter.as_ref() { + vm.invoke(setter.clone(), vec![obj, value]) + } else { + Err(vm.new_attribute_error("can't set attribute".to_string())) + } + } + + fn delete(self, obj: PyObjectRef, vm: &mut VirtualMachine) -> PyResult { + if let Some(deleter) = self.deleter.as_ref() { + vm.invoke(deleter.clone(), obj) + } else { + Err(vm.new_attribute_error("can't delete attribute".to_string())) + } + } +} + +pub struct PropertyBuilder<'a, I, T> { + vm: &'a mut VirtualMachine, + getter: Option, + setter: Option, + instance: PhantomData, + _return: PhantomData +} + + +impl<'a, I, T> PropertyBuilder<'a, I, T> { + pub(crate) fn new(vm: &'a mut VirtualMachine) -> Self { + Self { + vm, + getter: None, + setter: None, + instance: PhantomData, + _return: PhantomData + } + } + + pub fn add_getter>(self, func: F) -> Self { + let func = self.vm.ctx.new_rustfunc(func); + Self { + vm: self.vm, + getter: Some(func), + setter: self.setter, + instance: PhantomData, + _return: PhantomData + } + } + + pub fn add_setter>(self, func: F) -> Self { + let func = self.vm.ctx.new_rustfunc(func); + Self { + vm: self.vm, + getter: self.setter, + setter: Some(func), + instance: PhantomData, + _return: PhantomData + } + } + + pub fn create(self) -> PyObjectRef { + if self.setter.is_some() { + let payload = PyProperty { + getter: self.getter.clone(), + setter: self.setter.clone(), + deleter: None + }; + + PyObject::new( + PyObjectPayload::AnyRustValue { + value: Box::new(payload) + }, + self.vm.ctx.property_type(), + ) + } else { + let payload = PyReadOnlyProperty { + getter: self.getter.expect("One of add_getter/add_setter must be called when constructing a property") + }; + + PyObject::new( + PyObjectPayload::AnyRustValue { + value: Box::new(payload) + }, + self.vm.ctx.readonly_property_type(), + ) + } + } +} + pub fn init(context: &PyContext) { - let property_type = &context.property_type; + extend_class!(context, &context.readonly_property_type, { + "__get__" => context.new_rustfunc(PyReadOnlyPropertyRef::get), + }); let property_doc = "Property attribute.\n\n \ @@ -36,50 +176,14 @@ pub fn init(context: &PyContext) { def x(self):\n \ del self._x"; - context.set_attr( - &property_type, - "__get__", - context.new_rustfunc(property_get), - ); - context.set_attr( - &property_type, - "__new__", - context.new_rustfunc(property_new), - ); - context.set_attr( - &property_type, - "__doc__", - context.new_str(property_doc.to_string()), - ); - // TODO: how to handle __set__ ? -} + extend_class!(context, &context.property_type, { + "__new__" => context.new_rustfunc(property_new), + "__doc__" => context.new_str(property_doc.to_string()), -// `property` methods. -fn property_get(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { - trace!("property.__get__ {:?}", args.args); - arg_check!( - vm, - args, - required = [ - (cls, Some(vm.ctx.property_type())), - (inst, None), - (_owner, None) - ] - ); - - match vm.ctx.get_attr(&cls, "fget") { - Some(getter) => { - let py_method = vm.ctx.new_bound_method(getter, inst.clone()); - vm.invoke(py_method, PyFuncArgs::default()) - } - None => { - let attribute_error = vm.context().exceptions.attribute_error.clone(); - Err(vm.new_exception( - attribute_error, - String::from("Attribute Error: property must have 'fget' attribute"), - )) - } - } + "__get__" => context.new_rustfunc(PyPropertyRef::get), + "__set__" => context.new_rustfunc(PyPropertyRef::set), + "__delete__" => context.new_rustfunc(PyPropertyRef::delete), + }); } fn property_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index afdb1fb475..fc14ac52fd 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -147,6 +147,7 @@ pub struct PyContext { pub function_type: PyObjectRef, pub builtin_function_or_method_type: PyObjectRef, pub property_type: PyObjectRef, + pub readonly_property_type: PyObjectRef, pub module_type: PyObjectRef, pub bound_method_type: PyObjectRef, pub member_descriptor_type: PyObjectRef, @@ -197,6 +198,7 @@ impl PyContext { &dict_type, ); let property_type = create_type("property", &type_type, &object_type, &dict_type); + let readonly_property_type = create_type("readonly_property", &type_type, &object_type, &dict_type); let super_type = create_type("super", &type_type, &object_type, &dict_type); let generator_type = create_type("generator", &type_type, &object_type, &dict_type); let bound_method_type = create_type("method", &type_type, &object_type, &dict_type); @@ -297,6 +299,7 @@ impl PyContext { builtin_function_or_method_type, super_type, property_type, + readonly_property_type, generator_type, module_type, bound_method_type, @@ -448,6 +451,10 @@ impl PyContext { self.property_type.clone() } + pub fn readonly_property_type(&self) -> PyObjectRef { + self.readonly_property_type.clone() + } + pub fn classmethod_type(&self) -> PyObjectRef { self.classmethod_type.clone() } @@ -962,6 +969,16 @@ impl From> for PyFuncArgs { } } +impl From for PyFuncArgs { + fn from(arg: PyObjectRef) -> Self { + PyFuncArgs { + args: vec![arg], + kwargs: vec![], + } + } +} + + impl PyFuncArgs { pub fn new(mut args: Vec, kwarg_names: Vec) -> PyFuncArgs { let mut kwargs = vec![]; diff --git a/vm/src/vm.rs b/vm/src/vm.rs index e3d06d5fae..652c05df14 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -132,6 +132,11 @@ impl VirtualMachine { self.invoke(exc_type, args).unwrap() } + pub fn new_attribute_error(&mut self, msg: String) -> PyObjectRef { + let type_error = self.ctx.exceptions.arithmetic_error.clone(); + self.new_exception(type_error, msg) + } + pub fn new_type_error(&mut self, msg: String) -> PyObjectRef { let type_error = self.ctx.exceptions.type_error.clone(); self.new_exception(type_error, msg) From 2edfe4c7be09484f1b7a3c64747daab92ffb92e0 Mon Sep 17 00:00:00 2001 From: ben Date: Sat, 9 Mar 2019 13:00:54 +1300 Subject: [PATCH 2/4] Migrate usage of member_descriptor and data_descriptor to new_property/PropertyBuilder --- vm/src/obj/objcode.rs | 5 ++-- vm/src/obj/objfunction.rs | 50 +-------------------------------------- vm/src/obj/objobject.rs | 12 ++++++---- vm/src/obj/objproperty.rs | 38 +++++++++++++---------------- vm/src/obj/objtype.rs | 5 ++-- vm/src/pyobject.rs | 45 ++--------------------------------- 6 files changed, 31 insertions(+), 124 deletions(-) diff --git a/vm/src/obj/objcode.rs b/vm/src/obj/objcode.rs index b6a1850899..ec3e48ec0f 100644 --- a/vm/src/obj/objcode.rs +++ b/vm/src/obj/objcode.rs @@ -47,7 +47,7 @@ pub fn init(context: &PyContext) { ("co_kwonlyargcount", code_co_kwonlyargcount), ("co_name", code_co_name), ] { - context.set_attr(code_type, name, context.new_member_descriptor(f)) + context.set_attr(code_type, name, context.new_property(f)) } } @@ -83,8 +83,7 @@ fn member_code_obj(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult PyResult { } } -fn member_get(vm: &mut VirtualMachine, mut args: PyFuncArgs) -> PyResult { - match args.shift().get_attr("function") { - Some(function) => vm.invoke(function, args), - None => { - let attribute_error = vm.context().exceptions.attribute_error.clone(); - Err(vm.new_exception(attribute_error, String::from("Attribute Error"))) - } - } -} - -fn data_get(vm: &mut VirtualMachine, mut args: PyFuncArgs) -> PyResult { - match args.shift().get_attr("fget") { - Some(function) => vm.invoke(function, args), - None => { - let attribute_error = vm.context().exceptions.attribute_error.clone(); - Err(vm.new_exception(attribute_error, String::from("Attribute Error"))) - } - } -} - -fn data_set(vm: &mut VirtualMachine, mut args: PyFuncArgs) -> PyResult { - match args.shift().get_attr("fset") { - Some(function) => vm.invoke(function, args), - None => { - let attribute_error = vm.context().exceptions.attribute_error.clone(); - Err(vm.new_exception(attribute_error, String::from("Attribute Error"))) - } - } -} - // Classmethod type methods: fn classmethod_get(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { trace!("classmethod.__get__ {:?}", args.args); diff --git a/vm/src/obj/objobject.rs b/vm/src/obj/objobject.rs index e2bf4463e2..9b161f9d26 100644 --- a/vm/src/obj/objobject.rs +++ b/vm/src/obj/objobject.rs @@ -8,6 +8,7 @@ use crate::pyobject::{ use crate::vm::VirtualMachine; use std::cell::RefCell; use std::collections::HashMap; +use crate::obj::objproperty::PropertyBuilder; #[derive(Clone, Debug)] pub struct PyInstance; @@ -171,7 +172,9 @@ pub fn init(context: &PyContext) { context.set_attr( &object, "__class__", - context.new_data_descriptor(object_class, object_class_setter), + PropertyBuilder::new(context) + .add_getter(object_class) + .add_setter(object_class_setter).create(), ); context.set_attr(&object, "__eq__", context.new_rustfunc(object_eq)); context.set_attr(&object, "__ne__", context.new_rustfunc(object_ne)); @@ -183,7 +186,7 @@ pub fn init(context: &PyContext) { context.set_attr( &object, "__dict__", - context.new_member_descriptor(object_dict), + context.new_property(object_dict), ); context.set_attr(&object, "__dir__", context.new_rustfunc(object_dir)); context.set_attr(&object, "__hash__", context.new_rustfunc(object_hash)); @@ -202,9 +205,8 @@ fn object_init(vm: &mut VirtualMachine, _args: PyFuncArgs) -> PyResult { Ok(vm.ctx.none()) } -// TODO Use PyClassRef for owner to enforce type -fn object_class(_obj: PyObjectRef, owner: PyObjectRef, _vm: &mut VirtualMachine) -> PyObjectRef { - owner +fn object_class(obj: PyObjectRef, _vm: &mut VirtualMachine) -> PyObjectRef { + obj.typ() } fn object_class_setter( diff --git a/vm/src/obj/objproperty.rs b/vm/src/obj/objproperty.rs index e1e6bfa3c6..ac3c21d5f1 100644 --- a/vm/src/obj/objproperty.rs +++ b/vm/src/obj/objproperty.rs @@ -4,10 +4,10 @@ use crate::pyobject::{PyContext, PyFuncArgs, PyObject, PyObjectRef, PyObjectPayload, PyObjectPayload2, PyResult, TypeProtocol}; use crate::pyobject::IntoPyNativeFunc; -use crate::obj::objnone::PyNone; use crate::VirtualMachine; use crate::function::PyRef; use std::marker::PhantomData; +use crate::obj::objtype::PyClassRef; /// Read-only property, doesn't have __set__ or __delete__ #[derive(Debug)] @@ -24,7 +24,7 @@ impl PyObjectPayload2 for PyReadOnlyProperty { pub type PyReadOnlyPropertyRef = PyRef; impl PyReadOnlyPropertyRef { - fn get(self, obj: PyObjectRef, _owner: PyObjectRef, vm: &mut VirtualMachine) -> PyResult { + fn get(self, obj: PyObjectRef, _owner: PyClassRef, vm: &mut VirtualMachine) -> PyResult { vm.invoke(self.getter.clone(), obj) } } @@ -46,7 +46,7 @@ impl PyObjectPayload2 for PyProperty { pub type PyPropertyRef = PyRef; impl PyPropertyRef { - fn get(self, obj: PyObjectRef, _owner: PyObjectRef, vm: &mut VirtualMachine) -> PyResult { + fn get(self, obj: PyObjectRef, _owner: PyClassRef, vm: &mut VirtualMachine) -> PyResult { if let Some(getter) = self.getter.as_ref() { vm.invoke(getter.clone(), obj) } else { @@ -71,44 +71,40 @@ impl PyPropertyRef { } } -pub struct PropertyBuilder<'a, I, T> { - vm: &'a mut VirtualMachine, +pub struct PropertyBuilder<'a, T> { + ctx: &'a PyContext, getter: Option, setter: Option, - instance: PhantomData, _return: PhantomData } -impl<'a, I, T> PropertyBuilder<'a, I, T> { - pub(crate) fn new(vm: &'a mut VirtualMachine) -> Self { +impl<'a, T> PropertyBuilder<'a, T> { + pub fn new(ctx: &'a PyContext) -> Self { Self { - vm, + ctx, getter: None, setter: None, - instance: PhantomData, _return: PhantomData } } - pub fn add_getter>(self, func: F) -> Self { - let func = self.vm.ctx.new_rustfunc(func); + pub fn add_getter>(self, func: F) -> Self { + let func = self.ctx.new_rustfunc(func); Self { - vm: self.vm, + ctx: self.ctx, getter: Some(func), setter: self.setter, - instance: PhantomData, _return: PhantomData } } - pub fn add_setter>(self, func: F) -> Self { - let func = self.vm.ctx.new_rustfunc(func); + pub fn add_setter>(self, func: F) -> Self { + let func = self.ctx.new_rustfunc(func); Self { - vm: self.vm, - getter: self.setter, + ctx: self.ctx, + getter: self.getter, setter: Some(func), - instance: PhantomData, _return: PhantomData } } @@ -125,7 +121,7 @@ impl<'a, I, T> PropertyBuilder<'a, I, T> { PyObjectPayload::AnyRustValue { value: Box::new(payload) }, - self.vm.ctx.property_type(), + self.ctx.property_type(), ) } else { let payload = PyReadOnlyProperty { @@ -136,7 +132,7 @@ impl<'a, I, T> PropertyBuilder<'a, I, T> { PyObjectPayload::AnyRustValue { value: Box::new(payload) }, - self.vm.ctx.readonly_property_type(), + self.ctx.readonly_property_type(), ) } } diff --git a/vm/src/obj/objtype.rs b/vm/src/obj/objtype.rs index b82e39465a..55d4443d38 100644 --- a/vm/src/obj/objtype.rs +++ b/vm/src/obj/objtype.rs @@ -53,7 +53,7 @@ pub fn init(context: &PyContext) { context.set_attr( &type_type, "__mro__", - context.new_member_descriptor(type_mro), + context.new_property(type_mro), ); context.set_attr(&type_type, "__repr__", context.new_rustfunc(type_repr)); context.set_attr( @@ -85,8 +85,7 @@ fn type_mro(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { vm, args, required = [ - (cls, Some(vm.ctx.type_type())), - (_typ, Some(vm.ctx.type_type())) + (cls, Some(vm.ctx.type_type())) ] ); match _mro(cls.clone()) { diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 07afc8f4fa..c32c71667b 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -151,8 +151,6 @@ pub struct PyContext { pub readonly_property_type: PyObjectRef, pub module_type: PyObjectRef, pub bound_method_type: PyObjectRef, - pub member_descriptor_type: PyObjectRef, - pub data_descriptor_type: PyObjectRef, pub object: PyObjectRef, pub exceptions: exceptions::ExceptionZoo, } @@ -198,10 +196,6 @@ impl PyContext { let super_type = create_type("super", &type_type, &object_type, &dict_type); let generator_type = create_type("generator", &type_type, &object_type, &dict_type); let bound_method_type = create_type("method", &type_type, &object_type, &dict_type); - let member_descriptor_type = - create_type("member_descriptor", &type_type, &object_type, &dict_type); - let data_descriptor_type = - create_type("data_descriptor", &type_type, &object_type, &dict_type); let str_type = create_type("str", &type_type, &object_type, &dict_type); let list_type = create_type("list", &type_type, &object_type, &dict_type); let set_type = create_type("set", &type_type, &object_type, &dict_type); @@ -299,8 +293,6 @@ impl PyContext { generator_type, module_type, bound_method_type, - member_descriptor_type, - data_descriptor_type, type_type, exceptions, }; @@ -466,12 +458,6 @@ impl PyContext { pub fn bound_method_type(&self) -> PyObjectRef { self.bound_method_type.clone() } - pub fn member_descriptor_type(&self) -> PyObjectRef { - self.member_descriptor_type.clone() - } - pub fn data_descriptor_type(&self) -> PyObjectRef { - self.data_descriptor_type.clone() - } pub fn type_type(&self) -> PyObjectRef { self.type_type.clone() @@ -640,10 +626,7 @@ impl PyContext { where F: IntoPyNativeFunc, { - let fget = self.new_rustfunc(f); - let py_obj = self.new_instance(self.property_type(), None); - self.set_attr(&py_obj, "fget", fget); - py_obj + PropertyBuilder::new(self).add_getter(f).create() } pub fn new_code_object(&self, code: bytecode::CodeObject) -> PyObjectRef { @@ -678,31 +661,6 @@ impl PyContext { ) } - pub fn new_member_descriptor PyResult>( - &self, - function: F, - ) -> PyObjectRef { - let mut dict = PyAttributes::new(); - dict.insert("function".to_string(), self.new_rustfunc(function)); - self.new_instance(self.member_descriptor_type(), Some(dict)) - } - - pub fn new_data_descriptor< - G: IntoPyNativeFunc<(I, PyObjectRef), T>, - S: IntoPyNativeFunc<(I, T), PyResult>, - T, - I, - >( - &self, - getter: G, - setter: S, - ) -> PyObjectRef { - let mut dict = PyAttributes::new(); - dict.insert("fget".to_string(), self.new_rustfunc(getter)); - dict.insert("fset".to_string(), self.new_rustfunc(setter)); - self.new_instance(self.data_descriptor_type(), Some(dict)) - } - pub fn new_instance(&self, class: PyObjectRef, dict: Option) -> PyObjectRef { let dict = if let Some(dict) = dict { dict @@ -1292,6 +1250,7 @@ pub enum OptionalArg { } use self::OptionalArg::*; +use crate::obj::objproperty::PropertyBuilder; impl OptionalArg { pub fn into_option(self) -> Option { From 0ec034df51adb15c4808f658f2221fabf6780a76 Mon Sep 17 00:00:00 2001 From: ben Date: Sat, 9 Mar 2019 14:07:42 +1300 Subject: [PATCH 3/4] Change property.__new__ to use new style function and construct PyProperty --- vm/src/function.rs | 20 ++++++++++++ vm/src/obj/objcode.rs | 8 +---- vm/src/obj/objfunction.rs | 1 - vm/src/obj/objobject.rs | 11 +++---- vm/src/obj/objproperty.rs | 67 +++++++++++++++++++++++---------------- vm/src/obj/objtype.rs | 14 ++------ vm/src/pyobject.rs | 4 +-- 7 files changed, 69 insertions(+), 56 deletions(-) diff --git a/vm/src/function.rs b/vm/src/function.rs index 993167ffc9..1589008514 100644 --- a/vm/src/function.rs +++ b/vm/src/function.rs @@ -3,6 +3,7 @@ use std::marker::PhantomData; use std::ops::Deref; use crate::obj::objtype; +use crate::obj::objtype::PyClassRef; use crate::pyobject::{ IntoPyObject, PyContext, PyObject, PyObjectPayload, PyObjectPayload2, PyObjectRef, PyResult, TryFromObject, TypeProtocol, @@ -44,6 +45,25 @@ where _payload: PhantomData, } } + + pub fn new_with_type(vm: &mut VirtualMachine, payload: T, cls: PyClassRef) -> PyResult { + let required_type = T::required_type(&vm.ctx); + if objtype::issubclass(&cls.obj, &required_type) { + Ok(PyRef { + obj: PyObject::new( + PyObjectPayload::AnyRustValue { + value: Box::new(payload), + }, + cls.obj, + ), + _payload: PhantomData, + }) + } else { + let subtype = vm.to_pystr(&cls.obj)?; + let basetype = vm.to_pystr(&required_type)?; + Err(vm.new_type_error(format!("{} is not a subtype of {}", subtype, basetype))) + } + } } impl Deref for PyRef diff --git a/vm/src/obj/objcode.rs b/vm/src/obj/objcode.rs index ec3e48ec0f..ccd2114aed 100644 --- a/vm/src/obj/objcode.rs +++ b/vm/src/obj/objcode.rs @@ -79,13 +79,7 @@ fn code_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { } fn member_code_obj(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { - arg_check!( - vm, - args, - required = [ - (zelf, Some(vm.ctx.code_type())) - ] - ); + arg_check!(vm, args, required = [(zelf, Some(vm.ctx.code_type()))]); Ok(get_value(zelf)) } diff --git a/vm/src/obj/objfunction.rs b/vm/src/obj/objfunction.rs index 12bba360ad..0c87826f25 100644 --- a/vm/src/obj/objfunction.rs +++ b/vm/src/obj/objfunction.rs @@ -20,7 +20,6 @@ pub fn init(context: &PyContext) { context.new_rustfunc(bind_method), ); - let classmethod_type = &context.classmethod_type; context.set_attr( &classmethod_type, diff --git a/vm/src/obj/objobject.rs b/vm/src/obj/objobject.rs index 9b161f9d26..0d9d21f1ea 100644 --- a/vm/src/obj/objobject.rs +++ b/vm/src/obj/objobject.rs @@ -1,6 +1,7 @@ use super::objstr; use super::objtype; use crate::function::PyRef; +use crate::obj::objproperty::PropertyBuilder; use crate::pyobject::{ AttributeProtocol, DictProtocol, IdProtocol, PyAttributes, PyContext, PyFuncArgs, PyObject, PyObjectPayload, PyObjectRef, PyResult, TypeProtocol, @@ -8,7 +9,6 @@ use crate::pyobject::{ use crate::vm::VirtualMachine; use std::cell::RefCell; use std::collections::HashMap; -use crate::obj::objproperty::PropertyBuilder; #[derive(Clone, Debug)] pub struct PyInstance; @@ -174,7 +174,8 @@ pub fn init(context: &PyContext) { "__class__", PropertyBuilder::new(context) .add_getter(object_class) - .add_setter(object_class_setter).create(), + .add_setter(object_class_setter) + .create(), ); context.set_attr(&object, "__eq__", context.new_rustfunc(object_eq)); context.set_attr(&object, "__ne__", context.new_rustfunc(object_ne)); @@ -183,11 +184,7 @@ pub fn init(context: &PyContext) { context.set_attr(&object, "__gt__", context.new_rustfunc(object_gt)); context.set_attr(&object, "__ge__", context.new_rustfunc(object_ge)); context.set_attr(&object, "__delattr__", context.new_rustfunc(object_delattr)); - context.set_attr( - &object, - "__dict__", - context.new_property(object_dict), - ); + context.set_attr(&object, "__dict__", context.new_property(object_dict)); context.set_attr(&object, "__dir__", context.new_rustfunc(object_dir)); context.set_attr(&object, "__hash__", context.new_rustfunc(object_hash)); context.set_attr(&object, "__str__", context.new_rustfunc(object_str)); diff --git a/vm/src/obj/objproperty.rs b/vm/src/obj/objproperty.rs index ac3c21d5f1..b94692177e 100644 --- a/vm/src/obj/objproperty.rs +++ b/vm/src/obj/objproperty.rs @@ -2,17 +2,20 @@ */ -use crate::pyobject::{PyContext, PyFuncArgs, PyObject, PyObjectRef, PyObjectPayload, PyObjectPayload2, PyResult, TypeProtocol}; +use crate::function::PyRef; +use crate::obj::objstr::PyStringRef; +use crate::obj::objtype::PyClassRef; use crate::pyobject::IntoPyNativeFunc; +use crate::pyobject::{ + OptionalArg, PyContext, PyObject, PyObjectPayload, PyObjectPayload2, PyObjectRef, PyResult, +}; use crate::VirtualMachine; -use crate::function::PyRef; use std::marker::PhantomData; -use crate::obj::objtype::PyClassRef; /// Read-only property, doesn't have __set__ or __delete__ #[derive(Debug)] pub struct PyReadOnlyProperty { - getter: PyObjectRef + getter: PyObjectRef, } impl PyObjectPayload2 for PyReadOnlyProperty { @@ -34,7 +37,7 @@ impl PyReadOnlyPropertyRef { pub struct PyProperty { getter: Option, setter: Option, - deleter: Option + deleter: Option, } impl PyObjectPayload2 for PyProperty { @@ -46,6 +49,25 @@ impl PyObjectPayload2 for PyProperty { pub type PyPropertyRef = PyRef; impl PyPropertyRef { + fn new_property( + cls: PyClassRef, + fget: OptionalArg, + fset: OptionalArg, + fdel: OptionalArg, + _doc: OptionalArg, + vm: &mut VirtualMachine, + ) -> PyResult { + Self::new_with_type( + vm, + PyProperty { + getter: fget.into_option(), + setter: fset.into_option(), + deleter: fdel.into_option(), + }, + cls, + ) + } + fn get(self, obj: PyObjectRef, _owner: PyClassRef, vm: &mut VirtualMachine) -> PyResult { if let Some(getter) = self.getter.as_ref() { vm.invoke(getter.clone(), obj) @@ -75,37 +97,36 @@ pub struct PropertyBuilder<'a, T> { ctx: &'a PyContext, getter: Option, setter: Option, - _return: PhantomData + _return: PhantomData, } - impl<'a, T> PropertyBuilder<'a, T> { pub fn new(ctx: &'a PyContext) -> Self { Self { ctx, getter: None, setter: None, - _return: PhantomData + _return: PhantomData, } } - pub fn add_getter>(self, func: F) -> Self { + pub fn add_getter>(self, func: F) -> Self { let func = self.ctx.new_rustfunc(func); Self { ctx: self.ctx, getter: Some(func), setter: self.setter, - _return: PhantomData + _return: PhantomData, } } - pub fn add_setter>(self, func: F) -> Self { + pub fn add_setter>(self, func: F) -> Self { let func = self.ctx.new_rustfunc(func); Self { ctx: self.ctx, getter: self.getter, setter: Some(func), - _return: PhantomData + _return: PhantomData, } } @@ -114,23 +135,25 @@ impl<'a, T> PropertyBuilder<'a, T> { let payload = PyProperty { getter: self.getter.clone(), setter: self.setter.clone(), - deleter: None + deleter: None, }; PyObject::new( PyObjectPayload::AnyRustValue { - value: Box::new(payload) + value: Box::new(payload), }, self.ctx.property_type(), ) } else { let payload = PyReadOnlyProperty { - getter: self.getter.expect("One of add_getter/add_setter must be called when constructing a property") + getter: self.getter.expect( + "One of add_getter/add_setter must be called when constructing a property", + ), }; PyObject::new( PyObjectPayload::AnyRustValue { - value: Box::new(payload) + value: Box::new(payload), }, self.ctx.readonly_property_type(), ) @@ -138,7 +161,6 @@ impl<'a, T> PropertyBuilder<'a, T> { } } - pub fn init(context: &PyContext) { extend_class!(context, &context.readonly_property_type, { "__get__" => context.new_rustfunc(PyReadOnlyPropertyRef::get), @@ -173,7 +195,7 @@ pub fn init(context: &PyContext) { del self._x"; extend_class!(context, &context.property_type, { - "__new__" => context.new_rustfunc(property_new), + "__new__" => context.new_rustfunc(PyPropertyRef::new_property), "__doc__" => context.new_str(property_doc.to_string()), "__get__" => context.new_rustfunc(PyPropertyRef::get), @@ -181,12 +203,3 @@ pub fn init(context: &PyContext) { "__delete__" => context.new_rustfunc(PyPropertyRef::delete), }); } - -fn property_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { - trace!("property.__new__ {:?}", args.args); - arg_check!(vm, args, required = [(cls, None), (fget, None)]); - - let py_obj = vm.ctx.new_instance(cls.clone(), None); - vm.ctx.set_attr(&py_obj, "fget", fget.clone()); - Ok(py_obj) -} diff --git a/vm/src/obj/objtype.rs b/vm/src/obj/objtype.rs index 55d4443d38..be4591dfcd 100644 --- a/vm/src/obj/objtype.rs +++ b/vm/src/obj/objtype.rs @@ -50,11 +50,7 @@ pub fn init(context: &PyContext) { context.set_attr(&type_type, "__call__", context.new_rustfunc(type_call)); context.set_attr(&type_type, "__new__", context.new_rustfunc(type_new)); - context.set_attr( - &type_type, - "__mro__", - context.new_property(type_mro), - ); + context.set_attr(&type_type, "__mro__", context.new_property(type_mro)); context.set_attr(&type_type, "__repr__", context.new_rustfunc(type_repr)); context.set_attr( &type_type, @@ -81,13 +77,7 @@ pub fn init(context: &PyContext) { } fn type_mro(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { - arg_check!( - vm, - args, - required = [ - (cls, Some(vm.ctx.type_type())) - ] - ); + arg_check!(vm, args, required = [(cls, Some(vm.ctx.type_type()))]); match _mro(cls.clone()) { Some(mro) => Ok(vm.context().new_tuple(mro)), None => Err(vm.new_type_error("Only classes have an MRO.".to_string())), diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index c32c71667b..c42f047ffb 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -192,7 +192,8 @@ impl PyContext { &dict_type, ); let property_type = create_type("property", &type_type, &object_type, &dict_type); - let readonly_property_type = create_type("readonly_property", &type_type, &object_type, &dict_type); + let readonly_property_type = + create_type("readonly_property", &type_type, &object_type, &dict_type); let super_type = create_type("super", &type_type, &object_type, &dict_type); let generator_type = create_type("generator", &type_type, &object_type, &dict_type); let bound_method_type = create_type("method", &type_type, &object_type, &dict_type); @@ -948,7 +949,6 @@ impl From for PyFuncArgs { } } - impl PyFuncArgs { pub fn new(mut args: Vec, kwarg_names: Vec) -> PyFuncArgs { let mut kwargs = vec![]; From 3c3c1f2b6f6fd8e1938b63153554f90cfa611ac9 Mon Sep 17 00:00:00 2001 From: ben Date: Sun, 10 Mar 2019 09:01:46 +1300 Subject: [PATCH 4/4] Fixed new_attribute_error and added more tests for property --- tests/snippets/class.py | 16 ---------------- tests/snippets/property.py | 32 ++++++++++++++++++++++++++++++++ vm/src/vm.rs | 2 +- 3 files changed, 33 insertions(+), 17 deletions(-) create mode 100644 tests/snippets/property.py diff --git a/tests/snippets/class.py b/tests/snippets/class.py index 5bba7be96b..ba2d423ea1 100644 --- a/tests/snippets/class.py +++ b/tests/snippets/class.py @@ -16,22 +16,6 @@ def square(self): assert foo.square() == 25 -class Fubar: - def __init__(self): - self.x = 100 - - @property - def foo(self): - value = self.x - self.x += 1 - return value - - -f = Fubar() -assert f.foo == 100 -assert f.foo == 101 - - class Bar: """ W00t """ def __init__(self, x): diff --git a/tests/snippets/property.py b/tests/snippets/property.py new file mode 100644 index 0000000000..a55870e3b8 --- /dev/null +++ b/tests/snippets/property.py @@ -0,0 +1,32 @@ +from testutils import assertRaises + + +class Fubar: + def __init__(self): + self.x = 100 + + @property + def foo(self): + value = self.x + self.x += 1 + return value + + +f = Fubar() +assert f.foo == 100 +assert f.foo == 101 + + +null_property = property() +assert type(null_property) is property + +p = property(lambda x: x[0]) +assert p.__get__((2,), tuple) == 2 +# TODO owner parameter is optional +# assert p.__get__((2,)) == 2 + +with assertRaises(AttributeError): + null_property.__get__((), tuple) + +with assertRaises(TypeError): + property.__new__(object) diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 7f50a4636e..5dabf19cb8 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -135,7 +135,7 @@ impl VirtualMachine { } pub fn new_attribute_error(&mut self, msg: String) -> PyObjectRef { - let type_error = self.ctx.exceptions.arithmetic_error.clone(); + let type_error = self.ctx.exceptions.attribute_error.clone(); self.new_exception(type_error, msg) }