Merge pull request RustPython#430 from RustPython/dict5

cthulahoops · web-flow · commit 1033f20e36ed · 2019-02-11T13:04:28.000Z
Change to more usage of VirtualMachine in anticipation of proper dicts
diff --git a/vm/src/builtins.rs b/vm/src/builtins.rs
@@ -29,7 +29,7 @@ fn get_locals(vm: &mut VirtualMachine) -> PyObjectRef {
     let locals = vm.get_locals();
     let key_value_pairs = objdict::get_key_value_pairs(&locals);
     for (key, value) in key_value_pairs {
-        objdict::set_item(&d, &key, &value);
+        objdict::set_item(&d, vm, &key, &value);
     }
     d
 }
@@ -656,7 +656,7 @@ pub fn make_module(ctx: &PyContext) -> PyObjectRef {
     //set __name__ fixes: https://github.com/RustPython/RustPython/issues/146
     ctx.set_attr(&py_mod, "__name__", ctx.new_str(String::from("__main__")));
 
-    ctx.set_item(&py_mod, "abs", ctx.new_rustfunc(builtin_abs));
+    ctx.set_attr(&py_mod, "abs", ctx.new_rustfunc(builtin_abs));
     ctx.set_attr(&py_mod, "all", ctx.new_rustfunc(builtin_all));
     ctx.set_attr(&py_mod, "any", ctx.new_rustfunc(builtin_any));
     ctx.set_attr(&py_mod, "bin", ctx.new_rustfunc(builtin_bin));
diff --git a/vm/src/frame.rs b/vm/src/frame.rs
@@ -247,11 +247,11 @@ impl Frame {
                         // Take all key-value pairs from the dict:
                         let dict_elements = objdict::get_key_value_pairs(&obj);
                         for (key, value) in dict_elements.iter() {
-                            objdict::set_item(&map_obj, key, value);
+                            objdict::set_item(&map_obj, vm, key, value);
                         }
                     } else {
                         let key = self.pop_value();
-                        objdict::set_item(&map_obj, &key, &obj);
+                        objdict::set_item(&map_obj, vm, &key, &obj);
                     }
                 }
                 self.push_value(map_obj);
diff --git a/vm/src/obj/objdict.rs b/vm/src/obj/objdict.rs
@@ -44,7 +44,13 @@ fn get_mut_elements<'a>(obj: &'a PyObjectRef) -> impl DerefMut<Target = DictCont
     })
 }
 
-pub fn set_item(dict: &PyObjectRef, needle: &PyObjectRef, value: &PyObjectRef) {
+pub fn set_item(
+    dict: &PyObjectRef,
+    _vm: &mut VirtualMachine,
+    needle: &PyObjectRef,
+    value: &PyObjectRef,
+) {
+    // TODO: use vm to call eventual __hash__ and __eq__methods.
     let mut elements = get_mut_elements(dict);
     set_item_in_content(&mut elements, needle, value);
 }
@@ -121,7 +127,7 @@ fn dict_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     if let Some(dict_obj) = dict_obj {
         if objtype::isinstance(&dict_obj, &vm.ctx.dict_type()) {
             for (needle, value) in get_key_value_pairs(&dict_obj) {
-                set_item(&dict, &needle, &value);
+                set_item(&dict, vm, &needle, &value);
             }
         } else {
             let iter = objiter::get_iter(vm, dict_obj)?;
@@ -139,12 +145,13 @@ fn dict_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
                 if objiter::get_next_object(vm, &elem_iter)?.is_some() {
                     return Err(err(vm));
                 }
-                set_item(&dict, &needle, &value);
+                set_item(&dict, vm, &needle, &value);
             }
         }
     }
     for (needle, value) in args.kwargs {
-        set_item(&dict, &vm.new_str(needle), &value);
+        let py_needle = vm.new_str(needle);
+        set_item(&dict, vm, &py_needle, &value);
     }
     Ok(dict)
 }
@@ -249,7 +256,7 @@ fn dict_setitem(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         ]
     );
 
-    set_item(dict, needle, value);
+    set_item(dict, vm, needle, value);
 
     Ok(vm.get_none())
 }
diff --git a/vm/src/stdlib/json.rs b/vm/src/stdlib/json.rs
@@ -17,14 +17,14 @@ use num_traits::cast::ToPrimitive;
 // PyObject serialisation via a proxy object which holds a reference to a VM
 struct PyObjectSerializer<'s> {
     pyobject: &'s PyObjectRef,
