Merge pull request #683 from RustPython/objdict_refactor

Objdict refactor

vm/src/builtins.rs

@@ -605,8 +605,7 @@ pub fn builtin_print(
         } else {
             write!(stdout_lock, " ").unwrap();
         }
-        let v = vm.to_str(&object)?;
-        let s = objstr::borrow_value(&v);
+        let s = &vm.to_str(&object)?.value;
         write!(stdout_lock, "{}", s).unwrap();
     }
 
@@ -623,9 +622,8 @@ pub fn builtin_print(
     Ok(())
 }
 
-fn builtin_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(vm, args, required = [(obj, None)]);
-    vm.to_repr(obj)
+fn builtin_repr(obj: PyObjectRef, vm: &mut VirtualMachine) -> PyResult<PyStringRef> {
+    vm.to_repr(&obj)
 }
 
 fn builtin_reversed(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {

vm/src/exceptions.rs

@@ -1,6 +1,5 @@
 use crate::function::PyFuncArgs;
 use crate::obj::objsequence;
-use crate::obj::objstr;
 use crate::obj::objtype;
 use crate::pyobject::{
     create_type, AttributeProtocol, PyContext, PyObjectRef, PyResult, TypeProtocol,
@@ -31,19 +30,19 @@ pub fn print_exception(vm: &mut VirtualMachine, exc: &PyObjectRef) {
                 if objtype::isinstance(&element, &vm.ctx.tuple_type()) {
                     let element = objsequence::get_elements(&element);
                     let filename = if let Ok(x) = vm.to_str(&element[0]) {
-                        objstr::get_value(&x)
+                        x.value.clone()
                     } else {
                         "<error>".to_string()
                     };
 
                     let lineno = if let Ok(x) = vm.to_str(&element[1]) {
-                        objstr::get_value(&x)
+                        x.value.clone()
                     } else {
                         "<error>".to_string()
                     };
 
                     let obj_name = if let Ok(x) = vm.to_str(&element[2]) {
-                        objstr::get_value(&x)
+                        x.value.clone()
                     } else {
                         "<error>".to_string()
                     };
@@ -59,7 +58,7 @@ pub fn print_exception(vm: &mut VirtualMachine, exc: &PyObjectRef) {
     }
 
     match vm.to_str(exc) {
-        Ok(txt) => println!("{}", objstr::get_value(&txt)),
+        Ok(txt) => println!("{}", txt.value),
         Err(err) => println!("Error during error {:?}", err),
     }
 }

vm/src/frame.rs

@@ -694,7 +694,7 @@ impl Frame {
                     let repr = vm.to_repr(&expr)?;
                     // TODO: implement sys.displayhook
                     if let Some(print) = vm.ctx.get_attr(&vm.builtins, "print") {
-                        vm.invoke(print, vec![repr])?;
+                        vm.invoke(print, vec![repr.into_object()])?;
                     }
                 }
                 Ok(None)
@@ -763,8 +763,8 @@ impl Frame {
             bytecode::Instruction::FormatValue { conversion, spec } => {
                 use ast::ConversionFlag::*;
                 let value = match conversion {
-                    Some(Str) => vm.to_str(&self.pop_value())?,
-                    Some(Repr) => vm.to_repr(&self.pop_value())?,
+                    Some(Str) => vm.to_str(&self.pop_value())?.into_object(),
+                    Some(Repr) => vm.to_repr(&self.pop_value())?.into_object(),
                     Some(Ascii) => self.pop_value(), // TODO
                     None => self.pop_value(),
                 };

vm/src/obj/objdict.rs

@@ -3,15 +3,14 @@ use std::collections::HashMap;
 use std::fmt;
 use std::ops::{Deref, DerefMut};
 
-use crate::function::PyFuncArgs;
+use crate::function::{OptionalArg, PyFuncArgs};
 use crate::pyobject::{
-    PyAttributes, PyContext, PyIteratorValue, PyObject, PyObjectRef, PyRef, PyResult, PyValue,
-    TypeProtocol,
+    PyAttributes, PyContext, PyIteratorValue, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol,
 };
 use crate::vm::{ReprGuard, VirtualMachine};
 
 use super::objiter;
-use super::objstr;
+use super::objstr::{self, PyStringRef};
 use super::objtype;
 
 pub type DictContentType = HashMap<String, (PyObjectRef, PyObjectRef)>;
@@ -134,7 +133,6 @@ pub fn attributes_to_py_dict(vm: &mut VirtualMachine, attributes: PyAttributes)
 }
 
 // Python dict methods:
-
 fn dict_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     arg_check!(
         vm,
@@ -175,231 +173,144 @@ fn dict_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     Ok(dict)
 }
 
-fn dict_len(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(vm, args, required = [(dict_obj, Some(vm.ctx.dict_type()))]);
-    let elements = get_elements(dict_obj);
-    Ok(vm.ctx.new_int(elements.len()))
-}
+impl PyDictRef {
+    fn len(self, _vm: &mut VirtualMachine) -> usize {
+        self.entries.borrow().len()
+    }
 
-fn dict_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(vm, args, required = [(dict_obj, Some(vm.ctx.dict_type()))]);
+    fn repr(self, vm: &mut VirtualMachine) -> PyResult {
+        let s = if let Some(_guard) = ReprGuard::enter(self.as_object()) {
+            let elements = get_key_value_pairs(self.as_object());
+            let mut str_parts = vec![];
+            for (key, value) in elements {
+                let key_repr = vm.to_repr(&key)?;
+                let value_repr = vm.to_repr(&value)?;
+                str_parts.push(format!("{}: {}", key_repr.value, value_repr.value));
+            }
 
-    let s = if let Some(_guard) = ReprGuard::enter(dict_obj) {
-        let elements = get_key_value_pairs(dict_obj);
-        let mut str_parts = vec![];
-        for (key, value) in elements {
-            let key_repr = vm.to_repr(&key)?;
-            let value_repr = vm.to_repr(&value)?;
-            let key_str = objstr::get_value(&key_repr);
-            let value_str = objstr::get_value(&value_repr);
-            str_parts.push(format!("{}: {}", key_str, value_str));
-        }
+            format!("{{{}}}", str_parts.join(", "))
+        } else {
+            "{...}".to_string()
+        };
+        Ok(vm.new_str(s))
+    }
 
-        format!("{{{}}}", str_parts.join(", "))
-    } else {
-        "{...}".to_string()
-    };
-    Ok(vm.new_str(s))
-}
+    fn contains(self, key: PyStringRef, _vm: &mut VirtualMachine) -> bool {
+        self.entries.borrow().contains_key(&key.value)
+    }
 
-pub fn dict_contains(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(
-        vm,
-        args,
-        required = [
-            (dict, Some(vm.ctx.dict_type())),
-            (needle, Some(vm.ctx.str_type()))
-        ]
-    );
-
-    let needle = objstr::get_value(&needle);
-    for element in get_elements(dict).iter() {
-        if &needle == element.0 {
-            return Ok(vm.new_bool(true));
+    fn delitem(self, key: PyStringRef, vm: &mut VirtualMachine) -> PyResult<()> {
+        let key = &key.value;
+        // Delete the item:
+        let mut elements = self.entries.borrow_mut();
+        match elements.remove(key) {
+            Some(_) => Ok(()),
+            None => Err(vm.new_value_error(format!("Key not found: {}", key))),
         }
     }
 
-    Ok(vm.new_bool(false))
-}
-
-fn dict_delitem(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(
-        vm,
-        args,
-        required = [
-            (dict, Some(vm.ctx.dict_type())),
-            (needle, Some(vm.ctx.str_type()))
-        ]
-    );
-
-    // What we are looking for:
-    let needle = objstr::get_value(&needle);
-
-    // Delete the item:
-    let mut elements = get_mut_elements(dict);
-    match elements.remove(&needle) {
-        Some(_) => Ok(vm.get_none()),
-        None => Err(vm.new_value_error(format!("Key not found: {}", needle))),
+    fn clear(self, _vm: &mut VirtualMachine) {
+        self.entries.borrow_mut().clear()
     }
-}
 
-fn dict_clear(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(vm, args, required = [(dict, Some(vm.ctx.dict_type()))]);
-    get_mut_elements(dict).clear();
-    Ok(vm.get_none())
-}
+    /// When iterating over a dictionary, we iterate over the keys of it.
+    fn iter(self, vm: &mut VirtualMachine) -> PyIteratorValue {
+        let keys = self
+            .entries
+            .borrow()
+            .values()
+            .map(|(k, _v)| k.clone())
+            .collect();
+        let key_list = vm.ctx.new_list(keys);
 
-/// When iterating over a dictionary, we iterate over the keys of it.
-fn dict_iter(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(vm, args, required = [(dict, Some(vm.ctx.dict_type()))]);
-
-    let keys = get_elements(dict)
-        .values()
-        .map(|(k, _v)| k.clone())
-        .collect();
-    let key_list = vm.ctx.new_list(keys);
-
-    let iter_obj = PyObject::new(
         PyIteratorValue {
             position: Cell::new(0),
             iterated_obj: key_list,
-        },
-        vm.ctx.iter_type(),
-    );
+        }
+    }
 
-    Ok(iter_obj)
-}
+    fn values(self, vm: &mut VirtualMachine) -> PyIteratorValue {
+        let values = self
+            .entries
+            .borrow()
+            .values()
+            .map(|(_k, v)| v.clone())
+            .collect();
+        let values_list = vm.ctx.new_list(values);
 
-fn dict_values(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(vm, args, required = [(dict, Some(vm.ctx.dict_type()))]);
-
-    let values = get_elements(dict)
-        .values()
-        .map(|(_k, v)| v.clone())
-        .collect();
-    let values_list = vm.ctx.new_list(values);
-
-    let iter_obj = PyObject::new(
         PyIteratorValue {
             position: Cell::new(0),
             iterated_obj: values_list,
-        },
-        vm.ctx.iter_type(),
-    );
+        }
+    }
 
-    Ok(iter_obj)
-}
+    fn items(self, vm: &mut VirtualMachine) -> PyIteratorValue {
+        let items = self
+            .entries
+            .borrow()
+            .values()
+            .map(|(k, v)| vm.ctx.new_tuple(vec![k.clone(), v.clone()]))
+            .collect();
+        let items_list = vm.ctx.new_list(items);
 
-fn dict_items(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(vm, args, required = [(dict, Some(vm.ctx.dict_type()))]);
-
-    let items = get_elements(dict)
-        .values()
-        .map(|(k, v)| vm.ctx.new_tuple(vec![k.clone(), v.clone()]))
-        .collect();
-    let items_list = vm.ctx.new_list(items);
-
-    let iter_obj = PyObject::new(
         PyIteratorValue {
             position: Cell::new(0),
             iterated_obj: items_list,
-        },
-        vm.ctx.iter_type(),
-    );
-
-    Ok(iter_obj)
-}
-
-fn dict_setitem(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(
-        vm,
-        args,
-        required = [
-            (dict, Some(vm.ctx.dict_type())),
-            (needle, Some(vm.ctx.str_type())),
-            (value, None)
-        ]
-    );
-
-    set_item(dict, vm, needle, value);
-
-    Ok(vm.get_none())
-}
-
-fn dict_getitem(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(
-        vm,
-        args,
-        required = [
-            (dict, Some(vm.ctx.dict_type())),
-            (needle, Some(vm.ctx.str_type()))
-        ]
-    );
-
-    // What we are looking for:
-    let needle = objstr::get_value(&needle);
-
-    let elements = get_elements(dict);
-    if elements.contains_key(&needle) {
-        Ok(elements[&needle].1.clone())
-    } else {
-        Err(vm.new_value_error(format!("Key not found: {}", needle)))
+        }
     }
-}
 
-fn dict_get(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(
-        vm,
-        args,
-        required = [
-            (dict, Some(vm.ctx.dict_type())),
-            (key, Some(vm.ctx.str_type()))
-        ],
-        optional = [(default, None)]
-    );
+    fn setitem(self, needle: PyObjectRef, value: PyObjectRef, _vm: &mut VirtualMachine) {
+        let mut elements = self.entries.borrow_mut();
+        set_item_in_content(&mut elements, &needle, &value)
+    }
 
-    // What we are looking for:
-    let key = objstr::get_value(&key);
+    fn getitem(self, key: PyStringRef, vm: &mut VirtualMachine) -> PyResult {
+        let key = &key.value;
 
-    let elements = get_elements(dict);
-    if elements.contains_key(&key) {
-        Ok(elements[&key].1.clone())
-    } else if let Some(value) = default {
-        Ok(value.clone())
-    } else {
-        Ok(vm.get_none())
+        // What we are looking for:
+        let elements = self.entries.borrow();
+        if elements.contains_key(key) {
+            Ok(elements[key].1.clone())
+        } else {
+            Err(vm.new_value_error(format!("Key not found: {}", key)))
+        }
+    }
+
+    fn get(
+        self,
+        key: PyStringRef,
+        default: OptionalArg<PyObjectRef>,
+        vm: &mut VirtualMachine,
+    ) -> PyObjectRef {
+        // What we are looking for:
+        let key = &key.value;
+
+        let elements = self.entries.borrow();
+        if elements.contains_key(key) {
+            elements[key].1.clone()
+        } else {
+            match default {
+                OptionalArg::Present(value) => value,
+                OptionalArg::Missing => vm.ctx.none(),
+            }
+        }
     }
 }
 
 pub fn init(context: &PyContext) {
-    let dict_type = &context.dict_type;
-    context.set_attr(&dict_type, "__len__", context.new_rustfunc(dict_len));
-    context.set_attr(
-        &dict_type,
-        "__contains__",
-        context.new_rustfunc(dict_contains),
-    );
-    context.set_attr(
-        &dict_type,
-        "__delitem__",
-        context.new_rustfunc(dict_delitem),
-    );
-    context.set_attr(
-        &dict_type,
-        "__getitem__",
-        context.new_rustfunc(dict_getitem),
-    );
-    context.set_attr(&dict_type, "__iter__", context.new_rustfunc(dict_iter));
-    context.set_attr(&dict_type, "__new__", context.new_rustfunc(dict_new));
-    context.set_attr(&dict_type, "__repr__", context.new_rustfunc(dict_repr));
-    context.set_attr(
-        &dict_type,
-        "__setitem__",
-        context.new_rustfunc(dict_setitem),
-    );
-    context.set_attr(&dict_type, "clear", context.new_rustfunc(dict_clear));
-    context.set_attr(&dict_type, "values", context.new_rustfunc(dict_values));
-    context.set_attr(&dict_type, "items", context.new_rustfunc(dict_items));
-    context.set_attr(&dict_type, "keys", context.new_rustfunc(dict_iter));
-    context.set_attr(&dict_type, "get", context.new_rustfunc(dict_get));
+    extend_class!(context, &context.dict_type, {
+        "__len__" => context.new_rustfunc(PyDictRef::len),
+        "__contains__" => context.new_rustfunc(PyDictRef::contains),
+        "__delitem__" => context.new_rustfunc(PyDictRef::delitem),
+        "__getitem__" => context.new_rustfunc(PyDictRef::getitem),
+        "__iter__" => context.new_rustfunc(PyDictRef::iter),
+        "__new__" => context.new_rustfunc(dict_new),
+        "__repr__" => context.new_rustfunc(PyDictRef::repr),
+        "__setitem__" => context.new_rustfunc(PyDictRef::setitem),
+        "clear" => context.new_rustfunc(PyDictRef::clear),
+        "values" => context.new_rustfunc(PyDictRef::values),
+        "items" => context.new_rustfunc(PyDictRef::items),
+        "keys" => context.new_rustfunc(PyDictRef::iter),
+        "get" => context.new_rustfunc(PyDictRef::get),
+    });
 }

vm/src/obj/objlist.rs

@@ -16,7 +16,6 @@ use super::objsequence::{
     get_elements, get_elements_cell, get_item, seq_equal, seq_ge, seq_gt, seq_le, seq_lt, seq_mul,
     PySliceableSequence,
 };
-use super::objstr;
 use super::objtype;
 
 #[derive(Default)]
@@ -154,7 +153,7 @@ impl PyListRef {
             let mut str_parts = vec![];
             for elem in self.elements.borrow().iter() {
                 let s = vm.to_repr(elem)?;
-                str_parts.push(objstr::get_value(&s));
+                str_parts.push(s.value.clone());
             }
             format!("[{}]", str_parts.join(", "))
         } else {
@@ -207,7 +206,7 @@ impl PyListRef {
                 return Ok(index);
             }
         }
-        let needle_str = objstr::get_value(&vm.to_str(&needle).unwrap());
+        let ref needle_str = vm.to_str(&needle)?.value;
         Err(vm.new_value_error(format!("'{}' is not in list", needle_str)))
     }
 
@@ -244,7 +243,7 @@ impl PyListRef {
             self.elements.borrow_mut().remove(index);
             Ok(())
         } else {
-            let needle_str = objstr::get_value(&vm.to_str(&needle)?);
+            let ref needle_str = vm.to_str(&needle)?.value;
             Err(vm.new_value_error(format!("'{}' is not in list", needle_str)))
         }
     }

vm/src/obj/objobject.rs

@@ -1,3 +1,6 @@
+use super::objlist::PyList;
+use super::objstr::{self, PyStringRef};
+use super::objtype;
 use crate::function::PyFuncArgs;
 use crate::obj::objproperty::PropertyBuilder;
 use crate::pyobject::{
@@ -6,10 +9,6 @@ use crate::pyobject::{
 };
 use crate::vm::VirtualMachine;
 
-use super::objlist::PyList;
-use super::objstr;
-use super::objtype;
-
 #[derive(Clone, Debug)]
 pub struct PyInstance;
 
@@ -136,17 +135,13 @@ pub fn object_dir(obj: PyObjectRef, vm: &mut VirtualMachine) -> PyList {
     PyList::from(attributes)
 }
 
-fn object_format(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(
-        vm,
-        args,
-        required = [
-            (obj, Some(vm.ctx.object())),
-            (format_spec, Some(vm.ctx.str_type()))
-        ]
-    );
-    if objstr::get_value(format_spec).is_empty() {
-        vm.to_str(obj)
+fn object_format(
+    obj: PyObjectRef,
+    format_spec: PyStringRef,
+    vm: &mut VirtualMachine,
+) -> PyResult<PyStringRef> {
+    if format_spec.value.is_empty() {
+        vm.to_str(&obj)
     } else {
         Err(vm.new_type_error("unsupported format string passed to object.__format__".to_string()))
     }

vm/src/obj/objset.rs

@@ -16,7 +16,6 @@ use crate::vm::{ReprGuard, VirtualMachine};
 use super::objbool;
 use super::objint;
 use super::objiter;
-use super::objstr;
 use super::objtype;
 
 #[derive(Default)]
@@ -213,7 +212,7 @@ fn set_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         let mut str_parts = vec![];
         for elem in elements.values() {
             let part = vm.to_repr(elem)?;
-            str_parts.push(objstr::get_value(&part));
+            str_parts.push(part.value.clone());
         }
 
         format!("{{{}}}", str_parts.join(", "))
@@ -584,7 +583,7 @@ fn frozenset_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         let mut str_parts = vec![];
         for elem in elements.values() {
             let part = vm.to_repr(elem)?;
-            str_parts.push(objstr::get_value(&part));
+            str_parts.push(part.value.clone());
         }
 
         format!("frozenset({{{}}})", str_parts.join(", "))

vm/src/obj/objstr.rs

@@ -804,7 +804,7 @@ fn str_new(
     vm: &mut VirtualMachine,
 ) -> PyResult<PyStringRef> {
     let string = match object {
-        OptionalArg::Present(ref input) => vm.to_str(input)?,
+        OptionalArg::Present(ref input) => vm.to_str(input)?.into_object(),
         OptionalArg::Missing => vm.new_str("".to_string()),
     };
     if string.typ().is(&cls) {

vm/src/obj/objtuple.rs

@@ -13,7 +13,6 @@ use super::objint;
 use super::objsequence::{
     get_elements, get_item, seq_equal, seq_ge, seq_gt, seq_le, seq_lt, seq_mul,
 };
-use super::objstr;
 use super::objtype;
 
 #[derive(Default)]
@@ -152,7 +151,7 @@ impl PyTupleRef {
             let mut str_parts = vec![];
             for elem in self.elements.borrow().iter() {
                 let s = vm.to_repr(elem)?;
-                str_parts.push(objstr::get_value(&s));
+                str_parts.push(s.value.clone());
             }
 
             if str_parts.len() == 1 {

vm/src/vm.rs

@@ -24,11 +24,12 @@ use crate::obj::objgenerator;
 use crate::obj::objiter;
 use crate::obj::objlist::PyList;
 use crate::obj::objsequence;
-use crate::obj::objstr;
+use crate::obj::objstr::PyStringRef;
 use crate::obj::objtuple::PyTuple;
 use crate::obj::objtype;
 use crate::pyobject::{
-    AttributeProtocol, DictProtocol, IdProtocol, PyContext, PyObjectRef, PyResult, TypeProtocol,
+    AttributeProtocol, DictProtocol, IdProtocol, PyContext, PyObjectRef, PyResult, TryFromObject,
+    TypeProtocol,
 };
 use crate::stdlib;
 use crate::sysmodule;
@@ -228,17 +229,19 @@ impl VirtualMachine {
     }
 
     // Container of the virtual machine state:
-    pub fn to_str(&mut self, obj: &PyObjectRef) -> PyResult {
-        self.call_method(&obj, "__str__", vec![])
+    pub fn to_str(&mut self, obj: &PyObjectRef) -> PyResult<PyStringRef> {
+        let str = self.call_method(&obj, "__str__", vec![])?;
+        TryFromObject::try_from_object(self, str)
     }
 
     pub fn to_pystr(&mut self, obj: &PyObjectRef) -> Result<String, PyObjectRef> {
         let py_str_obj = self.to_str(obj)?;
-        Ok(objstr::get_value(&py_str_obj))
+        Ok(py_str_obj.value.clone())
     }
 
-    pub fn to_repr(&mut self, obj: &PyObjectRef) -> PyResult {
-        self.call_method(obj, "__repr__", vec![])
+    pub fn to_repr(&mut self, obj: &PyObjectRef) -> PyResult<PyStringRef> {
+        let repr = self.call_method(obj, "__repr__", vec![])?;
+        TryFromObject::try_from_object(self, repr)
     }
 
     pub fn import(&mut self, module: &str) -> PyResult {

wasm/lib/src/browser_module.rs

@@ -75,9 +75,9 @@ fn browser_fetch(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     if let Some(headers) = headers {
         let h = request.headers();
         for (key, value) in rustpython_vm::obj::objdict::get_key_value_pairs(&headers) {
-            let key = objstr::get_value(&vm.to_str(&key)?);
-            let value = objstr::get_value(&vm.to_str(&value)?);
-            h.set(&key, &value)
+            let ref key = vm.to_str(&key)?.value;
+            let ref value = vm.to_str(&value)?.value;
+            h.set(key, value)
                 .map_err(|err| convert::js_py_typeerror(vm, err))?;
         }
     }