Change property.__new__ to use new style function and construct

PyProperty
cthulahoops · Mar 10, 2019 · Mar 8, 2019 · Mar 8, 2019 · Mar 9, 2019 · Mar 9, 2019
commit 0ec034df51adb15c4808f658f2221fabf6780a76
diff --git 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<Self> {
+        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<T> Deref for PyRef<T>

diff --git 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<bytecode::CodeObject> {
-    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
@@ -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
@@ -1,14 +1,14 @@
 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,
 };
 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
@@ -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<PyObjectRef>,
     setter: Option<PyObjectRef>,
-    deleter: Option<PyObjectRef>
+    deleter: Option<PyObjectRef>,
 }
 
 impl PyObjectPayload2 for PyProperty {
@@ -46,6 +49,25 @@ impl PyObjectPayload2 for PyProperty {
 pub type PyPropertyRef = PyRef<PyProperty>;
 
 impl PyPropertyRef {
+    fn new_property(
+        cls: PyClassRef,
+        fget: OptionalArg<PyObjectRef>,
+        fset: OptionalArg<PyObjectRef>,
+        fdel: OptionalArg<PyObjectRef>,
+        _doc: OptionalArg<PyStringRef>,
+        vm: &mut VirtualMachine,
+    ) -> PyResult<PyPropertyRef> {
+        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<PyObjectRef>,
     setter: Option<PyObjectRef>,
-    _return: PhantomData<T>
+    _return: PhantomData<T>,
 }
 
-
 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<I, F:IntoPyNativeFunc<I, T>>(self, func: F) -> Self {
+    pub fn add_getter<I, F: IntoPyNativeFunc<I, T>>(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<I, F:IntoPyNativeFunc<(I, T), PyResult>>(self, func: F) -> Self {
+    pub fn add_setter<I, F: IntoPyNativeFunc<(I, T), PyResult>>(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,31 +135,32 @@ 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(),
             )
         }
     }
 }
 
-
 pub fn init(context: &PyContext) {
     extend_class!(context, &context.readonly_property_type, {
         "__get__" => context.new_rustfunc(PyReadOnlyPropertyRef::get),
@@ -173,20 +195,11 @@ 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),
         "__set__" => context.new_rustfunc(PyPropertyRef::set),
         "__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
@@ -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
@@ -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<PyObjectRef> for PyFuncArgs {
     }
 }
 
-
 impl PyFuncArgs {
     pub fn new(mut args: Vec<PyObjectRef>, kwarg_names: Vec<String>) -> PyFuncArgs {
         let mut kwargs = vec![];