-    ctx: &'s PyContext,
+    vm: &'s VirtualMachine,
 }
 
 impl<'s> PyObjectSerializer<'s> {
     fn clone_with_object(&self, pyobject: &'s PyObjectRef) -> PyObjectSerializer {
         PyObjectSerializer {
             pyobject,
-            ctx: self.ctx,
+            vm: self.vm,
         }
     }
 }
@@ -42,23 +42,23 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> {
                 }
                 seq.end()
             };
-        if objtype::isinstance(self.pyobject, &self.ctx.str_type()) {
+        if objtype::isinstance(self.pyobject, &self.vm.ctx.str_type()) {
             serializer.serialize_str(&objstr::get_value(&self.pyobject))
-        } else if objtype::isinstance(self.pyobject, &self.ctx.float_type()) {
+        } else if objtype::isinstance(self.pyobject, &self.vm.ctx.float_type()) {
             serializer.serialize_f64(objfloat::get_value(self.pyobject))
-        } else if objtype::isinstance(self.pyobject, &self.ctx.bool_type()) {
+        } else if objtype::isinstance(self.pyobject, &self.vm.ctx.bool_type()) {
             serializer.serialize_bool(objbool::get_value(self.pyobject))
-        } else if objtype::isinstance(self.pyobject, &self.ctx.int_type()) {
+        } else if objtype::isinstance(self.pyobject, &self.vm.ctx.int_type()) {
             let v = objint::get_value(self.pyobject);
             serializer.serialize_i64(v.to_i64().unwrap())
         // Although this may seem nice, it does not give the right result:
         // v.serialize(serializer)
-        } else if objtype::isinstance(self.pyobject, &self.ctx.list_type())
-            || objtype::isinstance(self.pyobject, &self.ctx.tuple_type())
+        } else if objtype::isinstance(self.pyobject, &self.vm.ctx.list_type())
+            || objtype::isinstance(self.pyobject, &self.vm.ctx.tuple_type())
         {
             let elements = objsequence::get_elements(self.pyobject);
             serialize_seq_elements(serializer, &elements)
-        } else if objtype::isinstance(self.pyobject, &self.ctx.dict_type()) {
+        } else if objtype::isinstance(self.pyobject, &self.vm.ctx.dict_type()) {
             let pairs = objdict::get_elements(self.pyobject);
             let mut map = serializer.serialize_map(Some(pairs.len()))?;
             for (key, e) in pairs.iter() {
@@ -80,7 +80,7 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> {
 // creation
 #[derive(Clone)]
 struct PyObjectDeserializer<'c> {
-    ctx: &'c PyContext,
+    vm: &'c VirtualMachine,
 }
 
 impl<'de> serde::de::DeserializeSeed<'de> for PyObjectDeserializer<'de> {
@@ -90,121 +90,121 @@ impl<'de> serde::de::DeserializeSeed<'de> for PyObjectDeserializer<'de> {
     where
         D: serde::Deserializer<'de>,
     {
-        impl<'de> Visitor<'de> for PyObjectDeserializer<'de> {
-            type Value = PyObjectRef;
+        deserializer.deserialize_any(self.clone())
+    }
+}
 
-            fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
-                formatter.write_str("a type that can deserialise in Python")
-            }
+impl<'de> Visitor<'de> for PyObjectDeserializer<'de> {
+    type Value = PyObjectRef;
 
-            fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                Ok(self.ctx.new_str(value.to_string()))
-            }
+    fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+        formatter.write_str("a type that can deserialise in Python")
+    }
 
-            fn visit_string<E>(self, value: String) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                Ok(self.ctx.new_str(value))
-            }
+    fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        Ok(self.vm.ctx.new_str(value.to_string()))
+    }
 
-            fn visit_i64<E>(self, value: i64) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                // The JSON deserialiser always uses the i64/u64 deserialisers, so we only need to
-                // implement those for now
-                Ok(self.ctx.new_int(value))
-            }
+    fn visit_string<E>(self, value: String) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        Ok(self.vm.ctx.new_str(value))
+    }
 
-            fn visit_u64<E>(self, value: u64) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                // The JSON deserialiser always uses the i64/u64 deserialisers, so we only need to
-                // implement those for now
-                Ok(self.ctx.new_int(value))
-            }
+    fn visit_i64<E>(self, value: i64) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        // The JSON deserialiser always uses the i64/u64 deserialisers, so we only need to
+        // implement those for now
+        Ok(self.vm.ctx.new_int(value))
+    }
 
