Migrate usage of member_descriptor and data_descriptor to

new_property/PropertyBuilder
cthulahoops · Mar 10, 2019 · Mar 8, 2019 · Mar 8, 2019 · Mar 9, 2019 · Mar 9, 2019
commit 2edfe4c7be09484f1b7a3c64747daab92ffb92e0
diff --git 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<byteco
         vm,
         args,
         required = [
-            (zelf, Some(vm.ctx.code_type())),
-            (_cls, Some(vm.ctx.type_type()))
+            (zelf, Some(vm.ctx.code_type()))
         ]
     );
     Ok(get_value(zelf))

diff --git a/vm/src/obj/objfunction.rs b/vm/src/obj/objfunction.rs
@@ -10,7 +10,7 @@ pub fn init(context: &PyContext) {
     context.set_attr(
         &function_type,
         "__code__",
-        context.new_member_descriptor(function_code),
+        context.new_property(function_code),
     );
 
     let builtin_function_or_method_type = &context.builtin_function_or_method_type;
@@ -20,24 +20,6 @@ pub fn init(context: &PyContext) {
         context.new_rustfunc(bind_method),
     );
 
-    let member_descriptor_type = &context.member_descriptor_type;
-    context.set_attr(
-        &member_descriptor_type,
-        "__get__",
-        context.new_rustfunc(member_get),
-    );
-
-    let data_descriptor_type = &context.data_descriptor_type;
-    context.set_attr(
-        &data_descriptor_type,
-        "__get__",
-        context.new_rustfunc(data_get),
-    );
-    context.set_attr(
-        &data_descriptor_type,
-        "__set__",
-        context.new_rustfunc(data_set),
-    );
 
     let classmethod_type = &context.classmethod_type;
     context.set_attr(
@@ -85,36 +67,6 @@ fn function_code(vm: &mut VirtualMachine, args: PyFuncArgs) -> 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
@@ -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
@@ -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<PyReadOnlyProperty>;
 
 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<PyProperty>;
 
 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<PyObjectRef>,
     setter: Option<PyObjectRef>,
-    instance: PhantomData<I>,
     _return: PhantomData<T>
 }
 
 
-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<F:IntoPyNativeFunc<I, T>>(self, func: F) -> Self {
-        let func = self.vm.ctx.new_rustfunc(func);
+    pub fn add_getter<I, F:IntoPyNativeFunc<I, T>>(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<F:IntoPyNativeFunc<(I, T), PyNone>>(self, func: F) -> Self {
-        let func = self.vm.ctx.new_rustfunc(func);
+    pub fn add_setter<I, F:IntoPyNativeFunc<(I, T), PyResult>>(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
@@ -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
@@ -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<T, R>,
     {
-        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<F: 'static + Fn(&mut VirtualMachine, PyFuncArgs) -> 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<PyAttributes>) -> PyObjectRef {
         let dict = if let Some(dict) = dict {
             dict
@@ -1292,6 +1250,7 @@ pub enum OptionalArg<T> {
 }
 
 use self::OptionalArg::*;
+use crate::obj::objproperty::PropertyBuilder;
 
 impl<T> OptionalArg<T> {
     pub fn into_option(self) -> Option<T> {