Merge pull request #722 from RustPython/joey/unboxed-payload-2

Remove Box from PyObject

vm/src/obj/objtype.rs

@@ -35,12 +35,6 @@ impl PyValue for PyClass {
     }
 }
 
-impl IdProtocol for PyClassRef {
-    fn get_id(&self) -> usize {
-        self.as_object().get_id()
-    }
-}
-
 impl TypeProtocol for PyClassRef {
     fn type_ref(&self) -> &PyObjectRef {
         &self.as_object().type_ref()
@@ -335,10 +329,7 @@ fn take_next_base(mut bases: Vec<Vec<PyClassRef>>) -> Option<(PyClassRef, Vec<Ve
 
     for base in &bases {
         let head = base[0].clone();
-        if !(&bases)
-            .iter()
-            .any(|x| x[1..].iter().any(|x| x.get_id() == head.get_id()))
-        {
+        if !(&bases).iter().any(|x| x[1..].iter().any(|x| x.is(&head))) {
             next = Some(head);
             break;
         }
@@ -346,7 +337,7 @@ fn take_next_base(mut bases: Vec<Vec<PyClassRef>>) -> Option<(PyClassRef, Vec<Ve
 
     if let Some(head) = next {
         for item in &mut bases {
-            if item[0].get_id() == head.get_id() {
+            if item[0].is(&head) {
                 item.remove(0);
             }
         }
@@ -382,10 +373,10 @@ pub fn new(
     let mros = bases.into_iter().map(|x| _mro(&x)).collect();
     let mro = linearise_mro(mros).unwrap();
     Ok(PyObject {
-        payload: Box::new(PyClass {
+        payload: PyClass {
             name: String::from(name),
             mro,
-        }),
+        },
         dict: Some(RefCell::new(dict)),
         typ,
     }

vm/src/obj/objweakref.rs

@@ -1,13 +1,13 @@
 use crate::obj::objtype::PyClassRef;
 use crate::pyobject::PyValue;
-use crate::pyobject::{PyContext, PyObject, PyObjectRef, PyRef, PyResult};
+use crate::pyobject::{PyContext, PyObject, PyObjectPayload, PyObjectRef, PyRef, PyResult};
 use crate::vm::VirtualMachine;
 
 use std::rc::{Rc, Weak};
 
 #[derive(Debug)]
 pub struct PyWeak {
-    referent: Weak<PyObject>,
+    referent: Weak<PyObject<dyn PyObjectPayload>>,
 }
 
 impl PyWeak {

vm/src/pyobject.rs

@@ -72,7 +72,7 @@ Basically reference counting, but then done by rust.
 /// this reference counting is accounted for by this type. Use the `.clone()`
 /// method to create a new reference and increment the amount of references
 /// to the python object by 1.
-pub type PyObjectRef = Rc<PyObject>;
+pub type PyObjectRef = Rc<PyObject<dyn PyObjectPayload>>;
 
 /// Use this type for function which return a python object or and exception.
 /// Both the python object and the python exception are `PyObjectRef` types
@@ -83,7 +83,7 @@ pub type PyResult<T = PyObjectRef> = Result<T, PyObjectRef>; // A valid value, o
 /// faster, unordered, and only supports strings as keys.
 pub type PyAttributes = HashMap<String, PyObjectRef>;
 
-impl fmt::Display for PyObject {
+impl fmt::Display for PyObject<dyn PyObjectPayload> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         use self::TypeProtocol;
         if let Some(PyClass { ref name, .. }) = self.payload::<PyClass>() {
@@ -193,25 +193,25 @@ fn init_type_hierarchy() -> (PyClassRef, PyClassRef) {
         let object_type = PyObject {
             typ: mem::uninitialized(), // !
             dict: Some(RefCell::new(PyAttributes::new())),
-            payload: Box::new(PyClass {
+            payload: PyClass {
                 name: String::from("object"),
                 mro: vec![],
-            }),
+            },
         }
         .into_ref();
 
         let type_type = PyObject {
             typ: mem::uninitialized(), // !
             dict: Some(RefCell::new(PyAttributes::new())),
-            payload: Box::new(PyClass {
+            payload: PyClass {
                 name: String::from("type"),
                 mro: vec![FromPyObjectRef::from_pyobj(&object_type)],
-            }),
+            },
         }
         .into_ref();
 
-        let object_type_ptr = PyObjectRef::into_raw(object_type.clone()) as *mut PyObject;
-        let type_type_ptr = PyObjectRef::into_raw(type_type.clone()) as *mut PyObject;
+        let object_type_ptr = PyObjectRef::into_raw(object_type.clone()) as *mut PyObject<PyClass>;
+        let type_type_ptr = PyObjectRef::into_raw(type_type.clone()) as *mut PyObject<PyClass>;
         ptr::write(&mut (*object_type_ptr).typ, type_type.clone());
         ptr::write(&mut (*type_type_ptr).typ, type_type.clone());
 
@@ -639,7 +639,7 @@ impl PyContext {
         PyObject {
             typ: class.into_object(),
             dict: Some(RefCell::new(dict)),
-            payload: Box::new(objobject::PyInstance),
+            payload: objobject::PyInstance,
         }
         .into_ref()
     }
@@ -708,10 +708,13 @@ impl Default for PyContext {
 /// This is an actual python object. It consists of a `typ` which is the
 /// python class, and carries some rust payload optionally. This rust
 /// payload can be a rust float or rust int in case of float and int objects.
-pub struct PyObject {
+pub struct PyObject<T>
+where
+    T: ?Sized + PyObjectPayload,
+{
     pub typ: PyObjectRef,
     pub dict: Option<RefCell<PyAttributes>>, // __dict__ member
-    pub payload: Box<dyn PyObjectPayload>,
+    pub payload: T,
 }
 
 /// A reference to a Python object.
@@ -820,9 +823,30 @@ pub trait IdProtocol {
     }
 }
 
-impl IdProtocol for PyObjectRef {
+#[derive(Debug)]
+enum Never {}
+
+impl PyValue for Never {
+    fn class(_vm: &VirtualMachine) -> PyObjectRef {
+        unreachable!()
+    }
+}
+
+impl<T: ?Sized + PyObjectPayload> IdProtocol for PyObject<T> {
     fn get_id(&self) -> usize {
-        &*self as &PyObject as *const PyObject as usize
+        self as *const _ as *const PyObject<Never> as usize
+    }
+}
+
+impl<T: ?Sized + IdProtocol> IdProtocol for Rc<T> {
+    fn get_id(&self) -> usize {
+        (**self).get_id()
+    }
+}
+
+impl<T: PyObjectPayload> IdProtocol for PyRef<T> {
+    fn get_id(&self) -> usize {
+        self.obj.get_id()
     }
 }
 
@@ -846,7 +870,10 @@ impl TypeProtocol for PyObjectRef {
     }
 }
 
-impl TypeProtocol for PyObject {
+impl<T> TypeProtocol for PyObject<T>
+where
+    T: ?Sized + PyObjectPayload,
+{
     fn type_ref(&self) -> &PyObjectRef {
         &self.typ
     }
@@ -985,9 +1012,9 @@ impl BufferProtocol for PyObjectRef {
     }
 }
 
-impl fmt::Debug for PyObject {
+impl fmt::Debug for PyObject<dyn PyObjectPayload> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "[PyObj {:?}]", self.payload)
+        write!(f, "[PyObj {:?}]", &self.payload)
     }
 }
 
@@ -1159,21 +1186,24 @@ impl PyValue for PyIteratorValue {
     }
 }
 
-impl PyObject {
-    pub fn new<T: PyObjectPayload>(payload: T, typ: PyObjectRef) -> PyObjectRef {
+impl<T> PyObject<T>
+where
+    T: Sized + PyObjectPayload,
+{
+    pub fn new(payload: T, typ: PyObjectRef) -> PyObjectRef {
         PyObject {
             typ,
             dict: Some(RefCell::new(PyAttributes::new())),
-            payload: Box::new(payload),
+            payload,
         }
         .into_ref()
     }
 
-    pub fn new_without_dict<T: PyObjectPayload>(payload: T, typ: PyObjectRef) -> PyObjectRef {
+    pub fn new_without_dict(payload: T, typ: PyObjectRef) -> PyObjectRef {
         PyObject {
             typ,
             dict: None,
-            payload: Box::new(payload),
+            payload,
         }
         .into_ref()
     }
@@ -1182,7 +1212,9 @@ impl PyObject {
     pub fn into_ref(self) -> PyObjectRef {
         Rc::new(self)
     }
+}
 
+impl PyObject<dyn PyObjectPayload> {
     #[inline]
     pub fn payload<T: PyObjectPayload>(&self) -> Option<&T> {
         self.payload.as_any().downcast_ref()
@@ -1245,7 +1277,7 @@ impl FromPyObjectRef for PyRef<PyClass> {
 
 #[cfg(test)]
 mod tests {
-    use super::PyContext;
+    use super::*;
 
     #[test]
     fn test_type_type() {

vm/src/vm.rs

@@ -335,7 +335,7 @@ impl VirtualMachine {
         }
 
         // TODO: is it safe to just invoke __call__ otherwise?
-        trace!("invoke __call__ for: {:?}", func_ref.payload);
+        trace!("invoke __call__ for: {:?}", &func_ref.payload);
         self.call_method(&func_ref, "__call__", args)
     }
 

wasm/lib/src/vm_class.rs

@@ -8,23 +8,19 @@ use wasm_bindgen::prelude::*;
 use rustpython_vm::compile;
 use rustpython_vm::frame::{NameProtocol, Scope};
 use rustpython_vm::function::PyFuncArgs;
-use rustpython_vm::pyobject::{PyContext, PyObjectRef, PyResult};
+use rustpython_vm::pyobject::{PyContext, PyObject, PyObjectPayload, PyObjectRef, PyResult};
 use rustpython_vm::VirtualMachine;
 
 use crate::browser_module::setup_browser_module;
 use crate::convert;
 use crate::wasm_builtins;
 
-pub trait HeldRcInner {}
-
-impl<T> HeldRcInner for T {}
-
 pub(crate) struct StoredVirtualMachine {
     pub vm: VirtualMachine,
     pub scope: RefCell<Scope>,
     /// you can put a Rc in here, keep it as a Weak, and it'll be held only for
     /// as long as the StoredVM is alive
-    held_rcs: RefCell<Vec<Rc<dyn HeldRcInner>>>,
+    held_objects: RefCell<Vec<PyObjectRef>>,
 }
 
 impl StoredVirtualMachine {
@@ -38,7 +34,7 @@ impl StoredVirtualMachine {
         StoredVirtualMachine {
             vm,
             scope: RefCell::new(scope),
-            held_rcs: RefCell::new(Vec::new()),
+            held_objects: RefCell::new(Vec::new()),
         }
     }
 }
@@ -178,13 +174,13 @@ impl WASMVirtualMachine {
         STORED_VMS.with(|cell| cell.borrow().contains_key(&self.id))
     }
 
-    pub(crate) fn push_held_rc<T: HeldRcInner + 'static>(
+    pub(crate) fn push_held_rc(
         &self,
-        rc: Rc<T>,
-    ) -> Result<Weak<T>, JsValue> {
+        obj: PyObjectRef,
+    ) -> Result<Weak<PyObject<dyn PyObjectPayload>>, JsValue> {
         self.with(|stored_vm| {
-            let weak = Rc::downgrade(&rc);
-            stored_vm.held_rcs.borrow_mut().push(rc);
+            let weak = Rc::downgrade(&obj);
+            stored_vm.held_objects.borrow_mut().push(obj);
             weak
         })
     }