-            fn visit_f64<E>(self, value: f64) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                Ok(self.ctx.new_float(value))
-            }
+    fn visit_u64<E>(self, value: u64) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        // The JSON deserialiser always uses the i64/u64 deserialisers, so we only need to
+        // implement those for now
+        Ok(self.vm.ctx.new_int(value))
+    }
 
-            fn visit_bool<E>(self, value: bool) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                Ok(self.ctx.new_bool(value))
-            }
+    fn visit_f64<E>(self, value: f64) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        Ok(self.vm.ctx.new_float(value))
+    }
 
-            fn visit_seq<A>(self, mut access: A) -> Result<Self::Value, A::Error>
-            where
-                A: serde::de::SeqAccess<'de>,
-            {
-                let mut seq = Vec::with_capacity(access.size_hint().unwrap_or(0));
-                while let Some(value) = access.next_element_seed(self.clone())? {
-                    seq.push(value);
-                }
-                Ok(self.ctx.new_list(seq))
-            }
+    fn visit_bool<E>(self, value: bool) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        Ok(self.vm.ctx.new_bool(value))
+    }
 
-            fn visit_map<M>(self, mut access: M) -> Result<Self::Value, M::Error>
-            where
-                M: serde::de::MapAccess<'de>,
-            {
-                let dict = self.ctx.new_dict();
-                // TODO: Given keys must be strings, we can probably do something more efficient
-                // than wrapping the given object up and then unwrapping it to determine whether or
-                // not it is a string
-                while let Some((key_obj, value)) =
-                    access.next_entry_seed(self.clone(), self.clone())?
-                {
-                    let key = match key_obj.borrow().payload {
-                        PyObjectPayload::String { ref value } => value.clone(),
-                        _ => unimplemented!("map keys must be strings"),
-                    };
-                    self.ctx.set_item(&dict, &key, value);
-                }
-                Ok(dict)
-            }
+    fn visit_seq<A>(self, mut access: A) -> Result<Self::Value, A::Error>
+    where
+        A: serde::de::SeqAccess<'de>,
+    {
+        let mut seq = Vec::with_capacity(access.size_hint().unwrap_or(0));
+        while let Some(value) = access.next_element_seed(self.clone())? {
+            seq.push(value);
+        }
+        Ok(self.vm.ctx.new_list(seq))
+    }
 
-            fn visit_unit<E>(self) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                Ok(self.ctx.none.clone())
-            }
+    fn visit_map<M>(self, mut access: M) -> Result<Self::Value, M::Error>
+    where
+        M: serde::de::MapAccess<'de>,
+    {
+        let dict = self.vm.ctx.new_dict();
+        // TODO: Given keys must be strings, we can probably do something more efficient
+        // than wrapping the given object up and then unwrapping it to determine whether or
+        // not it is a string
+        while let Some((key_obj, value)) = access.next_entry_seed(self.clone(), self.clone())? {
+            let key = match key_obj.borrow().payload {
+                PyObjectPayload::String { ref value } => value.clone(),
+                _ => unimplemented!("map keys must be strings"),
+            };
+            self.vm.ctx.set_item(&dict, &key, value);
         }
+        Ok(dict)
+    }
 
-        deserializer.deserialize_any(self.clone())
+    fn visit_unit<E>(self) -> Result<Self::Value, E>
+    where
+        E: serde::de::Error,
+    {
+        Ok(self.vm.ctx.none.clone())
     }
 }
 
