From 39c086aea86d1bed9793be983784511389490de4 Mon Sep 17 00:00:00 2001 From: Joey Hain Date: Tue, 12 Mar 2019 19:00:41 -0700 Subject: [PATCH 1/3] Remove inner Box from PyObject --- vm/src/obj/objtype.rs | 4 +-- vm/src/obj/objweakref.rs | 2 +- vm/src/pyobject.rs | 61 ++++++++++++++++++++-------------------- vm/src/vm.rs | 4 +-- 4 files changed, 35 insertions(+), 36 deletions(-) diff --git a/vm/src/obj/objtype.rs b/vm/src/obj/objtype.rs index dd317c5175..23fb1812c9 100644 --- a/vm/src/obj/objtype.rs +++ b/vm/src/obj/objtype.rs @@ -330,10 +330,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, } diff --git a/vm/src/obj/objweakref.rs b/vm/src/obj/objweakref.rs index d2e2d08dc4..52739fdd4e 100644 --- a/vm/src/obj/objweakref.rs +++ b/vm/src/obj/objweakref.rs @@ -7,7 +7,7 @@ use std::rc::{Rc, Weak}; #[derive(Debug)] pub struct PyWeak { - referent: Weak, + referent: Weak>, } impl PyWeak { diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 0535779487..80093f6fb6 100644 --- a/vm/src/pyobject.rs +++ b/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; +pub type PyObjectRef = Rc>; /// 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 = Result; // A valid value, o /// faster, unordered, and only supports strings as keys. pub type PyAttributes = HashMap; -impl fmt::Display for PyObject { +impl fmt::Display for PyObject { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { use self::TypeProtocol; if let Some(PyClass { ref name, .. }) = self.payload::() { @@ -188,25 +188,25 @@ fn init_type_hierarchy() -> (PyObjectRef, PyObjectRef) { 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; + let type_type_ptr = PyObjectRef::into_raw(type_type.clone()) as *mut PyObject; ptr::write(&mut (*object_type_ptr).typ, type_type.clone()); ptr::write(&mut (*type_type_ptr).typ, type_type.clone()); @@ -632,7 +632,7 @@ impl PyContext { PyObject { typ: class, dict: Some(RefCell::new(dict)), - payload: Box::new(objobject::PyInstance), + payload: objobject::PyInstance, } .into_ref() } @@ -695,10 +695,10 @@ 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 { pub typ: PyObjectRef, pub dict: Option>, // __dict__ member - pub payload: Box, + pub payload: T, } /// A reference to a Python object. @@ -711,7 +711,7 @@ pub struct PyObject { /// situations (such as when implementing in-place methods such as `__iadd__`) /// where a reference to the same object must be returned. #[derive(Clone, Debug)] -pub struct PyRef { +pub struct PyRef { // invariant: this obj must always have payload of type T obj: PyObjectRef, _payload: PhantomData, @@ -747,10 +747,7 @@ impl PyRef { } } -impl Deref for PyRef -where - T: PyValue, -{ +impl Deref for PyRef { type Target = T; fn deref(&self) -> &T { @@ -758,10 +755,7 @@ where } } -impl TryFromObject for PyRef -where - T: PyValue, -{ +impl TryFromObject for PyRef { fn try_from_object(vm: &mut VirtualMachine, obj: PyObjectRef) -> PyResult { if objtype::isinstance(&obj, &T::required_type(&vm.ctx)) { Ok(PyRef { @@ -785,9 +779,9 @@ impl IntoPyObject for PyRef { } } -impl fmt::Display for PyRef { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.obj.fmt(f) +impl fmt::Display for PyRef { + fn fmt(&self, _f: &mut fmt::Formatter) -> fmt::Result { + Ok(()) // TODO } } @@ -803,7 +797,10 @@ pub trait IdProtocol { impl IdProtocol for PyObjectRef { fn get_id(&self) -> usize { - &*self as &PyObject as *const PyObject as usize + // This is kinda ridiculous... + // + // Rc -> (fat) shared ref -> (fat) pointer -> (thin) pointer -> usize + &*self as &PyObject as *const PyObject as *const PyObject<()> as usize } } @@ -827,7 +824,7 @@ impl TypeProtocol for PyObjectRef { } } -impl TypeProtocol for PyObject { +impl TypeProtocol for PyObject { fn type_ref(&self) -> &PyObjectRef { &self.typ } @@ -966,9 +963,9 @@ impl BufferProtocol for PyObjectRef { } } -impl fmt::Debug for PyObject { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "[PyObj {:?}]", self.payload) +impl fmt::Debug for PyObject { + fn fmt(&self, _f: &mut fmt::Formatter) -> fmt::Result { + Ok(()) // TODO } } @@ -1546,12 +1543,12 @@ impl PyValue for PyIteratorValue { } } -impl PyObject { - pub fn new(payload: T, typ: PyObjectRef) -> PyObjectRef { +impl PyObject { + pub fn new(payload: T, typ: PyObjectRef) -> PyObjectRef { PyObject { typ, dict: Some(RefCell::new(PyAttributes::new())), - payload: Box::new(payload), + payload, } .into_ref() } @@ -1560,7 +1557,9 @@ impl PyObject { pub fn into_ref(self) -> PyObjectRef { Rc::new(self) } +} +impl PyObject { pub fn payload(&self) -> Option<&T> { self.payload.downcast_ref() } diff --git a/vm/src/vm.rs b/vm/src/vm.rs index d57d0e4388..9b42d835ce 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -323,7 +323,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) } @@ -547,7 +547,7 @@ impl VirtualMachine { let cls = obj.typ(); match cls.get_attr(method_name) { Some(method) => self.call_get_descriptor(method, obj.clone()), - None => Err(self.new_type_error(format!("{} has no method {:?}", obj, method_name))), + None => Err(self.new_type_error(format!("{} has no method {:?}", &obj, method_name))), } } From 60250588561eb1317cbc0a4a6aef13a88b3ec8d2 Mon Sep 17 00:00:00 2001 From: Joey Date: Thu, 14 Mar 2019 21:59:20 -0700 Subject: [PATCH 2/3] Fix Debug and Display impls --- vm/src/pyobject.rs | 10 +++++----- vm/src/vm.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 530c6acd99..efabed2c50 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -782,8 +782,8 @@ impl IntoPyObject for PyRef { } impl fmt::Display for PyRef { - fn fmt(&self, _f: &mut fmt::Formatter) -> fmt::Result { - Ok(()) // TODO + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.obj.fmt(f) } } @@ -965,9 +965,9 @@ impl BufferProtocol for PyObjectRef { } } -impl fmt::Debug for PyObject { - fn fmt(&self, _f: &mut fmt::Formatter) -> fmt::Result { - Ok(()) // TODO +impl fmt::Debug for PyObject { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "[PyObj {:?}]", &self.payload) } } diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 7f9e568359..9d3e3372d0 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -323,7 +323,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) } From 4d359319cda5c5d7e83fec251a73213e58dade03 Mon Sep 17 00:00:00 2001 From: Joey Date: Fri, 15 Mar 2019 20:48:28 -0700 Subject: [PATCH 3/3] Fix wasm --- wasm/lib/src/vm_class.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/wasm/lib/src/vm_class.rs b/wasm/lib/src/vm_class.rs index b3ef506272..37329e85d6 100644 --- a/wasm/lib/src/vm_class.rs +++ b/wasm/lib/src/vm_class.rs @@ -5,7 +5,7 @@ use js_sys::{Object, Reflect, SyntaxError, TypeError}; use rustpython_vm::{ compile, frame::{NameProtocol, Scope}, - pyobject::{PyContext, PyFuncArgs, PyObjectRef, PyResult}, + pyobject::{PyContext, PyFuncArgs, PyObject, PyObjectPayload, PyObjectRef, PyResult}, VirtualMachine, }; use std::cell::RefCell; @@ -13,16 +13,12 @@ use std::collections::HashMap; use std::rc::{Rc, Weak}; use wasm_bindgen::{prelude::*, JsCast}; -pub trait HeldRcInner {} - -impl HeldRcInner for T {} - pub(crate) struct StoredVirtualMachine { pub vm: VirtualMachine, pub scope: 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: Vec>, + held_rcs: Vec, } impl StoredVirtualMachine { @@ -221,10 +217,10 @@ impl WASMVirtualMachine { STORED_VMS.with(|cell| cell.borrow().contains_key(&self.id)) } - pub(crate) fn push_held_rc( + pub(crate) fn push_held_rc( &self, - rc: Rc, - ) -> Result, JsValue> { + rc: PyObjectRef, + ) -> Result>, JsValue> { self.with(|stored_vm| { let weak = Rc::downgrade(&rc); stored_vm.held_rcs.push(rc);