From 9f5cd17f2bc296ddf97c4e8311000234f8222d43 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 2 Feb 2020 21:43:24 +0900 Subject: [PATCH 01/10] Add getset_descriptor --- vm/src/obj/mod.rs | 1 + vm/src/obj/objgetset.rs | 71 +++++++++++++++++++++++++++++++++++++++++ vm/src/obj/objobject.rs | 4 +-- vm/src/pyobject.rs | 4 +++ vm/src/types.rs | 5 +++ 5 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 vm/src/obj/objgetset.rs diff --git a/vm/src/obj/mod.rs b/vm/src/obj/mod.rs index 57233025da..af5d531d1d 100644 --- a/vm/src/obj/mod.rs +++ b/vm/src/obj/mod.rs @@ -17,6 +17,7 @@ pub mod objfloat; pub mod objframe; pub mod objfunction; pub mod objgenerator; +pub mod objgetset; pub mod objint; pub mod objiter; pub mod objlist; diff --git a/vm/src/obj/objgetset.rs b/vm/src/obj/objgetset.rs new file mode 100644 index 0000000000..e8ee40b67d --- /dev/null +++ b/vm/src/obj/objgetset.rs @@ -0,0 +1,71 @@ +/*! Python `attribute` descriptor class. (PyGetSet) + +*/ +use super::objtype::PyClassRef; +use crate::function::OptionalArg; +use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue}; +use crate::slots::PyBuiltinDescriptor; +use crate::vm::VirtualMachine; + +pub type PyGetter = dyn Fn(PyObjectRef, &VirtualMachine) -> PyResult; +pub type PySetter = dyn Fn(PyObjectRef, PyObjectRef, &VirtualMachine) -> PyResult<()>; + +#[pyclass] +pub struct PyGetSet { + // name: String, + getter: Box, + setter: Box, + // doc: Option, +} + +impl std::fmt::Debug for PyGetSet { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "PyGetSet {{ getter: {:p}, setter: {:p} }}", + self.getter, self.setter + ) + } +} + +impl PyValue for PyGetSet { + fn class(vm: &VirtualMachine) -> PyClassRef { + vm.ctx.getset_type() + } +} + +pub type PyGetSetRef = PyRef; + +impl PyBuiltinDescriptor for PyGetSet { + fn get( + zelf: PyRef, + obj: PyObjectRef, + _cls: OptionalArg, + vm: &VirtualMachine, + ) -> PyResult { + (zelf.getter)(obj, vm) + } +} + +impl PyGetSet { + pub fn new(getter: &'static PyGetter, setter: &'static PySetter) -> Self { + Self { + getter: Box::new(getter), + setter: Box::new(setter), + } + } +} + +#[pyimpl(with(PyBuiltinDescriptor))] +impl PyGetSet { + // Descriptor methods + + #[pymethod(magic)] + fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + (self.setter)(obj, value, vm) + } +} + +pub(crate) fn init(context: &PyContext) { + PyGetSet::extend_class(context, &context.types.getset_type); +} diff --git a/vm/src/obj/objobject.rs b/vm/src/obj/objobject.rs index 671dcd9e68..bafec4abbe 100644 --- a/vm/src/obj/objobject.rs +++ b/vm/src/obj/objobject.rs @@ -175,12 +175,12 @@ impl PyBaseObject { } #[pyproperty(name = "__class__")] - fn _class(obj: PyObjectRef, _vm: &VirtualMachine) -> PyObjectRef { + fn get_class(obj: PyObjectRef, _vm: &VirtualMachine) -> PyObjectRef { obj.class().into_object() } #[pyproperty(name = "__class__", setter)] - fn _set_class(instance: PyObjectRef, _value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + fn set_class(instance: PyObjectRef, _value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let type_repr = vm.to_pystr(&instance.class())?; Err(vm.new_type_error(format!("can't change class of type '{}'", type_repr))) } diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index db0df14a11..555d480bd4 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -330,6 +330,10 @@ impl PyContext { self.types.readonly_property_type.clone() } + pub fn getset_type(&self) -> PyClassRef { + self.types.getset_type.clone() + } + pub fn classmethod_type(&self) -> PyClassRef { self.types.classmethod_type.clone() } diff --git a/vm/src/types.rs b/vm/src/types.rs index 2c9d940265..a81755a010 100644 --- a/vm/src/types.rs +++ b/vm/src/types.rs @@ -14,6 +14,7 @@ use crate::obj::objfloat; use crate::obj::objframe; use crate::obj::objfunction; use crate::obj::objgenerator; +use crate::obj::objgetset; use crate::obj::objint; use crate::obj::objiter; use crate::obj::objlist; @@ -94,6 +95,7 @@ pub struct TypeZoo { pub method_descriptor_type: PyClassRef, pub property_type: PyClassRef, pub readonly_property_type: PyClassRef, + pub getset_type: PyClassRef, pub module_type: PyClassRef, pub namespace_type: PyClassRef, pub bound_method_type: PyClassRef, @@ -125,6 +127,7 @@ impl TypeZoo { let method_descriptor_type = create_type("method_descriptor", &type_type, &object_type); let property_type = create_type("property", &type_type, &object_type); let readonly_property_type = create_type("readonly_property", &type_type, &object_type); + let getset_type = create_type("getset_descriptor", &type_type, &object_type); let super_type = create_type("super", &type_type, &object_type); let weakref_type = create_type("ref", &type_type, &object_type); let weakproxy_type = create_type("weakproxy", &type_type, &object_type); @@ -220,6 +223,7 @@ impl TypeZoo { mappingproxy_type, property_type, readonly_property_type, + getset_type, generator_type, module_type, namespace_type, @@ -381,6 +385,7 @@ pub fn initialize_types(context: &PyContext) { objbytes::init(&context); objbytearray::init(&context); objproperty::init(&context); + objgetset::init(&context); objmemory::init(&context); objstr::init(&context); objrange::init(&context); From c3d5f6c14598be14bee72d2f1ec5405566d6e735 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 2 Feb 2020 22:56:28 +0900 Subject: [PATCH 02/10] IntoPyGetterFunc, IntoPySetterFunc --- derive/src/pyclass.rs | 7 ++- vm/src/obj/objgetset.rs | 107 +++++++++++++++++++++++++++++++++++----- 2 files changed, 97 insertions(+), 17 deletions(-) diff --git a/derive/src/pyclass.rs b/derive/src/pyclass.rs index 006bd4e60a..5265bdd739 100644 --- a/derive/src/pyclass.rs +++ b/derive/src/pyclass.rs @@ -337,17 +337,16 @@ impl Class { }; if name == "pymethod" { self.add_item(Self::extract_method(sig, meta)?, meta_span)?; - attr_idxs.push(i); } else if name == "pyclassmethod" { self.add_item(Self::extract_classmethod(sig, meta)?, meta_span)?; - attr_idxs.push(i); } else if name == "pyproperty" { self.add_item(Self::extract_property(sig, meta)?, meta_span)?; - attr_idxs.push(i); } else if name == "pyslot" { self.add_item(Self::extract_slot(sig, meta)?, meta_span)?; - attr_idxs.push(i); + } else { + continue; } + attr_idxs.push(i); } let mut i = 0; let mut attr_idxs = &*attr_idxs; diff --git a/vm/src/obj/objgetset.rs b/vm/src/obj/objgetset.rs index e8ee40b67d..ce3e73a13e 100644 --- a/vm/src/obj/objgetset.rs +++ b/vm/src/obj/objgetset.rs @@ -3,18 +3,57 @@ */ use super::objtype::PyClassRef; use crate::function::OptionalArg; -use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue}; +use crate::pyobject::{ + IntoPyObject, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, +}; use crate::slots::PyBuiltinDescriptor; use crate::vm::VirtualMachine; -pub type PyGetter = dyn Fn(PyObjectRef, &VirtualMachine) -> PyResult; -pub type PySetter = dyn Fn(PyObjectRef, PyObjectRef, &VirtualMachine) -> PyResult<()>; +pub type PyGetterFunc = Box PyResult>; +pub type PySetterFunc = Box PyResult<()>>; + +pub trait IntoPyGetterFunc { + fn into_getter(self) -> PyGetterFunc; +} + +impl IntoPyGetterFunc for F +where + F: Fn(T, &VirtualMachine) -> R + 'static, + T: TryFromObject, + R: IntoPyObject, +{ + fn into_getter(self) -> PyGetterFunc { + Box::new(move |vm, obj| { + let obj = T::try_from_object(vm, obj)?; + (self)(obj, vm).into_pyobject(vm) + }) + } +} + +pub trait IntoPySetterFunc { + fn into_setter(self) -> PySetterFunc; +} + +impl IntoPySetterFunc for F +where + F: Fn(T, V, &VirtualMachine) -> PyResult<()> + 'static, + T: TryFromObject, + V: TryFromObject, +{ + fn into_setter(self) -> PySetterFunc { + Box::new(move |vm, obj, value| { + let obj = T::try_from_object(vm, obj)?; + let value = V::try_from_object(vm, value)?; + (self)(obj, value, vm) + }) + } +} #[pyclass] pub struct PyGetSet { - // name: String, - getter: Box, - setter: Box, + name: String, + getter: Option, + setter: Option, // doc: Option, } @@ -22,8 +61,18 @@ impl std::fmt::Debug for PyGetSet { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, - "PyGetSet {{ getter: {:p}, setter: {:p} }}", - self.getter, self.setter + "PyGetSet {{ name: {}, getter: {}, setter: {} }}", + self.name, + if self.getter.is_some() { + "Some" + } else { + "None" + }, + if self.setter.is_some() { + "Some" + } else { + "None" + }, ) } } @@ -43,15 +92,39 @@ impl PyBuiltinDescriptor for PyGetSet { _cls: OptionalArg, vm: &VirtualMachine, ) -> PyResult { - (zelf.getter)(obj, vm) + if let Some(ref f) = zelf.getter { + f(vm, obj) + } else { + Err(vm.new_attribute_error(format!( + "attribute '{}' of '{}' objects is not readable", + zelf.name, + Self::class(vm).name + ))) + } } } impl PyGetSet { - pub fn new(getter: &'static PyGetter, setter: &'static PySetter) -> Self { + pub fn with_get(name: String, getter: G) -> Self + where + G: IntoPyGetterFunc, + { + Self { + name, + getter: Some(getter.into_getter()), + setter: None, + } + } + + pub fn with_get_set(name: String, getter: G, setter: S) -> Self + where + G: IntoPyGetterFunc, + S: IntoPySetterFunc, + { Self { - getter: Box::new(getter), - setter: Box::new(setter), + name, + getter: Some(getter.into_getter()), + setter: Some(setter.into_setter()), } } } @@ -62,7 +135,15 @@ impl PyGetSet { #[pymethod(magic)] fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - (self.setter)(obj, value, vm) + 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, + Self::class(vm).name + ))) + } } } From ca557788c8d9330079cfadd05798feeeb5fe57f0 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 4 Feb 2020 02:00:57 +0900 Subject: [PATCH 03/10] `&self` support for getter/setter --- vm/src/obj/objgetset.rs | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/vm/src/obj/objgetset.rs b/vm/src/obj/objgetset.rs index ce3e73a13e..c3c8d12f36 100644 --- a/vm/src/obj/objgetset.rs +++ b/vm/src/obj/objgetset.rs @@ -2,7 +2,7 @@ */ use super::objtype::PyClassRef; -use crate::function::OptionalArg; +use crate::function::{OptionalArg, OwnedParam, RefParam}; use crate::pyobject::{ IntoPyObject, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, }; @@ -16,7 +16,7 @@ pub trait IntoPyGetterFunc { fn into_getter(self) -> PyGetterFunc; } -impl IntoPyGetterFunc for F +impl IntoPyGetterFunc, R> for F where F: Fn(T, &VirtualMachine) -> R + 'static, T: TryFromObject, @@ -30,11 +30,25 @@ where } } +impl IntoPyGetterFunc, R> for F +where + F: Fn(&S, &VirtualMachine) -> R + 'static, + S: PyValue, + R: IntoPyObject, +{ + fn into_getter(self) -> PyGetterFunc { + Box::new(move |vm, obj| { + let zelf = PyRef::::try_from_object(vm, obj)?; + (self)(&zelf, vm).into_pyobject(vm) + }) + } +} + pub trait IntoPySetterFunc { fn into_setter(self) -> PySetterFunc; } -impl IntoPySetterFunc for F +impl IntoPySetterFunc, V> for F where F: Fn(T, V, &VirtualMachine) -> PyResult<()> + 'static, T: TryFromObject, @@ -49,6 +63,21 @@ where } } +impl IntoPySetterFunc, V> for F +where + F: Fn(&S, V, &VirtualMachine) -> PyResult<()> + 'static, + S: PyValue, + V: TryFromObject, +{ + fn into_setter(self) -> PySetterFunc { + Box::new(move |vm, obj, value| { + let zelf = PyRef::::try_from_object(vm, obj)?; + let value = V::try_from_object(vm, value)?; + (self)(&zelf, value, vm) + }) + } +} + #[pyclass] pub struct PyGetSet { name: String, From d1f9cb4e58762d8084f812b4936f180b0a3ebbe4 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 4 Feb 2020 03:17:42 +0900 Subject: [PATCH 04/10] PySetResult and IntoPySetResult --- vm/src/obj/objgetset.rs | 40 ++++++++++++++++++++++++++++++--------- vm/src/obj/objproperty.rs | 12 ++++++------ 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/vm/src/obj/objgetset.rs b/vm/src/obj/objgetset.rs index c3c8d12f36..1e257f1891 100644 --- a/vm/src/obj/objgetset.rs +++ b/vm/src/obj/objgetset.rs @@ -44,36 +44,57 @@ where } } -pub trait IntoPySetterFunc { +pub trait IntoPyNoResult { + fn into_noresult(self) -> PyResult<()>; +} + +impl IntoPyNoResult for () { + fn into_noresult(self) -> PyResult<()> { + Ok(()) + } +} + +impl IntoPyNoResult for PyResult<()> { + fn into_noresult(self) -> PyResult<()> { + self + } +} + +pub trait IntoPySetterFunc +where + R: IntoPyNoResult, +{ fn into_setter(self) -> PySetterFunc; } -impl IntoPySetterFunc, V> for F +impl IntoPySetterFunc, V, R> for F where - F: Fn(T, V, &VirtualMachine) -> PyResult<()> + 'static, + F: Fn(T, V, &VirtualMachine) -> R + 'static, T: TryFromObject, V: TryFromObject, + R: IntoPyNoResult, { fn into_setter(self) -> PySetterFunc { Box::new(move |vm, obj, value| { let obj = T::try_from_object(vm, obj)?; let value = V::try_from_object(vm, value)?; - (self)(obj, value, vm) + (self)(obj, value, vm).into_noresult() }) } } -impl IntoPySetterFunc, V> for F +impl IntoPySetterFunc, V, R> for F where - F: Fn(&S, V, &VirtualMachine) -> PyResult<()> + 'static, + F: Fn(&S, V, &VirtualMachine) -> R + 'static, S: PyValue, V: TryFromObject, + R: IntoPyNoResult, { fn into_setter(self) -> PySetterFunc { Box::new(move |vm, obj, value| { let zelf = PyRef::::try_from_object(vm, obj)?; let value = V::try_from_object(vm, value)?; - (self)(&zelf, value, vm) + (self)(&zelf, value, vm).into_noresult() }) } } @@ -145,10 +166,11 @@ impl PyGetSet { } } - pub fn with_get_set(name: String, getter: G, setter: S) -> Self + pub fn with_get_set(name: String, getter: G, setter: S) -> Self where G: IntoPyGetterFunc, - S: IntoPySetterFunc, + S: IntoPySetterFunc, + SR: IntoPyNoResult, { Self { name, diff --git a/vm/src/obj/objproperty.rs b/vm/src/obj/objproperty.rs index d25feb227c..52645e185f 100644 --- a/vm/src/obj/objproperty.rs +++ b/vm/src/obj/objproperty.rs @@ -259,11 +259,6 @@ pub struct PropertyBuilder<'a> { setter: Option, } -pub trait PropertySetterResult {} - -impl PropertySetterResult for PyResult<()> {} -impl PropertySetterResult for () {} - impl<'a> PropertyBuilder<'a> { pub fn new(ctx: &'a PyContext) -> Self { Self { @@ -282,7 +277,12 @@ impl<'a> PropertyBuilder<'a> { } } - pub fn add_setter>( + pub fn add_setter< + I, + V, + VM, + F: IntoPyNativeFunc<(I, V), impl super::objgetset::IntoPyNoResult, VM>, + >( self, func: F, ) -> Self { From 226a2a6cb9f5f71206430a2ef7f5b39135ba6e9f Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 4 Feb 2020 04:21:32 +0900 Subject: [PATCH 05/10] VM polymorphism for getter and setter --- vm/src/obj/objgetset.rs | 72 +++++++++++++++++++++++++++++++-------- vm/src/obj/objproperty.rs | 2 +- 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/vm/src/obj/objgetset.rs b/vm/src/obj/objgetset.rs index 1e257f1891..13dadd536d 100644 --- a/vm/src/obj/objgetset.rs +++ b/vm/src/obj/objgetset.rs @@ -12,11 +12,11 @@ use crate::vm::VirtualMachine; pub type PyGetterFunc = Box PyResult>; pub type PySetterFunc = Box PyResult<()>>; -pub trait IntoPyGetterFunc { +pub trait IntoPyGetterFunc { fn into_getter(self) -> PyGetterFunc; } -impl IntoPyGetterFunc, R> for F +impl IntoPyGetterFunc<(OwnedParam, R, VirtualMachine)> for F where F: Fn(T, &VirtualMachine) -> R + 'static, T: TryFromObject, @@ -30,7 +30,7 @@ where } } -impl IntoPyGetterFunc, R> for F +impl IntoPyGetterFunc<(RefParam, R, VirtualMachine)> for F where F: Fn(&S, &VirtualMachine) -> R + 'static, S: PyValue, @@ -44,6 +44,28 @@ where } } +impl IntoPyGetterFunc<(OwnedParam, R)> for F +where + F: Fn(T) -> R + 'static, + T: TryFromObject, + R: IntoPyObject, +{ + fn into_getter(self) -> PyGetterFunc { + IntoPyGetterFunc::into_getter(move |obj, _vm: &VirtualMachine| (self)(obj)) + } +} + +impl IntoPyGetterFunc<(RefParam, R)> for F +where + F: Fn(&S) -> R + 'static, + S: PyValue, + R: IntoPyObject, +{ + fn into_getter(self) -> PyGetterFunc { + IntoPyGetterFunc::into_getter(move |zelf: &S, _vm: &VirtualMachine| (self)(zelf)) + } +} + pub trait IntoPyNoResult { fn into_noresult(self) -> PyResult<()>; } @@ -60,14 +82,11 @@ impl IntoPyNoResult for PyResult<()> { } } -pub trait IntoPySetterFunc -where - R: IntoPyNoResult, -{ +pub trait IntoPySetterFunc { fn into_setter(self) -> PySetterFunc; } -impl IntoPySetterFunc, V, R> for F +impl IntoPySetterFunc<(OwnedParam, V, R, VirtualMachine)> for F where F: Fn(T, V, &VirtualMachine) -> R + 'static, T: TryFromObject, @@ -83,7 +102,7 @@ where } } -impl IntoPySetterFunc, V, R> for F +impl IntoPySetterFunc<(RefParam, V, R, VirtualMachine)> for F where F: Fn(&S, V, &VirtualMachine) -> R + 'static, S: PyValue, @@ -99,6 +118,30 @@ where } } +impl IntoPySetterFunc<(OwnedParam, V, R)> for F +where + F: Fn(T, V) -> R + 'static, + T: TryFromObject, + V: TryFromObject, + R: IntoPyNoResult, +{ + fn into_setter(self) -> PySetterFunc { + IntoPySetterFunc::into_setter(move |obj, v, _vm: &VirtualMachine| (self)(obj, v)) + } +} + +impl IntoPySetterFunc<(RefParam, V, R)> for F +where + F: Fn(&S, V) -> R + 'static, + S: PyValue, + V: TryFromObject, + R: IntoPyNoResult, +{ + fn into_setter(self) -> PySetterFunc { + IntoPySetterFunc::into_setter(move |zelf: &S, v, _vm: &VirtualMachine| (self)(zelf, v)) + } +} + #[pyclass] pub struct PyGetSet { name: String, @@ -155,9 +198,9 @@ impl PyBuiltinDescriptor for PyGetSet { } impl PyGetSet { - pub fn with_get(name: String, getter: G) -> Self + pub fn with_get(name: String, getter: G) -> Self where - G: IntoPyGetterFunc, + G: IntoPyGetterFunc, { Self { name, @@ -166,11 +209,10 @@ impl PyGetSet { } } - pub fn with_get_set(name: String, getter: G, setter: S) -> Self + pub fn with_get_set(name: String, getter: G, setter: S) -> Self where - G: IntoPyGetterFunc, - S: IntoPySetterFunc, - SR: IntoPyNoResult, + G: IntoPyGetterFunc, + S: IntoPySetterFunc, { Self { name, diff --git a/vm/src/obj/objproperty.rs b/vm/src/obj/objproperty.rs index 52645e185f..8dd3ce0a6f 100644 --- a/vm/src/obj/objproperty.rs +++ b/vm/src/obj/objproperty.rs @@ -268,7 +268,7 @@ impl<'a> PropertyBuilder<'a> { } } - pub fn add_getter>(self, func: F) -> Self { + pub fn add_getter>(self, func: F) -> Self { let func = self.ctx.new_method(func); Self { ctx: self.ctx, From 23381b9937e6ce241eb17d7ce056483562293340 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 5 Feb 2020 03:39:19 +0900 Subject: [PATCH 06/10] compatiibility for CPytthon descr_check --- derive/src/pyclass.rs | 7 +++- tests/snippets/types_snippet.py | 4 ++ vm/src/obj/objbuiltinfunc.rs | 26 +++++++------ vm/src/obj/objclassmethod.rs | 15 ++++---- vm/src/obj/objfunction.rs | 19 +++++----- vm/src/obj/objgetset.rs | 18 +++++---- vm/src/obj/objproperty.rs | 28 +++++++------- vm/src/obj/objstaticmethod.rs | 15 ++++---- vm/src/obj/objtype.rs | 8 +++- vm/src/obj/objweakref.rs | 6 +-- vm/src/slots.rs | 65 +++++++++++++++++++++++++++++---- vm/src/vm.rs | 9 ++++- 12 files changed, 151 insertions(+), 69 deletions(-) diff --git a/derive/src/pyclass.rs b/derive/src/pyclass.rs index 5265bdd739..7a75785a86 100644 --- a/derive/src/pyclass.rs +++ b/derive/src/pyclass.rs @@ -452,8 +452,13 @@ fn extract_impl_items(mut items: Vec) -> Result { + let transform = if vec!["new", "call"].contains(&slot_ident.to_string().as_str()) { + quote! { ::rustpython_vm::function::IntoPyNativeFunc::into_func } + } else { + quote! { Box::new } + }; let into_func = quote_spanned! {item_ident.span()=> - ::rustpython_vm::function::IntoPyNativeFunc::into_func(Self::#item_ident) + #transform(Self::#item_ident) }; Some(quote! { (*class.slots.borrow_mut()).#slot_ident = Some(#into_func); diff --git a/tests/snippets/types_snippet.py b/tests/snippets/types_snippet.py index 230c2eba29..81ee9ef732 100644 --- a/tests/snippets/types_snippet.py +++ b/tests/snippets/types_snippet.py @@ -86,3 +86,7 @@ class C(B, BB): pass assert C.mro() == [C, B, A, BB, AA, object] + + +assert type(Exception.args).__name__ == 'getset_descriptor' +assert type(None).__bool__(None) is False diff --git a/vm/src/obj/objbuiltinfunc.rs b/vm/src/obj/objbuiltinfunc.rs index 124eb4099a..767e6294ec 100644 --- a/vm/src/obj/objbuiltinfunc.rs +++ b/vm/src/obj/objbuiltinfunc.rs @@ -3,9 +3,9 @@ use std::fmt; use crate::function::{OptionalArg, PyFuncArgs, PyNativeFunc}; use crate::obj::objtype::PyClassRef; use crate::pyobject::{ - IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, + IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyResult, PyValue, TypeProtocol, }; -use crate::slots::{PyBuiltinCallable, PyBuiltinDescriptor}; +use crate::slots::{SlotCall, SlotDescriptor}; use crate::vm::VirtualMachine; #[pyclass] @@ -35,13 +35,13 @@ impl PyBuiltinFunction { } } -impl PyBuiltinCallable for PyBuiltinFunction { +impl SlotCall for PyBuiltinFunction { fn call(&self, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { (self.value)(vm, args) } } -#[pyimpl(with(PyBuiltinCallable))] +#[pyimpl(with(SlotCall))] impl PyBuiltinFunction {} #[pyclass] @@ -73,13 +73,17 @@ impl PyBuiltinMethod { } } -impl PyBuiltinDescriptor for PyBuiltinMethod { - fn get( - zelf: PyRef, - obj: PyObjectRef, - cls: OptionalArg, +impl SlotDescriptor for PyBuiltinMethod { + fn descr_get( vm: &VirtualMachine, + zelf: PyObjectRef, + obj: Option, + cls: OptionalArg, ) -> PyResult { + let (zelf, obj) = match Self::_check(zelf, obj, vm) { + Ok(obj) => obj, + Err(result) => return result, + }; if obj.is(&vm.get_none()) && !Self::_cls_is(&cls, &obj.class()) { Ok(zelf.into_object()) } else { @@ -88,13 +92,13 @@ impl PyBuiltinDescriptor for PyBuiltinMethod { } } -impl PyBuiltinCallable for PyBuiltinMethod { +impl SlotCall for PyBuiltinMethod { fn call(&self, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { (self.function.value)(vm, args) } } -#[pyimpl(with(PyBuiltinDescriptor, PyBuiltinCallable))] +#[pyimpl(with(SlotDescriptor, SlotCall))] impl PyBuiltinMethod {} pub fn init(context: &PyContext) { diff --git a/vm/src/obj/objclassmethod.rs b/vm/src/obj/objclassmethod.rs index 05505b9c79..8c3d5070d2 100644 --- a/vm/src/obj/objclassmethod.rs +++ b/vm/src/obj/objclassmethod.rs @@ -3,7 +3,7 @@ use crate::function::OptionalArg; use crate::pyobject::{ PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, }; -use crate::slots::PyBuiltinDescriptor; +use crate::slots::SlotDescriptor; use crate::vm::VirtualMachine; /// classmethod(function) -> method @@ -47,19 +47,20 @@ impl PyValue for PyClassMethod { } } -impl PyBuiltinDescriptor for PyClassMethod { - fn get( - zelf: PyRef, - obj: PyObjectRef, - cls: OptionalArg, +impl SlotDescriptor for PyClassMethod { + fn descr_get( vm: &VirtualMachine, + zelf: PyObjectRef, + obj: Option, + cls: OptionalArg, ) -> PyResult { + let (zelf, obj) = Self::_unwrap(zelf, obj, vm)?; let cls = cls.unwrap_or_else(|| obj.class().into_object()); Ok(vm.ctx.new_bound_method(zelf.callable.clone(), cls)) } } -#[pyimpl(with(PyBuiltinDescriptor), flags(BASETYPE))] +#[pyimpl(with(SlotDescriptor), flags(BASETYPE))] impl PyClassMethod { #[pyslot] fn tp_new( diff --git a/vm/src/obj/objfunction.rs b/vm/src/obj/objfunction.rs index f3b0e1fdb3..24371e85cb 100644 --- a/vm/src/obj/objfunction.rs +++ b/vm/src/obj/objfunction.rs @@ -13,7 +13,7 @@ use crate::pyobject::{ TypeProtocol, }; use crate::scope::Scope; -use crate::slots::{PyBuiltinCallable, PyBuiltinDescriptor}; +use crate::slots::{SlotCall, SlotDescriptor}; use crate::vm::VirtualMachine; pub type PyFunctionRef = PyRef; @@ -27,13 +27,14 @@ pub struct PyFunction { kw_only_defaults: Option, } -impl PyBuiltinDescriptor for PyFunction { - fn get( - zelf: PyRef, - obj: PyObjectRef, - cls: OptionalArg, +impl SlotDescriptor for PyFunction { + fn descr_get( vm: &VirtualMachine, + zelf: PyObjectRef, + obj: Option, + cls: OptionalArg, ) -> PyResult { + let (zelf, obj) = Self::_unwrap(zelf, obj, vm)?; if obj.is(&vm.get_none()) && !Self::_cls_is(&cls, &obj.class()) { Ok(zelf.into_object()) } else { @@ -240,7 +241,7 @@ impl PyValue for PyFunction { } } -#[pyimpl(with(PyBuiltinDescriptor))] +#[pyimpl(with(SlotDescriptor))] impl PyFunction { #[pyslot] #[pymethod(magic)] @@ -272,7 +273,7 @@ pub struct PyBoundMethod { pub function: PyObjectRef, } -impl PyBuiltinCallable for PyBoundMethod { +impl SlotCall for PyBoundMethod { fn call(&self, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { let args = args.insert(self.object.clone()); vm.invoke(&self.function, args) @@ -285,7 +286,7 @@ impl PyBoundMethod { } } -#[pyimpl(with(PyBuiltinCallable))] +#[pyimpl(with(SlotCall))] impl PyBoundMethod { #[pymethod(magic)] fn getattribute(&self, name: PyStringRef, vm: &VirtualMachine) -> PyResult { diff --git a/vm/src/obj/objgetset.rs b/vm/src/obj/objgetset.rs index 13dadd536d..20e89e09f4 100644 --- a/vm/src/obj/objgetset.rs +++ b/vm/src/obj/objgetset.rs @@ -6,7 +6,7 @@ use crate::function::{OptionalArg, OwnedParam, RefParam}; use crate::pyobject::{ IntoPyObject, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, }; -use crate::slots::PyBuiltinDescriptor; +use crate::slots::SlotDescriptor; use crate::vm::VirtualMachine; pub type PyGetterFunc = Box PyResult>; @@ -178,13 +178,17 @@ impl PyValue for PyGetSet { pub type PyGetSetRef = PyRef; -impl PyBuiltinDescriptor for PyGetSet { - fn get( - zelf: PyRef, - obj: PyObjectRef, - _cls: OptionalArg, +impl SlotDescriptor for PyGetSet { + fn descr_get( vm: &VirtualMachine, + zelf: PyObjectRef, + obj: Option, + _cls: OptionalArg, ) -> PyResult { + let (zelf, obj) = match Self::_check(zelf, obj, vm) { + Ok(obj) => obj, + Err(result) => return result, + }; if let Some(ref f) = zelf.getter { f(vm, obj) } else { @@ -222,7 +226,7 @@ impl PyGetSet { } } -#[pyimpl(with(PyBuiltinDescriptor))] +#[pyimpl(with(SlotDescriptor))] impl PyGetSet { // Descriptor methods diff --git a/vm/src/obj/objproperty.rs b/vm/src/obj/objproperty.rs index 8dd3ce0a6f..cc945f96bd 100644 --- a/vm/src/obj/objproperty.rs +++ b/vm/src/obj/objproperty.rs @@ -9,7 +9,7 @@ use crate::pyobject::{ IdProtocol, PyClassImpl, PyContext, PyObject, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, }; -use crate::slots::PyBuiltinDescriptor; +use crate::slots::SlotDescriptor; use crate::vm::VirtualMachine; // Read-only property, doesn't have __set__ or __delete__ @@ -27,13 +27,14 @@ impl PyValue for PyReadOnlyProperty { pub type PyReadOnlyPropertyRef = PyRef; -impl PyBuiltinDescriptor for PyReadOnlyProperty { - fn get( - zelf: PyRef, - obj: PyObjectRef, - cls: OptionalArg, +impl SlotDescriptor for PyReadOnlyProperty { + fn descr_get( vm: &VirtualMachine, + zelf: PyObjectRef, + obj: Option, + cls: OptionalArg, ) -> PyResult { + let (zelf, obj) = Self::_unwrap(zelf, obj, vm)?; if vm.is_none(&obj) { if Self::_cls_is(&cls, &vm.ctx.types.type_type) { vm.invoke(&zelf.getter, cls.unwrap()) @@ -46,7 +47,7 @@ impl PyBuiltinDescriptor for PyReadOnlyProperty { } } -#[pyimpl(with(PyBuiltinDescriptor))] +#[pyimpl(with(SlotDescriptor))] impl PyReadOnlyProperty {} /// Property attribute. @@ -110,13 +111,14 @@ struct PropertyArgs { doc: Option, } -impl PyBuiltinDescriptor for PyProperty { - fn get( - zelf: PyRef, - obj: PyObjectRef, - _cls: OptionalArg, +impl SlotDescriptor for PyProperty { + fn descr_get( vm: &VirtualMachine, + zelf: PyObjectRef, + obj: Option, + _cls: OptionalArg, ) -> PyResult { + let (zelf, obj) = Self::_unwrap(zelf, obj, vm)?; if let Some(getter) = zelf.getter.as_ref() { if obj.is(vm.ctx.none.as_object()) { Ok(zelf.into_object()) @@ -129,7 +131,7 @@ impl PyBuiltinDescriptor for PyProperty { } } -#[pyimpl(with(PyBuiltinDescriptor), flags(BASETYPE))] +#[pyimpl(with(SlotDescriptor), flags(BASETYPE))] impl PyProperty { #[pyslot] fn tp_new(cls: PyClassRef, args: PropertyArgs, vm: &VirtualMachine) -> PyResult { diff --git a/vm/src/obj/objstaticmethod.rs b/vm/src/obj/objstaticmethod.rs index eaaf86159d..fbf451064d 100644 --- a/vm/src/obj/objstaticmethod.rs +++ b/vm/src/obj/objstaticmethod.rs @@ -1,7 +1,7 @@ use super::objtype::PyClassRef; use crate::function::OptionalArg; use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue}; -use crate::slots::PyBuiltinDescriptor; +use crate::slots::SlotDescriptor; use crate::vm::VirtualMachine; #[pyclass(name = "staticmethod")] @@ -17,18 +17,19 @@ impl PyValue for PyStaticMethod { } } -impl PyBuiltinDescriptor for PyStaticMethod { - fn get( - zelf: PyRef, - _obj: PyObjectRef, +impl SlotDescriptor for PyStaticMethod { + fn descr_get( + vm: &VirtualMachine, + zelf: PyObjectRef, + _obj: Option, _cls: OptionalArg, - _vm: &VirtualMachine, ) -> PyResult { + let zelf = Self::_zelf(zelf, vm)?; Ok(zelf.callable.clone()) } } -#[pyimpl(with(PyBuiltinDescriptor), flags(BASETYPE))] +#[pyimpl(with(SlotDescriptor), flags(BASETYPE))] impl PyStaticMethod { #[pyslot] fn tp_new( diff --git a/vm/src/obj/objtype.rs b/vm/src/obj/objtype.rs index b4a0169417..5ca39913fc 100644 --- a/vm/src/obj/objtype.rs +++ b/vm/src/obj/objtype.rs @@ -9,7 +9,7 @@ use super::objproperty::PropertyBuilder; use super::objstr::PyStringRef; use super::objtuple::PyTuple; use super::objweakref::PyWeak; -use crate::function::PyFuncArgs; +use crate::function::{OptionalArg, PyFuncArgs}; use crate::pyobject::{ IdProtocol, PyAttributes, PyClassImpl, PyContext, PyIterable, PyObject, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, @@ -170,7 +170,11 @@ impl PyClassRef { if let Some(attr) = self.get_attr(&name) { let attr_class = attr.class(); - if let Some(ref descriptor) = attr_class.get_attr("__get__") { + let slots = attr_class.slots.borrow(); + if let Some(ref descr_get) = slots.descr_get { + return descr_get(vm, attr, None, OptionalArg::Present(self.into_object())); + } else if let Some(ref descriptor) = attr_class.get_attr("__get__") { + // TODO: is this nessessary? return vm.invoke(descriptor, vec![attr, vm.get_none(), self.into_object()]); } } diff --git a/vm/src/obj/objweakref.rs b/vm/src/obj/objweakref.rs index 436a4a5135..c74bcdfe02 100644 --- a/vm/src/obj/objweakref.rs +++ b/vm/src/obj/objweakref.rs @@ -3,7 +3,7 @@ use crate::function::{OptionalArg, PyFuncArgs}; use crate::pyobject::{ PyClassImpl, PyContext, PyObject, PyObjectPayload, PyObjectRef, PyRef, PyResult, PyValue, }; -use crate::slots::PyBuiltinCallable; +use crate::slots::SlotCall; use crate::vm::VirtualMachine; use std::rc::{Rc, Weak}; @@ -34,14 +34,14 @@ impl PyValue for PyWeak { pub type PyWeakRef = PyRef; -impl PyBuiltinCallable for PyWeak { +impl SlotCall for PyWeak { fn call(&self, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { args.bind::<()>(vm)?; Ok(self.referent.upgrade().unwrap_or_else(|| vm.get_none())) } } -#[pyimpl(with(PyBuiltinCallable), flags(BASETYPE))] +#[pyimpl(with(SlotCall), flags(BASETYPE))] impl PyWeak { // TODO callbacks #[pyslot] diff --git a/vm/src/slots.rs b/vm/src/slots.rs index 818a1b974b..b673a5d5ed 100644 --- a/vm/src/slots.rs +++ b/vm/src/slots.rs @@ -1,5 +1,5 @@ use crate::function::{OptionalArg, PyFuncArgs, PyNativeFunc}; -use crate::pyobject::{IdProtocol, PyObjectRef, PyRef, PyResult, PyValue}; +use crate::pyobject::{IdProtocol, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject}; use crate::VirtualMachine; bitflags! { @@ -28,7 +28,7 @@ pub struct PyClassSlots { pub flags: PyTpFlags, pub new: Option, pub call: Option, - pub descr_get: Option, + pub descr_get: Option, } impl std::fmt::Debug for PyClassSlots { @@ -38,22 +38,73 @@ impl std::fmt::Debug for PyClassSlots { } #[pyimpl] -pub trait PyBuiltinCallable: PyValue { +pub trait SlotCall: PyValue { #[pymethod(magic)] #[pyslot] fn call(&self, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult; } +pub type PyDescrGetFunc = Box< + dyn Fn(&VirtualMachine, PyObjectRef, Option, OptionalArg) -> PyResult, +>; + #[pyimpl] -pub trait PyBuiltinDescriptor: PyValue { +pub trait SlotDescriptor: PyValue { + #[pyslot] + fn descr_get( + vm: &VirtualMachine, + zelf: PyObjectRef, + obj: Option, + cls: OptionalArg, + ) -> PyResult; + #[pymethod(magic)] - #[pyslot(descr_get)] fn get( - zelf: PyRef, + zelf: PyObjectRef, obj: PyObjectRef, cls: OptionalArg, vm: &VirtualMachine, - ) -> PyResult; + ) -> PyResult { + Self::descr_get(vm, zelf, Some(obj), cls) + } + + fn _zelf(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult> { + PyRef::::try_from_object(vm, zelf) + } + + fn _unwrap( + zelf: PyObjectRef, + obj: Option, + vm: &VirtualMachine, + ) -> PyResult<(PyRef, PyObjectRef)> { + let zelf = Self::_zelf(zelf, vm)?; + let obj = obj.unwrap_or_else(|| vm.get_none()); + Ok((zelf, obj)) + } + + fn _check( + zelf: PyObjectRef, + obj: Option, + vm: &VirtualMachine, + ) -> Result<(PyRef, PyObjectRef), PyResult> { + // CPython descr_check + if let Some(obj) = obj { + // if (!PyObject_TypeCheck(obj, descr->d_type)) { + // PyErr_Format(PyExc_TypeError, + // "descriptor '%V' for '%.100s' objects " + // "doesn't apply to a '%.100s' object", + // descr_name((PyDescrObject *)descr), "?", + // descr->d_type->tp_name, + // obj->ob_type->tp_name); + // *pres = NULL; + // return 1; + // } else { + Ok((Self::_zelf(zelf, vm).unwrap(), obj)) + // } + } else { + Err(Ok(zelf)) + } + } fn _cls_is(cls: &OptionalArg, other: &T) -> bool where diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 8d573327da..ca6cce10da 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -24,7 +24,7 @@ use crate::bytecode; use crate::exceptions::{PyBaseException, PyBaseExceptionRef}; use crate::frame::{ExecutionResult, Frame, FrameRef}; use crate::frozen; -use crate::function::PyFuncArgs; +use crate::function::{OptionalArg, PyFuncArgs}; use crate::import; use crate::obj::objbool; use crate::obj::objcode::{PyCode, PyCodeRef}; @@ -623,7 +623,12 @@ impl VirtualMachine { let slots = attr_class.slots.borrow(); if let Some(descr_get) = slots.borrow().descr_get.as_ref() { let cls = obj.class(); - descr_get(self, vec![attr, obj.clone(), cls.into_object()].into()) + descr_get( + self, + attr, + Some(obj.clone()), + OptionalArg::Present(cls.into_object()), + ) } else if let Some(ref descriptor) = attr_class.get_attr("__get__") { let cls = obj.class(); self.invoke(descriptor, vec![attr, obj.clone(), cls.into_object()]) From 0aee78de18b3286b522fcc8d4d2d5f6301d98d93 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 4 Feb 2020 04:45:04 +0900 Subject: [PATCH 07/10] pyproperty generates PyGetSet instead of PyProperty --- derive/src/pyclass.rs | 17 ++++++++++------- vm/src/exceptions.rs | 16 +++++++++++++--- vm/src/obj/objfloat.rs | 4 ++-- vm/src/obj/objfunction.rs | 6 +++--- vm/src/obj/objint.rs | 1 + vm/src/obj/objtype.rs | 7 +++++++ 6 files changed, 36 insertions(+), 15 deletions(-) diff --git a/derive/src/pyclass.rs b/derive/src/pyclass.rs index 7a75785a86..b7bdff2c2d 100644 --- a/derive/src/pyclass.rs +++ b/derive/src/pyclass.rs @@ -405,8 +405,8 @@ fn extract_impl_items(mut items: Vec) -> Result getter, + let getter_func = match prop.0 { + Some(func) => func, None => { push_err_span!( diagnostics, @@ -417,14 +417,17 @@ fn extract_impl_items(mut items: Vec) -> Result (quote! { with_get_set }, quote! { , &Self::#func }), + None => (quote! { with_get }, quote! { }), + }; + let str_name = name.to_string(); quote! { class.set_str_attr( #name, - ::rustpython_vm::obj::objproperty::PropertyBuilder::new(ctx) - .add_getter(Self::#getter) - #add_setter - .create(), + ::rustpython_vm::pyobject::PyObject::new( + ::rustpython_vm::obj::objgetset::PyGetSet::#new(#str_name.into(), &Self::#getter_func #setter), + ctx.getset_type(), None) ); } }) diff --git a/vm/src/exceptions.rs b/vm/src/exceptions.rs index 135cb0d6f4..d30a2ed230 100644 --- a/vm/src/exceptions.rs +++ b/vm/src/exceptions.rs @@ -80,7 +80,7 @@ impl PyBaseException { } #[pyproperty(name = "__traceback__")] - fn get_traceback(&self, _vm: &VirtualMachine) -> Option { + fn get_traceback(&self) -> Option { self.traceback.borrow().clone() } @@ -95,8 +95,13 @@ impl PyBaseException { } #[pyproperty(name = "__cause__", setter)] - fn setter_cause(&self, cause: Option, _vm: &VirtualMachine) { + fn setter_cause( + &self, + cause: Option, + _vm: &VirtualMachine, + ) -> PyResult<()> { self.cause.replace(cause); + Ok(()) } #[pyproperty(name = "__context__")] @@ -105,8 +110,13 @@ impl PyBaseException { } #[pyproperty(name = "__context__", setter)] - fn setter_context(&self, context: Option, _vm: &VirtualMachine) { + fn setter_context( + &self, + context: Option, + _vm: &VirtualMachine, + ) -> PyResult<()> { self.context.replace(context); + Ok(()) } #[pyproperty(name = "__suppress_context__")] diff --git a/vm/src/obj/objfloat.rs b/vm/src/obj/objfloat.rs index 2b660994b0..ffc8e946b6 100644 --- a/vm/src/obj/objfloat.rs +++ b/vm/src/obj/objfloat.rs @@ -498,12 +498,12 @@ impl PyFloat { pyhash::hash_float(self.value) } - #[pyproperty(name = "real")] + #[pyproperty] fn real(zelf: PyRef, _vm: &VirtualMachine) -> PyFloatRef { zelf } - #[pyproperty(name = "imag")] + #[pyproperty] fn imag(&self, _vm: &VirtualMachine) -> f64 { 0.0f64 } diff --git a/vm/src/obj/objfunction.rs b/vm/src/obj/objfunction.rs index 24371e85cb..d3532d01ab 100644 --- a/vm/src/obj/objfunction.rs +++ b/vm/src/obj/objfunction.rs @@ -249,17 +249,17 @@ impl PyFunction { self.invoke(args, vm) } - #[pyproperty(name = "__code__")] + #[pyproperty(magic)] fn code(&self, _vm: &VirtualMachine) -> PyCodeRef { self.code.clone() } - #[pyproperty(name = "__defaults__")] + #[pyproperty(magic)] fn defaults(&self, _vm: &VirtualMachine) -> Option { self.defaults.clone() } - #[pyproperty(name = "__kwdefaults__")] + #[pyproperty(magic)] fn kwdefaults(&self, _vm: &VirtualMachine) -> Option { self.kw_only_defaults.clone() } diff --git a/vm/src/obj/objint.rs b/vm/src/obj/objint.rs index acb2cc70cc..3af40eef9f 100644 --- a/vm/src/obj/objint.rs +++ b/vm/src/obj/objint.rs @@ -626,6 +626,7 @@ impl PyInt { } #[pyproperty] fn real(&self, vm: &VirtualMachine) -> PyObjectRef { + // subclasses must return int here vm.ctx.new_bigint(&self.value) } diff --git a/vm/src/obj/objtype.rs b/vm/src/obj/objtype.rs index 5ca39913fc..7d5cafaeab 100644 --- a/vm/src/obj/objtype.rs +++ b/vm/src/obj/objtype.rs @@ -145,6 +145,13 @@ impl PyClassRef { .unwrap_or_else(|| vm.ctx.new_str("builtins".to_owned())) } + #[pyproperty(magic, setter)] + fn set_module(self, value: PyObjectRef) { + self.attributes + .borrow_mut() + .insert("__module__".to_owned(), value); + } + #[pymethod(magic)] fn prepare(_name: PyStringRef, _bases: PyObjectRef, vm: &VirtualMachine) -> PyDictRef { vm.ctx.new_dict() From facabfee1ac0ca5f5e2337a2e2aff253ce3810ee Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 6 Feb 2020 01:22:50 +0900 Subject: [PATCH 08/10] Remove PropertyBuilder and add new_getset --- derive/src/pyclass.rs | 3 +- vm/src/exceptions.rs | 40 ++++++------- vm/src/obj/objcode.rs | 22 +++---- vm/src/obj/objproperty.rs | 116 ++---------------------------------- vm/src/obj/objtype.rs | 46 +++++--------- vm/src/pyobject.rs | 20 +++++-- vm/src/stdlib/io.rs | 12 ++-- vm/src/stdlib/os.rs | 4 +- vm/src/stdlib/pwd.rs | 14 ++--- vm/src/stdlib/subprocess.rs | 12 ++-- 10 files changed, 86 insertions(+), 203 deletions(-) diff --git a/derive/src/pyclass.rs b/derive/src/pyclass.rs index b7bdff2c2d..b5894ce9b9 100644 --- a/derive/src/pyclass.rs +++ b/derive/src/pyclass.rs @@ -692,7 +692,8 @@ pub fn impl_pystruct_sequence(attr: AttributeArgs, item: Item) -> Result ctx.new_property(make_arg_getter(0)), - "filename" => ctx.new_property(make_arg_getter(1)), - "lineno" => ctx.new_property(make_arg_getter(2)), - "offset" => ctx.new_property(make_arg_getter(3)), - "text" => ctx.new_property(make_arg_getter(4)), + "msg" => ctx.new_readonly_getset("msg", make_arg_getter(0)), }); extend_class!(ctx, &excs.import_error, { "__init__" => ctx.new_method(import_error_init), - "msg" => ctx.new_property(make_arg_getter(0)), + "msg" => ctx.new_readonly_getset("msg", make_arg_getter(0)), }); extend_class!(ctx, &excs.stop_iteration, { - "value" => ctx.new_property(make_arg_getter(0)), + "value" => ctx.new_readonly_getset("value", make_arg_getter(0)), }); extend_class!(ctx, &excs.key_error, { @@ -665,27 +661,27 @@ pub fn init(ctx: &PyContext) { }); extend_class!(ctx, &excs.unicode_decode_error, { - "encoding" => ctx.new_property(make_arg_getter(0)), - "object" => ctx.new_property(make_arg_getter(1)), - "start" => ctx.new_property(make_arg_getter(2)), - "end" => ctx.new_property(make_arg_getter(3)), - "reason" => ctx.new_property(make_arg_getter(4)), + "encoding" => ctx.new_readonly_getset("encoding", make_arg_getter(0)), + "object" => ctx.new_readonly_getset("object", make_arg_getter(1)), + "start" => ctx.new_readonly_getset("start", make_arg_getter(2)), + "end" => ctx.new_readonly_getset("end", make_arg_getter(3)), + "reason" => ctx.new_readonly_getset("reason", make_arg_getter(4)), }); extend_class!(ctx, &excs.unicode_encode_error, { - "encoding" => ctx.new_property(make_arg_getter(0)), - "object" => ctx.new_property(make_arg_getter(1)), - "start" => ctx.new_property(make_arg_getter(2)), - "end" => ctx.new_property(make_arg_getter(3)), - "reason" => ctx.new_property(make_arg_getter(4)), + "encoding" => ctx.new_readonly_getset("encoding", make_arg_getter(0)), + "object" => ctx.new_readonly_getset("object", make_arg_getter(1)), + "start" => ctx.new_readonly_getset("start", make_arg_getter(2)), + "end" => ctx.new_readonly_getset("end", make_arg_getter(3)), + "reason" => ctx.new_readonly_getset("reason", make_arg_getter(4)), }); extend_class!(ctx, &excs.unicode_translate_error, { - "encoding" => ctx.new_property(none_getter), - "object" => ctx.new_property(make_arg_getter(0)), - "start" => ctx.new_property(make_arg_getter(1)), - "end" => ctx.new_property(make_arg_getter(2)), - "reason" => ctx.new_property(make_arg_getter(3)), + "encoding" => ctx.new_readonly_getset("encoding", none_getter), + "object" => ctx.new_readonly_getset("object", make_arg_getter(0)), + "start" => ctx.new_readonly_getset("start", make_arg_getter(1)), + "end" => ctx.new_readonly_getset("end", make_arg_getter(2)), + "reason" => ctx.new_readonly_getset("reason", make_arg_getter(3)), }); } diff --git a/vm/src/obj/objcode.rs b/vm/src/obj/objcode.rs index cfb634c8b1..b84e50c0e8 100644 --- a/vm/src/obj/objcode.rs +++ b/vm/src/obj/objcode.rs @@ -92,17 +92,17 @@ impl PyCodeRef { } } -pub fn init(context: &PyContext) { - extend_class!(context, &context.types.code_type, { +pub fn init(ctx: &PyContext) { + extend_class!(ctx, &ctx.types.code_type, { (slot new) => PyCodeRef::new, - "__repr__" => context.new_method(PyCodeRef::repr), - - "co_argcount" => context.new_property(PyCodeRef::co_argcount), - "co_consts" => context.new_property(PyCodeRef::co_consts), - "co_filename" => context.new_property(PyCodeRef::co_filename), - "co_firstlineno" => context.new_property(PyCodeRef::co_firstlineno), - "co_kwonlyargcount" => context.new_property(PyCodeRef::co_kwonlyargcount), - "co_name" => context.new_property(PyCodeRef::co_name), - "co_flags" => context.new_property(PyCodeRef::co_flags), + "__repr__" => ctx.new_method(PyCodeRef::repr), + + "co_argcount" => ctx.new_readonly_getset("co_argcount", PyCodeRef::co_argcount), + "co_consts" => ctx.new_readonly_getset("co_consts", PyCodeRef::co_consts), + "co_filename" => ctx.new_readonly_getset("co_filename", PyCodeRef::co_filename), + "co_firstlineno" => ctx.new_readonly_getset("co_firstlineno", PyCodeRef::co_firstlineno), + "co_kwonlyargcount" => ctx.new_readonly_getset("co_kwonlyargcount", PyCodeRef::co_kwonlyargcount), + "co_name" => ctx.new_readonly_getset("co_name", PyCodeRef::co_name), + "co_flags" => ctx.new_readonly_getset("co_flags", PyCodeRef::co_flags), }); } diff --git a/vm/src/obj/objproperty.rs b/vm/src/obj/objproperty.rs index cc945f96bd..7bc683bc27 100644 --- a/vm/src/obj/objproperty.rs +++ b/vm/src/obj/objproperty.rs @@ -4,52 +4,13 @@ use std::cell::RefCell; use super::objtype::PyClassRef; -use crate::function::{IntoPyNativeFunc, OptionalArg}; +use crate::function::OptionalArg; use crate::pyobject::{ - IdProtocol, PyClassImpl, PyContext, PyObject, PyObjectRef, PyRef, PyResult, PyValue, - TypeProtocol, + IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, }; use crate::slots::SlotDescriptor; use crate::vm::VirtualMachine; -// Read-only property, doesn't have __set__ or __delete__ -#[pyclass] -#[derive(Debug)] -pub struct PyReadOnlyProperty { - getter: PyObjectRef, -} - -impl PyValue for PyReadOnlyProperty { - fn class(vm: &VirtualMachine) -> PyClassRef { - vm.ctx.readonly_property_type() - } -} - -pub type PyReadOnlyPropertyRef = PyRef; - -impl SlotDescriptor for PyReadOnlyProperty { - fn descr_get( - vm: &VirtualMachine, - zelf: PyObjectRef, - obj: Option, - cls: OptionalArg, - ) -> PyResult { - let (zelf, obj) = Self::_unwrap(zelf, obj, vm)?; - if vm.is_none(&obj) { - if Self::_cls_is(&cls, &vm.ctx.types.type_type) { - vm.invoke(&zelf.getter, cls.unwrap()) - } else { - Ok(zelf.into_object()) - } - } else { - vm.invoke(&zelf.getter, obj) - } - } -} - -#[pyimpl(with(SlotDescriptor))] -impl PyReadOnlyProperty {} - /// Property attribute. /// /// fget @@ -255,81 +216,12 @@ fn py_none_to_option(vm: &VirtualMachine, value: &PyObjectRef) -> Option { - ctx: &'a PyContext, - getter: Option, - setter: Option, -} - -impl<'a> PropertyBuilder<'a> { - pub fn new(ctx: &'a PyContext) -> Self { - Self { - ctx, - getter: None, - setter: None, - } - } - - pub fn add_getter>(self, func: F) -> Self { - let func = self.ctx.new_method(func); - Self { - ctx: self.ctx, - getter: Some(func), - setter: self.setter, - } - } - - pub fn add_setter< - I, - V, - VM, - F: IntoPyNativeFunc<(I, V), impl super::objgetset::IntoPyNoResult, VM>, - >( - self, - func: F, - ) -> Self { - let func = self.ctx.new_method(func); - Self { - ctx: self.ctx, - getter: self.getter, - setter: Some(func), - } - } - - pub fn create(self) -> PyObjectRef { - if self.setter.is_some() { - let payload = PyProperty { - getter: self.getter.clone(), - setter: self.setter.clone(), - deleter: None, - doc: RefCell::new(None), - }; - - PyObject::new(payload, self.ctx.property_type(), None) - } else { - let payload = PyReadOnlyProperty { - getter: self.getter.expect( - "One of add_getter/add_setter must be called when constructing a property", - ), - }; - - PyObject::new(payload, self.ctx.readonly_property_type(), None) - } - } -} - -pub fn init(context: &PyContext) { - PyReadOnlyProperty::extend_class(context, &context.types.readonly_property_type); - +pub(crate) fn init(context: &PyContext) { PyProperty::extend_class(context, &context.types.property_type); // This is a bit unfortunate, but this instance attribute overlaps with the // class __doc__ string.. extend_class!(context, &context.types.property_type, { - "__doc__" => - PropertyBuilder::new(context) - .add_getter(PyProperty::doc_getter) - .add_setter(PyProperty::doc_setter) - .create(), + "__doc__" => context.new_getset("__doc__", PyProperty::doc_getter, PyProperty::doc_setter), }); } diff --git a/vm/src/obj/objtype.rs b/vm/src/obj/objtype.rs index 7d5cafaeab..4a4a6b6d32 100644 --- a/vm/src/obj/objtype.rs +++ b/vm/src/obj/objtype.rs @@ -5,7 +5,6 @@ use std::fmt; use super::objdict::PyDictRef; use super::objlist::PyList; use super::objmappingproxy::PyMappingProxy; -use super::objproperty::PropertyBuilder; use super::objstr::PyStringRef; use super::objtuple::PyTuple; use super::objweakref::PyWeak; @@ -80,16 +79,13 @@ impl PyClassRef { } } - fn _mro(self, _vm: &VirtualMachine) -> PyTuple { + #[pyproperty(name = "__mro__")] + fn get_mro(self, _vm: &VirtualMachine) -> PyTuple { let elements: Vec = _mro(&self).iter().map(|x| x.as_object().clone()).collect(); PyTuple::from(elements) } - fn _set_mro(self, _value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - Err(vm.new_attribute_error("read-only attribute".to_owned())) - } - #[pyproperty(magic)] fn bases(self, vm: &VirtualMachine) -> PyObjectRef { vm.ctx @@ -338,6 +334,18 @@ impl PyClassRef { } Ok(obj) } + + #[pyproperty(magic)] + fn dict(self) -> PyMappingProxy { + PyMappingProxy::new(self) + } + + #[pyproperty(magic, setter)] + fn set_dict(self, _value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + Err(vm.new_not_implemented_error( + "Setting __dict__ attribute on a type isn't yet implemented".to_owned(), + )) + } } /* @@ -346,18 +354,6 @@ impl PyClassRef { pub(crate) fn init(ctx: &PyContext) { PyClassRef::extend_class(ctx, &ctx.types.type_type); - extend_class!(&ctx, &ctx.types.type_type, { - "__dict__" => - PropertyBuilder::new(ctx) - .add_getter(type_dict) - .add_setter(type_dict_setter) - .create(), - "__mro__" => - PropertyBuilder::new(ctx) - .add_getter(PyClassRef::_mro) - .add_setter(PyClassRef::_set_mro) - .create(), - }); } fn _mro(cls: &PyClassRef) -> Vec { @@ -409,20 +405,6 @@ pub fn type_new( new(vm, args.insert(cls.into_object())) } -fn type_dict(class: PyClassRef, _vm: &VirtualMachine) -> PyMappingProxy { - PyMappingProxy::new(class) -} - -fn type_dict_setter( - _instance: PyClassRef, - _value: PyObjectRef, - vm: &VirtualMachine, -) -> PyResult<()> { - Err(vm.new_not_implemented_error( - "Setting __dict__ attribute on a type isn't yet implemented".to_owned(), - )) -} - impl PyClassRef { /// This is the internal get_attr implementation for fast lookup on a class. pub fn get_attr(&self, attr_name: &str) -> Option { diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 555d480bd4..b42be1ba80 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -25,13 +25,13 @@ use crate::obj::objcomplex::PyComplex; use crate::obj::objdict::{PyDict, PyDictRef}; use crate::obj::objfloat::PyFloat; use crate::obj::objfunction::{PyBoundMethod, PyFunction}; +use crate::obj::objgetset::{IntoPyGetterFunc, IntoPySetterFunc, PyGetSet}; use crate::obj::objint::{PyInt, PyIntRef}; use crate::obj::objiter; use crate::obj::objlist::PyList; use crate::obj::objnamespace::PyNamespace; use crate::obj::objnone::{PyNone, PyNoneRef}; use crate::obj::objobject; -use crate::obj::objproperty::PropertyBuilder; use crate::obj::objset::PySet; use crate::obj::objstr; use crate::obj::objtuple::{PyTuple, PyTupleRef}; @@ -503,11 +503,23 @@ impl PyContext { ) } - pub fn new_property(&self, f: F) -> PyObjectRef + pub fn new_readonly_getset(&self, name: impl Into, f: F) -> PyObjectRef where - F: IntoPyNativeFunc, + F: IntoPyGetterFunc, { - PropertyBuilder::new(self).add_getter(f).create() + PyObject::new(PyGetSet::with_get(name.into(), f), self.getset_type(), None) + } + + pub fn new_getset(&self, name: impl Into, g: G, s: S) -> PyObjectRef + where + G: IntoPyGetterFunc, + S: IntoPySetterFunc, + { + PyObject::new( + PyGetSet::with_get_set(name.into(), g, s), + self.getset_type(), + None, + ) } pub fn new_code_object(&self, code: bytecode::CodeObject) -> PyCodeRef { diff --git a/vm/src/stdlib/io.rs b/vm/src/stdlib/io.rs index 378fe5f4f1..cc43860e24 100644 --- a/vm/src/stdlib/io.rs +++ b/vm/src/stdlib/io.rs @@ -564,7 +564,7 @@ mod fileio { vm.set_attr(&file_io, "name", name)?; vm.set_attr(&file_io, "__fileno", vm.new_int(file_no))?; vm.set_attr(&file_io, "closefd", vm.new_bool(false))?; - vm.set_attr(&file_io, "closed", vm.new_bool(false))?; + vm.set_attr(&file_io, "__closed", vm.new_bool(false))?; Ok(vm.get_none()) } @@ -665,7 +665,7 @@ mod fileio { winapi::um::handleapi::CloseHandle(raw_handle as _); } vm.set_attr(&instance, "closefd", vm.new_bool(true))?; - vm.set_attr(&instance, "closed", vm.new_bool(true))?; + vm.set_attr(&instance, "__closed", vm.new_bool(true))?; Ok(()) } @@ -676,7 +676,7 @@ mod fileio { libc::close(raw_fd as _); } vm.set_attr(&instance, "closefd", vm.new_bool(true))?; - vm.set_attr(&instance, "closed", vm.new_bool(true))?; + vm.set_attr(&instance, "__closed", vm.new_bool(true))?; Ok(()) } @@ -952,7 +952,7 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { "readable" => ctx.new_method(io_base_readable), "writable" => ctx.new_method(io_base_writable), "flush" => ctx.new_method(io_base_flush), - "closed" => ctx.new_property(io_base_closed), + "closed" => ctx.new_readonly_getset("closed", io_base_closed), "__closed" => ctx.new_bool(false), "close" => ctx.new_method(io_base_close), "readline" => ctx.new_method(io_base_readline), @@ -1017,7 +1017,7 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { "tell" => ctx.new_method(PyStringIORef::tell), "readline" => ctx.new_method(PyStringIORef::readline), "truncate" => ctx.new_method(PyStringIORef::truncate), - "closed" => ctx.new_property(PyStringIORef::closed), + "closed" => ctx.new_readonly_getset("closed", PyStringIORef::closed), "close" => ctx.new_method(PyStringIORef::close), }); @@ -1033,7 +1033,7 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { "tell" => ctx.new_method(PyBytesIORef::tell), "readline" => ctx.new_method(PyBytesIORef::readline), "truncate" => ctx.new_method(PyBytesIORef::truncate), - "closed" => ctx.new_property(PyBytesIORef::closed), + "closed" => ctx.new_readonly_getset("closed", PyBytesIORef::closed), "close" => ctx.new_method(PyBytesIORef::close), }); diff --git a/vm/src/stdlib/os.rs b/vm/src/stdlib/os.rs index a6be3e5262..b96ec766d5 100644 --- a/vm/src/stdlib/os.rs +++ b/vm/src/stdlib/os.rs @@ -1235,8 +1235,8 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { ScandirIterator::extend_class(ctx, &scandir_iter); let dir_entry = py_class!(ctx, "DirEntry", ctx.object(), { - "name" => ctx.new_property(DirEntryRef::name), - "path" => ctx.new_property(DirEntryRef::path), + "name" => ctx.new_readonly_getset("name", DirEntryRef::name), + "path" => ctx.new_readonly_getset("path", DirEntryRef::path), "is_dir" => ctx.new_method(DirEntryRef::is_dir), "is_file" => ctx.new_method(DirEntryRef::is_file), "is_symlink" => ctx.new_method(DirEntryRef::is_symlink), diff --git a/vm/src/stdlib/pwd.rs b/vm/src/stdlib/pwd.rs index ecff167b84..0ad3942a49 100644 --- a/vm/src/stdlib/pwd.rs +++ b/vm/src/stdlib/pwd.rs @@ -68,13 +68,13 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { let ctx = &vm.ctx; let passwd_type = py_class!(ctx, "struct_passwd", ctx.object(), { - "pw_name" => ctx.new_property(PasswdRef::pw_name), - "pw_passwd" => ctx.new_property(PasswdRef::pw_passwd), - "pw_uid" => ctx.new_property(PasswdRef::pw_uid), - "pw_gid" => ctx.new_property(PasswdRef::pw_gid), - "pw_gecos" => ctx.new_property(PasswdRef::pw_gecos), - "pw_dir" => ctx.new_property(PasswdRef::pw_dir), - "pw_shell" => ctx.new_property(PasswdRef::pw_shell), + "pw_name" => ctx.new_readonly_getset("pw_name", PasswdRef::pw_name), + "pw_passwd" => ctx.new_readonly_getset("pw_passwd", PasswdRef::pw_passwd), + "pw_uid" => ctx.new_readonly_getset("pw_uid", PasswdRef::pw_uid), + "pw_gid" => ctx.new_readonly_getset("pw_gid", PasswdRef::pw_gid), + "pw_gecos" => ctx.new_readonly_getset("pw_gecos", PasswdRef::pw_gecos), + "pw_dir" => ctx.new_readonly_getset("pw_dir", PasswdRef::pw_dir), + "pw_shell" => ctx.new_readonly_getset("pw_shell", PasswdRef::pw_shell), }); py_module!(vm, "pwd", { diff --git a/vm/src/stdlib/subprocess.rs b/vm/src/stdlib/subprocess.rs index 78e8bfafc5..35ae5115a7 100644 --- a/vm/src/stdlib/subprocess.rs +++ b/vm/src/stdlib/subprocess.rs @@ -253,18 +253,18 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { let popen = py_class!(ctx, "Popen", ctx.object(), { (slot new) => PopenRef::new, "poll" => ctx.new_method(PopenRef::poll), - "returncode" => ctx.new_property(PopenRef::return_code), + "returncode" => ctx.new_readonly_getset("returncode", PopenRef::return_code), "wait" => ctx.new_method(PopenRef::wait), - "stdin" => ctx.new_property(PopenRef::stdin), - "stdout" => ctx.new_property(PopenRef::stdout), - "stderr" => ctx.new_property(PopenRef::stderr), + "stdin" => ctx.new_readonly_getset("stdin", PopenRef::stdin), + "stdout" => ctx.new_readonly_getset("stdout", PopenRef::stdout), + "stderr" => ctx.new_readonly_getset("stderr", PopenRef::stderr), "terminate" => ctx.new_method(PopenRef::terminate), "kill" => ctx.new_method(PopenRef::kill), "communicate" => ctx.new_method(PopenRef::communicate), - "pid" => ctx.new_property(PopenRef::pid), + "pid" => ctx.new_readonly_getset("pid", PopenRef::pid), "__enter__" => ctx.new_method(PopenRef::enter), "__exit__" => ctx.new_method(PopenRef::exit), - "args" => ctx.new_property(PopenRef::args), + "args" => ctx.new_readonly_getset("args", PopenRef::args), }); py_module!(vm, "_subprocess", { From 58744df1d594fd06a8211b9c3c1014514b1ed230 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sat, 1 Feb 2020 22:59:45 +0900 Subject: [PATCH 09/10] Revert 08e66b5002f3a3a30db72b350b18c61d367ef1fc which is not required anymore --- vm/src/obj/objnone.rs | 65 +-------------------------------------- vm/src/obj/objproperty.rs | 9 ------ 2 files changed, 1 insertion(+), 73 deletions(-) diff --git a/vm/src/obj/objnone.rs b/vm/src/obj/objnone.rs index 51984923d0..cb42e879e7 100644 --- a/vm/src/obj/objnone.rs +++ b/vm/src/obj/objnone.rs @@ -1,9 +1,6 @@ -use super::objproperty::PyPropertyRef; -use super::objstr::PyStringRef; use super::objtype::PyClassRef; use crate::pyobject::{ - IntoPyObject, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, - TypeProtocol, + IntoPyObject, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, }; use crate::vm::VirtualMachine; @@ -52,66 +49,6 @@ impl PyNone { Ok(false) } - #[pymethod(name = "__getattribute__")] - fn get_attribute(zelf: PyRef, name: PyStringRef, vm: &VirtualMachine) -> PyResult { - vm_trace!("None.__getattribute__({:?}, {:?})", self, name); - let cls = zelf.class(); - - // Properties use a comparision with None to determine if they are either invoked by am - // instance binding or a class binding. But if the object itself is None then this detection - // won't work. Instead we call a special function on property that bypasses this check, as - // we are invoking it as a instance binding. - // - // In CPython they instead call the slot tp_descr_get with NULL to indicates it's an - // instance binding. - // https://github.com/python/cpython/blob/master/Objects/typeobject.c#L3281 - fn call_descriptor( - descriptor: PyObjectRef, - get_func: PyObjectRef, - obj: PyObjectRef, - cls: PyObjectRef, - vm: &VirtualMachine, - ) -> PyResult { - if let Ok(property) = PyPropertyRef::try_from_object(vm, descriptor.clone()) { - property.instance_binding_get(obj, vm) - } else { - vm.invoke(&get_func, vec![descriptor, obj, cls]) - } - } - - if let Some(attr) = cls.get_attr(name.as_str()) { - let attr_class = attr.class(); - if attr_class.has_attr("__set__") { - if let Some(get_func) = attr_class.get_attr("__get__") { - return call_descriptor( - attr, - get_func, - zelf.into_object(), - cls.into_object(), - vm, - ); - } - } - } - - // None has no attributes and cannot have attributes set on it. - // if let Some(obj_attr) = zelf.as_object().get_attr(name.as_str()) { - // Ok(obj_attr) - // } else - if let Some(attr) = cls.get_attr(name.as_str()) { - let attr_class = attr.class(); - if let Some(get_func) = attr_class.get_attr("__get__") { - call_descriptor(attr, get_func, zelf.into_object(), cls.into_object(), vm) - } else { - Ok(attr) - } - } else if let Some(getter) = cls.get_attr("__getattr__") { - vm.invoke(&getter, vec![zelf.into_object(), name.into_object()]) - } else { - Err(vm.new_attribute_error(format!("{} has no attribute '{}'", zelf.as_object(), name))) - } - } - #[pymethod(name = "__eq__")] fn eq(&self, rhs: PyObjectRef, vm: &VirtualMachine) -> PyObjectRef { if vm.is_none(&rhs) { diff --git a/vm/src/obj/objproperty.rs b/vm/src/obj/objproperty.rs index 7bc683bc27..eb550ac71c 100644 --- a/vm/src/obj/objproperty.rs +++ b/vm/src/obj/objproperty.rs @@ -107,15 +107,6 @@ impl PyProperty { // Descriptor methods - // specialised version that doesn't check for None - pub(crate) fn instance_binding_get(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(ref getter) = self.getter.as_ref() { - vm.invoke(getter, obj) - } else { - Err(vm.new_attribute_error("unreadable attribute".to_owned())) - } - } - #[pymethod(name = "__set__")] fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(ref setter) = self.setter.as_ref() { From c0b235ed66f80ac71b7a13f76e198c18ea862411 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 2 Feb 2020 23:41:38 +0900 Subject: [PATCH 10/10] cleanup property and get descriptor codes --- vm/src/obj/objproperty.rs | 13 +++++++------ vm/src/obj/objsuper.rs | 2 +- vm/src/obj/objtype.rs | 2 +- vm/src/vm.rs | 38 +++++++++++++++++++++----------------- 4 files changed, 30 insertions(+), 25 deletions(-) diff --git a/vm/src/obj/objproperty.rs b/vm/src/obj/objproperty.rs index eb550ac71c..cb6c21cd1c 100644 --- a/vm/src/obj/objproperty.rs +++ b/vm/src/obj/objproperty.rs @@ -73,6 +73,7 @@ struct PropertyArgs { } impl SlotDescriptor for PyProperty { + #[allow(clippy::collapsible_if)] fn descr_get( vm: &VirtualMachine, zelf: PyObjectRef, @@ -80,14 +81,14 @@ impl SlotDescriptor for PyProperty { _cls: OptionalArg, ) -> PyResult { let (zelf, obj) = Self::_unwrap(zelf, obj, vm)?; - if let Some(getter) = zelf.getter.as_ref() { - if obj.is(vm.ctx.none.as_object()) { - Ok(zelf.into_object()) - } else { + if vm.is_none(&obj) { + Ok(zelf.into_object()) + } else { + if let Some(getter) = zelf.getter.as_ref() { vm.invoke(&getter, obj) + } else { + Err(vm.new_attribute_error("unreadable attribute".to_string())) } - } else { - Err(vm.new_attribute_error("unreadable attribute".to_owned())) } } } diff --git a/vm/src/obj/objsuper.rs b/vm/src/obj/objsuper.rs index a9b5aed07e..89520235b8 100644 --- a/vm/src/obj/objsuper.rs +++ b/vm/src/obj/objsuper.rs @@ -63,7 +63,7 @@ impl PySuper { // This is a classmethod return Ok(item); } - return vm.call_get_descriptor(item, inst.clone()); + return vm.call_if_get_descriptor(item, inst.clone()); } } Err(vm.new_attribute_error(format!( diff --git a/vm/src/obj/objtype.rs b/vm/src/obj/objtype.rs index 4a4a6b6d32..922993c985 100644 --- a/vm/src/obj/objtype.rs +++ b/vm/src/obj/objtype.rs @@ -185,7 +185,7 @@ impl PyClassRef { if let Some(cls_attr) = self.get_attr(&name) { Ok(cls_attr) } else if let Some(attr) = mcl.get_attr(&name) { - vm.call_get_descriptor(attr, self.into_object()) + vm.call_if_get_descriptor(attr, self.into_object()) } else if let Some(ref getter) = self.get_attr("__getattr__") { vm.invoke(getter, vec![mcl.into_object(), name_ref.into_object()]) } else { diff --git a/vm/src/vm.rs b/vm/src/vm.rs index ca6cce10da..53ef276cf7 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -618,23 +618,28 @@ impl VirtualMachine { objbool::boolval(self, ret) } - pub fn call_get_descriptor(&self, attr: PyObjectRef, obj: PyObjectRef) -> PyResult { - let attr_class = attr.class(); - let slots = attr_class.slots.borrow(); - if let Some(descr_get) = slots.borrow().descr_get.as_ref() { + pub fn call_get_descriptor(&self, descr: PyObjectRef, obj: PyObjectRef) -> Option { + let descr_class = descr.class(); + let slots = descr_class.slots.borrow(); + Some(if let Some(descr_get) = slots.borrow().descr_get.as_ref() { let cls = obj.class(); descr_get( self, - attr, + descr, Some(obj.clone()), OptionalArg::Present(cls.into_object()), ) - } else if let Some(ref descriptor) = attr_class.get_attr("__get__") { + } else if let Some(ref descriptor) = descr_class.get_attr("__get__") { let cls = obj.class(); - self.invoke(descriptor, vec![attr, obj.clone(), cls.into_object()]) + self.invoke(descriptor, vec![descr, obj.clone(), cls.into_object()]) } else { - Ok(attr) - } + return None; + }) + } + + pub fn call_if_get_descriptor(&self, attr: PyObjectRef, obj: PyObjectRef) -> PyResult { + self.call_get_descriptor(attr.clone(), obj) + .unwrap_or(Ok(attr)) } pub fn call_method(&self, obj: &PyObjectRef, method_name: &str, args: T) -> PyResult @@ -654,7 +659,7 @@ impl VirtualMachine { method_name, func ); - let wrapped = self.call_get_descriptor(func, obj.clone())?; + 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))), @@ -787,7 +792,7 @@ impl VirtualMachine { { let cls = obj.class(); match cls.get_attr(method_name) { - Some(method) => self.call_get_descriptor(method, obj.clone()), + Some(method) => self.call_if_get_descriptor(method, obj.clone()), None => Err(self.new_type_error(err_msg())), } } @@ -796,7 +801,7 @@ impl VirtualMachine { pub fn get_method(&self, obj: PyObjectRef, method_name: &str) -> Option { let cls = obj.class(); let method = cls.get_attr(method_name)?; - Some(self.call_get_descriptor(method, obj.clone())) + Some(self.call_if_get_descriptor(method, obj.clone())) } /// Calls a method on `obj` passing `arg`, if the method exists. @@ -848,6 +853,7 @@ impl VirtualMachine { }) } + /// CPython _PyObject_GenericGetAttrWithDict pub fn generic_getattribute( &self, obj: PyObjectRef, @@ -859,10 +865,8 @@ impl VirtualMachine { if let Some(attr) = cls.get_attr(&name) { let attr_class = attr.class(); if attr_class.has_attr("__set__") { - if let Some(descriptor) = attr_class.get_attr("__get__") { - return self - .invoke(&descriptor, vec![attr, obj, cls.into_object()]) - .map(Some); + if let Some(r) = self.call_get_descriptor(attr, obj.clone()) { + return r.map(Some); } } } @@ -876,7 +880,7 @@ impl VirtualMachine { if let Some(obj_attr) = attr { Ok(Some(obj_attr)) } else if let Some(attr) = cls.get_attr(&name) { - self.call_get_descriptor(attr, obj).map(Some) + self.call_if_get_descriptor(attr, obj).map(Some) } else if let Some(getter) = cls.get_attr("__getattr__") { self.invoke(&getter, vec![obj, name_str.into_object()]) .map(Some)