-fn dumps(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
+/// Implement json.dumps
+fn json_dumps(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     // TODO: Implement non-trivial serialisation case
     arg_check!(vm, args, required = [(obj, None)]);
     let res = {
         let serializer = PyObjectSerializer {
             pyobject: obj,
-            ctx: &vm.ctx,
+            vm: vm,
         };
         serde_json::to_string(&serializer)
     };
     let string = res.map_err(|err| vm.new_type_error(format!("{}", err)))?;
     Ok(vm.context().new_str(string))
 }
 
-fn loads(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
+/// Implement json.loads
+fn json_loads(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     // TODO: Implement non-trivial deserialisation case
     arg_check!(vm, args, required = [(string, Some(vm.ctx.str_type()))]);
     let res = {
-        let de = PyObjectDeserializer { ctx: &vm.ctx };
+        let de = PyObjectDeserializer { vm: vm };
         // TODO: Support deserializing string sub-classes
         de.deserialize(&mut serde_json::Deserializer::from_str(&objstr::get_value(
             &string,
@@ -221,17 +221,17 @@ fn loads(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
             .get_item("JSONDecodeError")
             .unwrap();
         let exc = vm.new_exception(json_decode_error, format!("{}", err));
-        vm.ctx.set_item(&exc, "lineno", vm.ctx.new_int(err.line()));
-        vm.ctx.set_item(&exc, "colno", vm.ctx.new_int(err.column()));
+        vm.ctx.set_attr(&exc, "lineno", vm.ctx.new_int(err.line()));
+        vm.ctx.set_attr(&exc, "colno", vm.ctx.new_int(err.column()));
         exc
     })
 }
 
 pub fn mk_module(ctx: &PyContext) -> PyObjectRef {
     let json_mod = ctx.new_module("json", ctx.new_scope(None));
 
-    ctx.set_attr(&json_mod, "dumps", ctx.new_rustfunc(dumps));
-    ctx.set_attr(&json_mod, "loads", ctx.new_rustfunc(loads));
+    ctx.set_attr(&json_mod, "dumps", ctx.new_rustfunc(json_dumps));
+    ctx.set_attr(&json_mod, "loads", ctx.new_rustfunc(json_loads));
 
     // TODO: Make this a proper type with a constructor
     let json_decode_error = create_type(
diff --git a/vm/src/sysmodule.rs b/vm/src/sysmodule.rs
@@ -126,16 +126,16 @@ settrace() -- set the global debug tracing function
 
     ctx.set_item(&modules, sys_name, sys_mod.clone());
 
-    ctx.set_item(&sys_mod, "modules", modules);
-    ctx.set_item(&sys_mod, "argv", argv(ctx));
-    ctx.set_item(&sys_mod, "getrefcount", ctx.new_rustfunc(sys_getrefcount));
-    ctx.set_item(&sys_mod, "getsizeof", ctx.new_rustfunc(sys_getsizeof));
-    ctx.set_item(&sys_mod, "maxsize", ctx.new_int(std::usize::MAX));
-    ctx.set_item(&sys_mod, "path", path);
-    ctx.set_item(&sys_mod, "ps1", ctx.new_str(">>>>> ".to_string()));
-    ctx.set_item(&sys_mod, "ps2", ctx.new_str("..... ".to_string()));
-    ctx.set_item(&sys_mod, "__doc__", ctx.new_str(sys_doc.to_string()));
-    ctx.set_item(&sys_mod, "_getframe", ctx.new_rustfunc(getframe));
+    ctx.set_attr(&sys_mod, "modules", modules);
+    ctx.set_attr(&sys_mod, "argv", argv(ctx));
+    ctx.set_attr(&sys_mod, "getrefcount", ctx.new_rustfunc(sys_getrefcount));
+    ctx.set_attr(&sys_mod, "getsizeof", ctx.new_rustfunc(sys_getsizeof));
+    ctx.set_attr(&sys_mod, "maxsize", ctx.new_int(std::usize::MAX));
+    ctx.set_attr(&sys_mod, "path", path);
+    ctx.set_attr(&sys_mod, "ps1", ctx.new_str(">>>>> ".to_string()));
+    ctx.set_attr(&sys_mod, "ps2", ctx.new_str("..... ".to_string()));
+    ctx.set_attr(&sys_mod, "__doc__", ctx.new_str(sys_doc.to_string()));
+    ctx.set_attr(&sys_mod, "_getframe", ctx.new_rustfunc(getframe));
 
     sys_mod
 }

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ fn get_locals(vm: &mut VirtualMachine) -> PyObjectRef {`
`29`	`29`	`let locals = vm.get_locals();`
`30`	`30`	`let key_value_pairs = objdict::get_key_value_pairs(&locals);`
`31`	`31`	`for (key, value) in key_value_pairs {`
`32`		`- objdict::set_item(&d, &key, &value);`
	`32`	`+ objdict::set_item(&d, vm, &key, &value);`
`33`	`33`	`}`
`34`	`34`	`d`
`35`	`35`	`}`
`@@ -656,7 +656,7 @@ pub fn make_module(ctx: &PyContext) -> PyObjectRef {`
`656`	`656`	`//set __name__ fixes: https://github.com/RustPython/RustPython/issues/146`
`657`	`657`	`ctx.set_attr(&py_mod, "__name__", ctx.new_str(String::from("__main__")));`
`658`	`658`
`659`		`- ctx.set_item(&py_mod, "abs", ctx.new_rustfunc(builtin_abs));`
	`659`	`+ ctx.set_attr(&py_mod, "abs", ctx.new_rustfunc(builtin_abs));`
`660`	`660`	`ctx.set_attr(&py_mod, "all", ctx.new_rustfunc(builtin_all));`
`661`	`661`	`ctx.set_attr(&py_mod, "any", ctx.new_rustfunc(builtin_any));`
`662`	`662`	`ctx.set_attr(&py_mod, "bin", ctx.new_rustfunc(builtin_bin));`
Original file line number	Diff line number	Diff line change
`@@ -247,11 +247,11 @@ impl Frame {`
`247`	`247`	`// Take all key-value pairs from the dict:`
`248`	`248`	`let dict_elements = objdict::get_key_value_pairs(&obj);`
`249`	`249`	`for (key, value) in dict_elements.iter() {`
`250`		`- objdict::set_item(&map_obj, key, value);`
	`250`	`+ objdict::set_item(&map_obj, vm, key, value);`
`251`	`251`	`}`
`252`	`252`	`} else {`
`253`	`253`	`let key = self.pop_value();`
`254`		`- objdict::set_item(&map_obj, &key, &obj);`
	`254`	`+ objdict::set_item(&map_obj, vm, &key, &obj);`
`255`	`255`	`}`
`256`	`256`	`}`
`257`	`257`	`self.push_value(map_obj);`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,13 @@ fn get_mut_elements<'a>(obj: &'a PyObjectRef) -> impl DerefMut<Target = DictCont`
`44`	`44`	`})`
`45`	`45`	`}`
`46`	`46`
`47`		`-pub fn set_item(dict: &PyObjectRef, needle: &PyObjectRef, value: &PyObjectRef) {`
	`47`	`+pub fn set_item(`
	`48`	`+ dict: &PyObjectRef,`
	`49`	`+ _vm: &mut VirtualMachine,`
	`50`	`+ needle: &PyObjectRef,`
	`51`	`+ value: &PyObjectRef,`
	`52`	`+) {`
	`53`	`+ // TODO: use vm to call eventual __hash__ and __eq__methods.`
`48`	`54`	`let mut elements = get_mut_elements(dict);`
`49`	`55`	`set_item_in_content(&mut elements, needle, value);`
`50`	`56`	`}`
`@@ -121,7 +127,7 @@ fn dict_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {`
`121`	`127`	`if let Some(dict_obj) = dict_obj {`
`122`	`128`	`if objtype::isinstance(&dict_obj, &vm.ctx.dict_type()) {`
`123`	`129`	`for (needle, value) in get_key_value_pairs(&dict_obj) {`
`124`		`- set_item(&dict, &needle, &value);`
	`130`	`+ set_item(&dict, vm, &needle, &value);`
`125`	`131`	`}`
`126`	`132`	`} else {`
`127`	`133`	`let iter = objiter::get_iter(vm, dict_obj)?;`
`@@ -139,12 +145,13 @@ fn dict_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {`
`139`	`145`	`if objiter::get_next_object(vm, &elem_iter)?.is_some() {`
`140`	`146`	`return Err(err(vm));`
`141`	`147`	`}`
`142`		`- set_item(&dict, &needle, &value);`
	`148`	`+ set_item(&dict, vm, &needle, &value);`
`143`	`149`	`}`
`144`	`150`	`}`
`145`	`151`	`}`
`146`	`152`	`for (needle, value) in args.kwargs {`
`147`		`- set_item(&dict, &vm.new_str(needle), &value);`
	`153`	`+ let py_needle = vm.new_str(needle);`
	`154`	`+ set_item(&dict, vm, &py_needle, &value);`
`148`	`155`	`}`
`149`	`156`	`Ok(dict)`
`150`	`157`	`}`
`@@ -249,7 +256,7 @@ fn dict_setitem(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {`
`249`	`256`	`]`
`250`	`257`	`);`
`251`	`258`
`252`		`- set_item(dict, needle, value);`
	`259`	`+ set_item(dict, vm, needle, value);`
`253`	`260`
`254`	`261`	`Ok(vm.get_none())`
`255`	`262`	`}`