Remove PropertyBuilder and add new_getset

RustPython · coolreader18 · Feb 6, 2020 · Oct 26, 2019 · Dec 14, 2019 · Feb 1, 2020
commit facabfee1ac0ca5f5e2337a2e2aff253ce3810ee
diff --git a/derive/src/pyclass.rs b/derive/src/pyclass.rs
@@ -692,7 +692,8 @@ pub fn impl_pystruct_sequence(attr: AttributeArgs, item: Item) -> Result<TokenSt
             let property = quote! {
                 class.set_str_attr(
                     #field_name_str,
-                    ctx.new_property(
+                    ctx.new_readonly_getset(
+                        #field_name_str,
                         |zelf: &::rustpython_vm::obj::objtuple::PyTuple,
                          _vm: &::rustpython_vm::VirtualMachine| {
                             zelf.fast_getitem(#idx)

diff --git a/vm/src/exceptions.rs b/vm/src/exceptions.rs
@@ -644,48 +644,44 @@ pub fn init(ctx: &PyContext) {
     PyBaseException::extend_class(ctx, &excs.base_exception_type);
 
     extend_class!(ctx, &excs.syntax_error, {
-        "msg" => 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, {
         "__str__" => ctx.new_method(key_error_str),
     });
 
     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
@@ -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
@@ -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<PyReadOnlyProperty>;
-
-impl SlotDescriptor for PyReadOnlyProperty {
-    fn descr_get(
-        vm: &VirtualMachine,
-        zelf: PyObjectRef,
-        obj: Option<PyObjectRef>,
-        cls: OptionalArg<PyObjectRef>,
-    ) -> 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<PyObjec
     }
 }
 
-pub struct PropertyBuilder<'a> {
-    ctx: &'a PyContext,
-    getter: Option<PyObjectRef>,
-    setter: Option<PyObjectRef>,
-}
-
-impl<'a> PropertyBuilder<'a> {
-    pub fn new(ctx: &'a PyContext) -> Self {
-        Self {
-            ctx,
-            getter: None,
-            setter: None,
-        }
-    }
-
-    pub fn add_getter<I, R, VM, F: IntoPyNativeFunc<I, R, VM>>(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
@@ -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<PyObjectRef> =
             _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<PyClassRef> {
@@ -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<PyObjectRef> {

diff --git 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<F, I, V, VM>(&self, f: F) -> PyObjectRef
+    pub fn new_readonly_getset<F, T>(&self, name: impl Into<String>, f: F) -> PyObjectRef
     where
-        F: IntoPyNativeFunc<I, V, VM>,
+        F: IntoPyGetterFunc<T>,
     {
-        PropertyBuilder::new(self).add_getter(f).create()
+        PyObject::new(PyGetSet::with_get(name.into(), f), self.getset_type(), None)
+    }
+
+    pub fn new_getset<G, S, T, U>(&self, name: impl Into<String>, g: G, s: S) -> PyObjectRef
+    where
+        G: IntoPyGetterFunc<T>,
+        S: IntoPySetterFunc<U>,
+    {
+        PyObject::new(
+            PyGetSet::with_get_set(name.into(), g, s),
+            self.getset_type(),
+            None,
+        )
     }
 
     pub fn new_code_object(&self, code: bytecode::CodeObject) -> PyCodeRef {