From 98a93b477d376b1066c0863d6cde751436ae8bfb Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 30 Aug 2018 16:34:28 -0400 Subject: [PATCH 1/5] Add tuple tests to json_snippet.py --- tests/snippets/json_snippet.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/snippets/json_snippet.py b/tests/snippets/json_snippet.py index 640dcf9f4..3d68c3846 100644 --- a/tests/snippets/json_snippet.py +++ b/tests/snippets/json_snippet.py @@ -17,6 +17,12 @@ assert '[1]' == json.dumps([1]) assert '[[1]]' == json.dumps([[1]]) round_trip_test([1, "string", 1.0, True]) +assert '[]' == json.dumps(()) +assert '[1]' == json.dumps((1,)) +assert '[[1]]' == json.dumps(((1,),)) +# tuples don't round-trip through json +assert [1, "string", 1.0, True] == json.loads(json.dumps((1, "string", 1.0, True))) + assert '{}' == json.dumps({}) # TODO: uncomment once dict comparison is implemented # round_trip_test({'a': 'b'}) From e0b71b8d93cffed7597a8001a406ea74b2009af1 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 30 Aug 2018 18:26:04 -0400 Subject: [PATCH 2/5] Add JSON snippet tests for None --- tests/snippets/json_snippet.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/snippets/json_snippet.py b/tests/snippets/json_snippet.py index 3d68c3846..d599f3219 100644 --- a/tests/snippets/json_snippet.py +++ b/tests/snippets/json_snippet.py @@ -11,6 +11,7 @@ assert "1" == json.dumps(1) assert "1.0" == json.dumps(1.0) assert "true" == json.dumps(True) assert "false" == json.dumps(False) +assert 'null' == json.dumps(None) assert '[]' == json.dumps([]) assert '[1]' == json.dumps([1]) From 934612314bde09e32ee05d6faafa6dd86f243ddb Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 30 Aug 2018 18:51:54 -0400 Subject: [PATCH 3/5] Add or make public get_elements for sequences/dicts --- vm/src/objdict.rs | 2 +- vm/src/objlist.rs | 2 +- vm/src/objsequence.rs | 8 ++++++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/vm/src/objdict.rs b/vm/src/objdict.rs index 92497f4bb..ece1df349 100644 --- a/vm/src/objdict.rs +++ b/vm/src/objdict.rs @@ -26,7 +26,7 @@ pub fn new(dict_type: PyObjectRef) -> PyObjectRef { ) } -fn get_elements(obj: &PyObjectRef) -> HashMap { +pub fn get_elements(obj: &PyObjectRef) -> HashMap { if let PyObjectKind::Dict { elements } = &obj.borrow().kind { elements.clone() } else { diff --git a/vm/src/objlist.rs b/vm/src/objlist.rs index 95d106596..48169c6fc 100644 --- a/vm/src/objlist.rs +++ b/vm/src/objlist.rs @@ -26,7 +26,7 @@ pub fn set_item( } } -fn get_elements(obj: PyObjectRef) -> Vec { +pub fn get_elements(obj: PyObjectRef) -> Vec { if let PyObjectKind::List { elements } = &obj.borrow().kind { elements.to_vec() } else { diff --git a/vm/src/objsequence.rs b/vm/src/objsequence.rs index b4c87d8ac..7647b0fcd 100644 --- a/vm/src/objsequence.rs +++ b/vm/src/objsequence.rs @@ -98,3 +98,11 @@ pub fn get_item( ))), } } + +pub fn get_elements(obj: PyObjectRef) -> Vec { + if let PyObjectKind::Tuple { elements } = &obj.borrow().kind { + elements.to_vec() + } else { + panic!("Cannot extract list elements from non-list"); + } +} From 3dca52cc4a1dbdf0beb8136050e6b4a2c2fdf75d Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 30 Aug 2018 18:52:15 -0400 Subject: [PATCH 4/5] Rework json.dumps to support subclasses We can currently only construct instances of string subclasses, so that's the only code path that is actually tested. Fixes #91. --- tests/snippets/json_snippet.py | 14 +++++++ vm/src/stdlib/json.rs | 72 ++++++++++++++++++++++++---------- 2 files changed, 65 insertions(+), 21 deletions(-) diff --git a/tests/snippets/json_snippet.py b/tests/snippets/json_snippet.py index d599f3219..389159aea 100644 --- a/tests/snippets/json_snippet.py +++ b/tests/snippets/json_snippet.py @@ -42,3 +42,17 @@ assert False == json.loads('false') assert [] == json.loads('[]') assert ['a'] == json.loads('["a"]') assert [['a'], 'b'] == json.loads('[["a"], "b"]') + +class String(str): pass + +assert '"string"' == json.dumps(String("string")) + +# TODO: Uncomment and test once int/float construction is supported +# class Int(int): pass +# class Float(float): pass + +# TODO: Uncomment and test once sequence/dict subclasses are supported by +# json.dumps +# class List(list): pass +# class Tuple(tuple): pass +# class Dict(dict): pass diff --git a/vm/src/stdlib/json.rs b/vm/src/stdlib/json.rs index 51cead0c9..8a970b664 100644 --- a/vm/src/stdlib/json.rs +++ b/vm/src/stdlib/json.rs @@ -6,41 +6,70 @@ use serde::de::Visitor; use serde::ser::{SerializeMap, SerializeSeq}; use serde_json; -use super::super::objtype; use super::super::pyobject::{ DictProtocol, PyContext, PyFuncArgs, PyObject, PyObjectKind, PyObjectRef, PyResult, TypeProtocol, }; use super::super::VirtualMachine; +use super::super::{objbool, objdict, objfloat, objint, objlist, objsequence, objstr, objtype}; -impl serde::Serialize for PyObjectKind { +// We need to have a VM available to serialise a PyObject based on its subclass, so we implement +// PyObject serialisation via a proxy object which holds a reference to a VM +struct PyObjectSerializer<'s> { + pyobject: &'s PyObjectRef, + vm: &'s VirtualMachine, +} + +impl<'s> serde::Serialize for PyObjectSerializer<'s> { fn serialize(&self, serializer: S) -> Result where S: serde::Serializer, { - match self { - PyObjectKind::String { value } => serializer.serialize_str(value), - PyObjectKind::Integer { value } => serializer.serialize_i32(*value), - PyObjectKind::Float { value } => serializer.serialize_f64(*value), - PyObjectKind::Boolean { value } => serializer.serialize_bool(*value), - PyObjectKind::List { elements } | PyObjectKind::Tuple { elements } => { + let serialize_seq_elements = + |serializer: S, elements: Vec| -> Result { let mut seq = serializer.serialize_seq(Some(elements.len()))?; for e in elements { - match e.borrow().kind { - ref kind => seq.serialize_element(kind)?, - } + seq.serialize_element(&PyObjectSerializer { + pyobject: &e, + vm: self.vm, + })?; } seq.end() + }; + if objtype::isinstance(self.pyobject.clone(), self.vm.ctx.str_type()) { + serializer.serialize_str(&objstr::get_value(&self.pyobject)) + } else if objtype::isinstance(self.pyobject.clone(), self.vm.ctx.int_type()) { + serializer.serialize_i32(objint::get_value(self.pyobject.clone())) + } else if objtype::isinstance(self.pyobject.clone(), self.vm.ctx.float_type()) { + serializer.serialize_f64(objfloat::get_value(self.pyobject.clone())) + } else if objtype::isinstance(self.pyobject.clone(), self.vm.ctx.bool_type()) { + serializer.serialize_bool(objbool::get_value(self.pyobject)) + } else if objtype::isinstance(self.pyobject.clone(), self.vm.ctx.list_type()) { + let elements = objlist::get_elements(self.pyobject.clone()); + serialize_seq_elements(serializer, elements) + } else if objtype::isinstance(self.pyobject.clone(), self.vm.ctx.tuple_type()) { + let elements = objsequence::get_elements(self.pyobject.clone()); + serialize_seq_elements(serializer, elements) + } else if objtype::isinstance(self.pyobject.clone(), self.vm.ctx.dict_type()) { + let elements = objdict::get_elements(self.pyobject); + let mut map = serializer.serialize_map(Some(elements.len()))?; + for (key, e) in elements { + map.serialize_entry( + &key, + &PyObjectSerializer { + pyobject: &e, + vm: self.vm, + }, + )?; } - PyObjectKind::Dict { elements } => { - let mut map = serializer.serialize_map(Some(elements.len()))?; - for (key, e) in elements { - map.serialize_entry(key, &e.borrow().kind)?; - } - map.end() - } - PyObjectKind::None => serializer.serialize_none(), - kind => unimplemented!("Object of type '{:?}' is not serializable", kind), + map.end() + } else if let PyObjectKind::None = self.pyobject.borrow().kind { + serializer.serialize_none() + } else { + unimplemented!( + "Object of type '{:?}' is not serializable", + self.pyobject.typ() + ); } } } @@ -172,7 +201,8 @@ fn dumps(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { // TODO: Implement non-trivial serialisation case arg_check!(vm, args, required = [(obj, None)]); // TODO: Raise an exception for serialisation errors - let string = serde_json::to_string(&obj.borrow().kind).unwrap(); + let serializer = PyObjectSerializer { pyobject: obj, vm }; + let string = serde_json::to_string(&serializer).unwrap(); Ok(vm.context().new_str(string)) } From 406b9a1c57f2f76d28db75ecdda186fe5f003655 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 30 Aug 2018 19:01:14 -0400 Subject: [PATCH 5/5] Add PyObjectSerializer method to reduce clutter --- vm/src/stdlib/json.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/vm/src/stdlib/json.rs b/vm/src/stdlib/json.rs index 8a970b664..d771cc907 100644 --- a/vm/src/stdlib/json.rs +++ b/vm/src/stdlib/json.rs @@ -20,6 +20,15 @@ struct PyObjectSerializer<'s> { vm: &'s VirtualMachine, } +impl<'s> PyObjectSerializer<'s> { + fn clone_with_object(&self, pyobject: &'s PyObjectRef) -> PyObjectSerializer { + PyObjectSerializer { + pyobject, + vm: self.vm, + } + } +} + impl<'s> serde::Serialize for PyObjectSerializer<'s> { fn serialize(&self, serializer: S) -> Result where @@ -29,10 +38,7 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> { |serializer: S, elements: Vec| -> Result { let mut seq = serializer.serialize_seq(Some(elements.len()))?; for e in elements { - seq.serialize_element(&PyObjectSerializer { - pyobject: &e, - vm: self.vm, - })?; + seq.serialize_element(&self.clone_with_object(&e))?; } seq.end() }; @@ -54,13 +60,7 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> { let elements = objdict::get_elements(self.pyobject); let mut map = serializer.serialize_map(Some(elements.len()))?; for (key, e) in elements { - map.serialize_entry( - &key, - &PyObjectSerializer { - pyobject: &e, - vm: self.vm, - }, - )?; + map.serialize_entry(&key, &self.clone_with_object(&e))?; } map.end() } else if let PyObjectKind::None = self.pyobject.borrow().kind {