From e49d7146a0fa8e6c6014d046d77a30b571f10b69 Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Tue, 12 Mar 2019 18:31:59 -0500 Subject: [PATCH 1/9] Add PyValuePayload trait and use it for PyObject.payload --- vm/src/obj/objobject.rs | 8 +++++++- vm/src/pyobject.rs | 33 ++++++++++++++++++++++----------- vm/src/stdlib/re.rs | 4 ++-- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/vm/src/obj/objobject.rs b/vm/src/obj/objobject.rs index 0906f2224a..60f4001103 100644 --- a/vm/src/obj/objobject.rs +++ b/vm/src/obj/objobject.rs @@ -4,7 +4,7 @@ use super::objtype; use crate::obj::objproperty::PropertyBuilder; use crate::pyobject::{ AttributeProtocol, DictProtocol, IdProtocol, PyAttributes, PyContext, PyFuncArgs, PyObject, - PyObjectRef, PyRef, PyResult, TypeProtocol, + PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, }; use crate::vm::VirtualMachine; use std::cell::RefCell; @@ -13,6 +13,12 @@ use std::collections::HashMap; #[derive(Clone, Debug)] pub struct PyInstance; +impl PyValue for PyInstance { + fn required_type(_ctx: &PyContext) -> PyObjectRef { + panic!("no specific type for PyInstance, don't type check me") + } +} + pub type PyInstanceRef = PyRef; pub fn new_instance(vm: &mut VirtualMachine, mut args: PyFuncArgs) -> PyResult { diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 6568e7e84c..7490d332d6 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -591,11 +591,7 @@ impl PyContext { } pub fn new_instance(&self, class: PyObjectRef, dict: Option) -> PyObjectRef { - let dict = if let Some(dict) = dict { - dict - } else { - PyAttributes::new() - }; + let dict = dict.unwrap_or_default(); PyObject { typ: Some(class), dict: Some(RefCell::new(dict)), @@ -665,7 +661,7 @@ impl Default for PyContext { pub struct PyObject { pub typ: Option, pub dict: Option>, // __dict__ member - pub payload: Box, + pub payload: Box, } impl Default for PyObject { @@ -1526,7 +1522,7 @@ impl PyValue for PyIteratorValue { } impl PyObject { - pub fn new(payload: T, typ: PyObjectRef) -> PyObjectRef { + pub fn new(payload: T, typ: PyObjectRef) -> PyObjectRef { PyObject { typ: Some(typ), dict: Some(RefCell::new(PyAttributes::new())), @@ -1541,16 +1537,31 @@ impl PyObject { } pub fn payload(&self) -> Option<&T> { - self.payload.downcast_ref() + let payload: &dyn Any = &self.payload; + payload.downcast_ref() } } -// The intention is for this to replace `PyObjectPayload` once everything is -// converted to use `PyObjectPayload::AnyRustvalue`. -pub trait PyValue: Any + fmt::Debug { +pub trait PyValue: fmt::Debug + 'static { fn required_type(ctx: &PyContext) -> PyObjectRef; } +pub trait PyValuePayload: Any + fmt::Debug + 'static { + fn required_type(&self, ctx: &PyContext) -> PyObjectRef; +} + +impl PyValuePayload for T { + fn required_type(&self, ctx: &PyContext) -> PyObjectRef { + T::required_type(ctx) + } +} + +impl PyValuePayload for () { + fn required_type(&self, _ctx: &PyContext) -> PyObjectRef { + panic!("No specific python type for rust unit, don't type check") + } +} + impl FromPyObjectRef for PyRef { fn from_pyobj(obj: &PyObjectRef) -> Self { if let Some(_) = obj.payload::() { diff --git a/vm/src/stdlib/re.rs b/vm/src/stdlib/re.rs index 3aee2bf61f..4d6c024dae 100644 --- a/vm/src/stdlib/re.rs +++ b/vm/src/stdlib/re.rs @@ -197,7 +197,7 @@ fn match_end(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { /// Retrieve inner rust regex from python object: fn get_regex<'a>(obj: &'a PyObjectRef) -> &'a Regex { // TODO: Regex shouldn't be stored in payload directly, create newtype wrapper - if let Some(regex) = obj.payload.downcast_ref::() { + if let Some(regex) = obj.payload::() { return regex; } panic!("Inner error getting regex {:?}", obj); @@ -205,7 +205,7 @@ fn get_regex<'a>(obj: &'a PyObjectRef) -> &'a Regex { /// Retrieve inner rust match from python object: fn get_match<'a>(obj: &'a PyObjectRef) -> &'a PyMatch { - if let Some(value) = obj.payload.downcast_ref::() { + if let Some(value) = obj.payload::() { return value; } panic!("Inner error getting match {:?}", obj); From c0e572767e18b08f2c06974649f145d9c0e6d935 Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Tue, 12 Mar 2019 18:59:43 -0500 Subject: [PATCH 2/9] Rename PyValuePayload to PyObjectPayload --- vm/src/pyobject.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 407ddffa2d..4cb8c849ab 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -694,7 +694,7 @@ impl Default for PyContext { pub struct PyObject { pub typ: PyObjectRef, pub dict: Option>, // __dict__ member - pub payload: Box, + pub payload: Box, } /// A reference to a Python object. @@ -1543,7 +1543,7 @@ impl PyValue for PyIteratorValue { } impl PyObject { - pub fn new(payload: T, typ: PyObjectRef) -> PyObjectRef { + pub fn new(payload: T, typ: PyObjectRef) -> PyObjectRef { PyObject { typ, dict: Some(RefCell::new(PyAttributes::new())), @@ -1567,17 +1567,17 @@ pub trait PyValue: fmt::Debug + 'static { fn required_type(ctx: &PyContext) -> PyObjectRef; } -pub trait PyValuePayload: Any + fmt::Debug + 'static { +pub trait PyObjectPayload: Any + fmt::Debug + 'static { fn required_type(&self, ctx: &PyContext) -> PyObjectRef; } -impl PyValuePayload for T { +impl PyObjectPayload for T { fn required_type(&self, ctx: &PyContext) -> PyObjectRef { T::required_type(ctx) } } -impl PyValuePayload for () { +impl PyObjectPayload for () { fn required_type(&self, _ctx: &PyContext) -> PyObjectRef { panic!("No specific python type for rust unit, don't type check") } From d305d4f74bab33a469143bcc310ece7fd5d6fe96 Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Tue, 12 Mar 2019 19:06:12 -0500 Subject: [PATCH 3/9] Clean up a bit --- vm/src/obj/objobject.rs | 4 ++-- vm/src/pyobject.rs | 16 ++-------------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/vm/src/obj/objobject.rs b/vm/src/obj/objobject.rs index a6c383ebb4..02689fea0b 100644 --- a/vm/src/obj/objobject.rs +++ b/vm/src/obj/objobject.rs @@ -12,8 +12,8 @@ use crate::vm::VirtualMachine; pub struct PyInstance; impl PyValue for PyInstance { - fn required_type(_ctx: &PyContext) -> PyObjectRef { - panic!("no specific type for PyInstance, don't type check me") + fn required_type(ctx: &PyContext) -> PyObjectRef { + ctx.object() } } diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 4cb8c849ab..dcb5f34c7a 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -1567,21 +1567,9 @@ pub trait PyValue: fmt::Debug + 'static { fn required_type(ctx: &PyContext) -> PyObjectRef; } -pub trait PyObjectPayload: Any + fmt::Debug + 'static { - fn required_type(&self, ctx: &PyContext) -> PyObjectRef; -} - -impl PyObjectPayload for T { - fn required_type(&self, ctx: &PyContext) -> PyObjectRef { - T::required_type(ctx) - } -} +pub trait PyObjectPayload: Any + fmt::Debug + 'static {} -impl PyObjectPayload for () { - fn required_type(&self, _ctx: &PyContext) -> PyObjectRef { - panic!("No specific python type for rust unit, don't type check") - } -} +impl PyObjectPayload for T {} impl FromPyObjectRef for PyRef { fn from_pyobj(obj: &PyObjectRef) -> Self { From f31003614a60b64891ef3e3212cf6f2b2133111f Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Tue, 12 Mar 2019 19:42:52 -0500 Subject: [PATCH 4/9] Fix wasm --- wasm/lib/src/browser_module.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wasm/lib/src/browser_module.rs b/wasm/lib/src/browser_module.rs index 035954a7c7..e09a63ac6a 100644 --- a/wasm/lib/src/browser_module.rs +++ b/wasm/lib/src/browser_module.rs @@ -171,7 +171,7 @@ impl PyPromise { } pub fn get_promise_value(obj: &PyObjectRef) -> Promise { - if let Some(promise) = obj.payload.downcast_ref::() { + if let Some(promise) = obj.payload::() { return promise.value.clone(); } panic!("Inner error getting promise") From 7bac1285d4cb3578c3bf5bd0ebc5926044d23aee Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Tue, 12 Mar 2019 20:26:33 -0500 Subject: [PATCH 5/9] Fix all the tests --- vm/src/pyobject.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index dcb5f34c7a..9c6ae2a4fc 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -1558,8 +1558,7 @@ impl PyObject { } pub fn payload(&self) -> Option<&T> { - let payload: &dyn Any = &self.payload; - payload.downcast_ref() + self.payload.as_any().downcast_ref() } } @@ -1567,9 +1566,15 @@ pub trait PyValue: fmt::Debug + 'static { fn required_type(ctx: &PyContext) -> PyObjectRef; } -pub trait PyObjectPayload: Any + fmt::Debug + 'static {} +pub trait PyObjectPayload: Any + fmt::Debug + 'static { + fn as_any(&self) -> &dyn Any; +} -impl PyObjectPayload for T {} +impl PyObjectPayload for T { + fn as_any(&self) -> &dyn Any { + self + } +} impl FromPyObjectRef for PyRef { fn from_pyobj(obj: &PyObjectRef) -> Self { From fb8351c5d1e1187bfab245b52270d81312e46bee Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Tue, 12 Mar 2019 20:59:03 -0500 Subject: [PATCH 6/9] Inline some functions --- vm/src/pyobject.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 9c6ae2a4fc..69021af7eb 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -1557,6 +1557,7 @@ impl PyObject { Rc::new(self) } + #[inline] pub fn payload(&self) -> Option<&T> { self.payload.as_any().downcast_ref() } @@ -1571,6 +1572,7 @@ pub trait PyObjectPayload: Any + fmt::Debug + 'static { } impl PyObjectPayload for T { + #[inline] fn as_any(&self) -> &dyn Any { self } From 4b655b9ab750730efa401b1a6f5ab03b0158cf3f Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Wed, 13 Mar 2019 22:48:16 -0500 Subject: [PATCH 7/9] Fix recursive Scope Debug impl --- vm/src/frame.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 2519733cb7..4183d4fce7 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -76,12 +76,19 @@ impl<'a, T> Iterator for Iter<'a, T> { } } -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct Scope { locals: RcList, pub globals: PyObjectRef, } +impl fmt::Debug for Scope { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // TODO: have a more informative Debug impl that DOESN'T recurse and cause a stack overflow + f.write_str("Scope") + } +} + impl Scope { pub fn new(locals: Option, globals: PyObjectRef) -> Scope { let locals = match locals { @@ -99,10 +106,7 @@ impl Scope { } pub fn get_only_locals(&self) -> Option { - match self.locals.iter().next() { - Some(dict) => Some(dict.clone()), - None => None, - } + self.locals.iter().next().cloned() } pub fn child_scope_with_locals(&self, locals: PyObjectRef) -> Scope { @@ -1212,21 +1216,18 @@ impl fmt::Debug for Frame { .borrow() .iter() .map(|elem| format!("\n > {:?}", elem)) - .collect::>() - .join(""); + .collect::(); let block_str = self .blocks .borrow() .iter() .map(|elem| format!("\n > {:?}", elem)) - .collect::>() - .join(""); + .collect::(); let local_str = match self.scope.get_locals().payload::() { Some(dict) => objdict::get_key_value_pairs_from_content(&dict.entries.borrow()) .iter() .map(|elem| format!("\n {:?} = {:?}", elem.0, elem.1)) - .collect::>() - .join(""), + .collect::(), None => panic!("locals unexpectedly not wrapping a dict!",), }; write!( From 2af998ea869e668f2e42eb16cfab25ccf58a6838 Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Thu, 14 Mar 2019 17:54:38 -0500 Subject: [PATCH 8/9] Prevent recursion for `Debug` impls --- vm/src/frame.rs | 9 ++++++++- vm/src/obj/objdict.rs | 10 +++++++++- vm/src/obj/objlist.rs | 10 +++++++++- vm/src/obj/objset.rs | 13 ++++++++++--- vm/src/obj/objtuple.rs | 10 +++++++++- 5 files changed, 45 insertions(+), 7 deletions(-) diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 4183d4fce7..a403814267 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -1211,11 +1211,18 @@ impl Frame { impl fmt::Debug for Frame { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + trace!("formatting stack"); let stack_str = self .stack .borrow() .iter() - .map(|elem| format!("\n > {:?}", elem)) + .map(|elem| { + if elem.payload.as_any().is::() { + "\n > {frame}".to_string() + } else { + format!("\n > {:?}", elem) + } + }) .collect::(); let block_str = self .blocks diff --git a/vm/src/obj/objdict.rs b/vm/src/obj/objdict.rs index 248ab3d539..2a0bffe45e 100644 --- a/vm/src/obj/objdict.rs +++ b/vm/src/obj/objdict.rs @@ -1,5 +1,6 @@ use std::cell::{Cell, RefCell}; use std::collections::HashMap; +use std::fmt; use std::ops::{Deref, DerefMut}; use crate::pyobject::{ @@ -14,13 +15,20 @@ use super::objtype; pub type DictContentType = HashMap; -#[derive(Default, Debug)] +#[derive(Default)] pub struct PyDict { // TODO: should be private pub entries: RefCell, } pub type PyDictRef = PyRef; +impl fmt::Debug for PyDict { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // TODO: implement more detailed, non-recursive Debug formatter + f.write_str("dict") + } +} + impl PyValue for PyDict { fn required_type(ctx: &PyContext) -> PyObjectRef { ctx.dict_type() diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index 290007e555..ff9d7c884c 100644 --- a/vm/src/obj/objlist.rs +++ b/vm/src/obj/objlist.rs @@ -14,13 +14,21 @@ use crate::pyobject::{ }; use crate::vm::{ReprGuard, VirtualMachine}; use num_traits::ToPrimitive; +use std::fmt; -#[derive(Debug, Default)] +#[derive(Default)] pub struct PyList { // TODO: shouldn't be public pub elements: RefCell>, } +impl fmt::Debug for PyList { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // TODO: implement more detailed, non-recursive Debug formatter + f.write_str("list") + } +} + impl From> for PyList { fn from(elements: Vec) -> Self { PyList { diff --git a/vm/src/obj/objset.rs b/vm/src/obj/objset.rs index bb8483a0e2..f46c6bcaed 100644 --- a/vm/src/obj/objset.rs +++ b/vm/src/obj/objset.rs @@ -3,8 +3,8 @@ */ use std::cell::{Cell, RefCell}; -use std::collections::hash_map::DefaultHasher; -use std::collections::HashMap; +use std::collections::{hash_map::DefaultHasher, HashMap}; +use std::fmt; use std::hash::{Hash, Hasher}; use super::objbool; @@ -17,11 +17,18 @@ use crate::pyobject::{ }; use crate::vm::{ReprGuard, VirtualMachine}; -#[derive(Debug, Default)] +#[derive(Default)] pub struct PySet { elements: RefCell>, } +impl fmt::Debug for PySet { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // TODO: implement more detailed, non-recursive Debug formatter + f.write_str("set") + } +} + impl PyValue for PySet { fn required_type(ctx: &PyContext) -> PyObjectRef { ctx.set_type() diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index 6be590a5ff..ccbfc46daf 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -1,4 +1,5 @@ use std::cell::{Cell, RefCell}; +use std::fmt; use std::hash::{Hash, Hasher}; use crate::pyobject::{ @@ -15,13 +16,20 @@ use super::objsequence::{ use super::objstr; use super::objtype; -#[derive(Debug, Default)] +#[derive(Default)] pub struct PyTuple { // TODO: shouldn't be public // TODO: tuples are immutable, remove this RefCell pub elements: RefCell>, } +impl fmt::Debug for PyTuple { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // TODO: implement more informational, non-recursive Debug formatter + f.write_str("tuple") + } +} + impl From> for PyTuple { fn from(elements: Vec) -> Self { PyTuple { From 8c1f18157d0d40455961b7666279217a44e05d0f Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Thu, 14 Mar 2019 20:41:23 -0500 Subject: [PATCH 9/9] Remove leftover trace from debugging --- vm/src/frame.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/vm/src/frame.rs b/vm/src/frame.rs index a403814267..8bf23bf986 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -1211,7 +1211,6 @@ impl Frame { impl fmt::Debug for Frame { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - trace!("formatting stack"); let stack_str = self .stack .borrow()