Use __setitem__ to store elements in list. Also refactor get_elements to return a reference to the elements, not a copy.

Author: windelbouwman

vm/src/builtins.rs

@@ -500,7 +500,7 @@ fn builtin_range(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
 // builtin_repr
 fn builtin_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     arg_check!(vm, args, required = [(obj, None)]);
-    vm.to_repr(obj.clone())
+    vm.to_repr(obj)
 }
 // builtin_reversed
 // builtin_round

vm/src/exceptions.rs

@@ -25,9 +25,9 @@ pub fn print_exception(vm: &mut VirtualMachine, exc: &PyObjectRef) {
     if let Some(tb) = exc.get_attr("__traceback__") {
         println!("Traceback (most recent call last):");
         if objtype::isinstance(&tb, &vm.ctx.list_type()) {
-            let mut elements = objlist::get_elements(&tb);
+            let mut elements = objlist::get_elements(&tb).to_vec();
             elements.reverse();
-            for element in elements {
+            for element in elements.iter() {
                 if objtype::isinstance(&element, &vm.ctx.tuple_type()) {
                     let element = objtuple::get_elements(&element);
                     let filename = if let Ok(x) = vm.to_str(element[0].clone()) {

vm/src/frame.rs

@@ -556,15 +556,14 @@ impl Frame {
                 match expr.borrow().kind {
                     PyObjectKind::None => (),
                     _ => {
-                        let repr = vm.to_repr(expr.clone()).unwrap();
+                        let repr = vm.to_repr(&expr)?;
                         builtins::builtin_print(
                             vm,
                             PyFuncArgs {
                                 args: vec![repr],
                                 kwargs: vec![],
                             },
-                        )
-                        .unwrap();
+                        )?;
                     }
                 }
                 Ok(None)
@@ -857,17 +856,8 @@ impl Frame {
         let idx = self.pop_value();
         let obj = self.pop_value();
         let value = self.pop_value();
-        let a2 = &mut *obj.borrow_mut();
-        match &mut a2.kind {
-            PyObjectKind::List { ref mut elements } => {
-                objlist::set_item(vm, elements, idx, value)?;
-                Ok(None)
-            }
-            _ => Err(vm.new_type_error(format!(
-                "TypeError: __setitem__ assign type {:?} with index {:?} is not supported (yet?)",
-                obj, idx
-            ))),
-        }
+        vm.call_method(&obj, "__setitem__", vec![idx, value])?;
+        Ok(None)
     }
 
     fn execute_delete_subscript(&mut self, vm: &mut VirtualMachine) -> FrameResult {

vm/src/obj/objbytes.rs

@@ -25,8 +25,8 @@ fn bytes_init(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     );
     let val = if objtype::isinstance(arg, &vm.ctx.list_type()) {
         let mut data_bytes = vec![];
-        for elem in objlist::get_elements(arg) {
-            let v = objint::to_int(vm, &elem, 10)?;
+        for elem in objlist::get_elements(arg).iter() {
+            let v = objint::to_int(vm, elem, 10)?;
             data_bytes.push(v.to_u8().unwrap());
         }
         data_bytes

vm/src/obj/objdict.rs

@@ -51,13 +51,9 @@ fn dict_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     let elements = get_elements(o);
     let mut str_parts = vec![];
     for elem in elements {
-        match vm.to_repr(elem.1) {
-            Ok(s) => {
-                let value_str = objstr::get_value(&s);
-                str_parts.push(format!("{}: {}", elem.0, value_str));
-            }
-            Err(err) => return Err(err),
-        }
+        let s = vm.to_repr(&elem.1)?;
+        let value_str = objstr::get_value(&s);
+        str_parts.push(format!("{}: {}", elem.0, value_str));
     }
 
     let s = format!("{{ {} }}", str_parts.join(", "));

vm/src/obj/objlist.rs

@@ -11,9 +11,11 @@ use super::objstr;
 use super::objtype;
 use num_bigint::ToBigInt;
 use num_traits::ToPrimitive;
+use std::cell::{Ref, RefMut};
+use std::ops::{Deref, DerefMut};
 
 // set_item:
-pub fn set_item(
+fn set_item(
     vm: &mut VirtualMachine,
     l: &mut Vec<PyObjectRef>,
     idx: PyObjectRef,
@@ -32,12 +34,26 @@ pub fn set_item(
     }
 }
 
-pub fn get_elements(obj: &PyObjectRef) -> Vec<PyObjectRef> {
-    if let PyObjectKind::List { elements } = &obj.borrow().kind {
-        elements.to_vec()
-    } else {
-        panic!("Cannot extract list elements from non-list");
-    }
+pub fn get_elements<'a>(obj: &'a PyObjectRef) -> impl Deref<Target = Vec<PyObjectRef>> + 'a {
+    Ref::map(obj.borrow(), |x| {
+        if let PyObjectKind::List { ref elements } = x.kind {
+            elements
+        } else {
+            panic!("Cannot extract list elements from non-list");
+        }
+    })
+}
+
+pub fn get_mut_elements<'a>(obj: &'a PyObjectRef) -> impl DerefMut<Target = Vec<PyObjectRef>> + 'a {
+    RefMut::map(obj.borrow_mut(), |x| {
+        if let PyObjectKind::List { ref mut elements } = x.kind {
+            elements
+        } else {
+            panic!("Cannot extract list elements from non-list");
+            // TODO: raise proper error?
+            // Err(vm.new_type_error("list.append is called with no list".to_string()))
+        }
+    })
 }
 
 fn list_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
@@ -76,7 +92,7 @@ fn list_eq(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     let result = if objtype::isinstance(other, &vm.ctx.list_type()) {
         let zelf = get_elements(zelf);
         let other = get_elements(other);
-        seq_equal(vm, zelf, other)?
+        seq_equal(vm, &zelf, &other)?
     } else {
         false
     };
@@ -105,7 +121,7 @@ fn list_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
 
     let elements = get_elements(o);
     let mut str_parts = vec![];
-    for elem in elements {
+    for elem in elements.iter() {
         let s = vm.to_repr(elem)?;
         str_parts.push(objstr::get_value(&s));
     }
@@ -121,25 +137,17 @@ pub fn list_append(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         args,
         required = [(list, Some(vm.ctx.list_type())), (x, None)]
     );
-    let mut list_obj = list.borrow_mut();
-    if let PyObjectKind::List { ref mut elements } = list_obj.kind {
-        elements.push(x.clone());
-        Ok(vm.get_none())
-    } else {
-        Err(vm.new_type_error("list.append is called with no list".to_string()))
-    }
+    let mut elements = get_mut_elements(list);
+    elements.push(x.clone());
+    Ok(vm.get_none())
 }
 
 fn list_clear(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     trace!("list.clear called with: {:?}", args);
     arg_check!(vm, args, required = [(list, Some(vm.ctx.list_type()))]);
-    let mut list_obj = list.borrow_mut();
-    if let PyObjectKind::List { ref mut elements } = list_obj.kind {
-        elements.clear();
-        Ok(vm.get_none())
-    } else {
-        Err(vm.new_type_error("list.clear is called with no list".to_string()))
-    }
+    let mut elements = get_mut_elements(list);
+    elements.clear();
+    Ok(vm.get_none())
 }
 
 pub fn list_extend(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
@@ -149,13 +157,9 @@ pub fn list_extend(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         required = [(list, Some(vm.ctx.list_type())), (x, None)]
     );
     let mut new_elements = vm.extract_elements(x)?;
-    let mut list_obj = list.borrow_mut();
-    if let PyObjectKind::List { ref mut elements } = list_obj.kind {
-        elements.append(&mut new_elements);
-        Ok(vm.get_none())
-    } else {
-        Err(vm.new_type_error("list.extend is called with no list".to_string()))
-    }
+    let mut elements = get_mut_elements(list);
+    elements.append(&mut new_elements);
+    Ok(vm.get_none())
 }
 
 fn list_len(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
@@ -168,13 +172,9 @@ fn list_len(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
 fn list_reverse(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     trace!("list.reverse called with: {:?}", args);
     arg_check!(vm, args, required = [(list, Some(vm.ctx.list_type()))]);
-    let mut list_obj = list.borrow_mut();
-    if let PyObjectKind::List { ref mut elements } = list_obj.kind {
-        elements.reverse();
-        Ok(vm.get_none())
-    } else {
-        Err(vm.new_type_error("list.reverse is called with no list".to_string()))
-    }
+    let mut elements = get_mut_elements(list);
+    elements.reverse();
+    Ok(vm.get_none())
 }
 
 fn list_contains(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
@@ -208,12 +208,23 @@ fn list_getitem(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     get_item(vm, list, &get_elements(list), needle.clone())
 }
 
+fn list_setitem(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
+    arg_check!(
+        vm,
+        args,
+        required = [(list, Some(vm.ctx.list_type())), (key, None), (value, None)]
+    );
+    let mut elements = get_mut_elements(list);
+    set_item(vm, &mut elements, key.clone(), value.clone())
+}
+
 pub fn init(context: &PyContext) {
     let ref list_type = context.list_type;
     list_type.set_attr("__add__", context.new_rustfunc(list_add));
     list_type.set_attr("__contains__", context.new_rustfunc(list_contains));
     list_type.set_attr("__eq__", context.new_rustfunc(list_eq));
     list_type.set_attr("__getitem__", context.new_rustfunc(list_getitem));
+    list_type.set_attr("__setitem__", context.new_rustfunc(list_setitem));
     list_type.set_attr("__len__", context.new_rustfunc(list_len));
     list_type.set_attr("__new__", context.new_rustfunc(list_new));
     list_type.set_attr("__repr__", context.new_rustfunc(list_repr));

vm/src/obj/objsequence.rs

@@ -104,8 +104,8 @@ pub fn get_item(
 
 pub fn seq_equal(
     vm: &mut VirtualMachine,
-    zelf: Vec<PyObjectRef>,
-    other: Vec<PyObjectRef>,
+    zelf: &Vec<PyObjectRef>,
+    other: &Vec<PyObjectRef>,
 ) -> Result<bool, PyObjectRef> {
     if zelf.len() == other.len() {
         for (a, b) in Iterator::zip(zelf.iter(), other.iter()) {

vm/src/obj/objset.rs

@@ -100,7 +100,7 @@ fn set_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     let elements = get_elements(o);
     let mut str_parts = vec![];
     for elem in elements.values() {
-        let part = vm.to_repr(elem.clone())?;
+        let part = vm.to_repr(elem)?;
         str_parts.push(objstr::get_value(&part));
     }
 

vm/src/obj/objtuple.rs

@@ -9,6 +9,8 @@ use super::objstr;
 use super::objtype;
 use num_bigint::ToBigInt;
 use num_traits::ToPrimitive;
+use std::cell::Ref;
+use std::ops::Deref;
 
 fn tuple_eq(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     arg_check!(
@@ -20,7 +22,7 @@ fn tuple_eq(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     let result = if objtype::isinstance(other, &vm.ctx.tuple_type()) {
         let zelf = get_elements(zelf);
         let other = get_elements(other);
-        seq_equal(vm, zelf, other)?
+        seq_equal(vm, &zelf, &other)?
     } else {
         false
     };
@@ -35,7 +37,7 @@ fn tuple_hash(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     let len: usize = elements.len();
     let mut mult = 0xf4243;
 
-    for elem in &elements {
+    for elem in elements.iter() {
         let y: usize = objint::get_value(&vm.call_method(elem, "__hash__", vec![])?)
             .to_usize()
             .unwrap();
@@ -59,7 +61,7 @@ fn tuple_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     let elements = get_elements(zelf);
 
     let mut str_parts = vec![];
-    for elem in elements {
+    for elem in elements.iter() {
         match vm.to_repr(elem) {
             Ok(s) => str_parts.push(objstr::get_value(&s)),
             Err(err) => return Err(err),
@@ -103,12 +105,14 @@ pub fn tuple_contains(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     Ok(vm.new_bool(false))
 }
 
-pub fn get_elements(obj: &PyObjectRef) -> Vec<PyObjectRef> {
-    if let PyObjectKind::Tuple { elements } = &obj.borrow().kind {
-        elements.to_vec()
-    } else {
-        panic!("Cannot extract elements from non-tuple");
-    }
+pub fn get_elements<'a>(obj: &'a PyObjectRef) -> impl Deref<Target = Vec<PyObjectRef>> + 'a {
+    Ref::map(obj.borrow(), |x| {
+        if let PyObjectKind::Tuple { ref elements } = x.kind {
+            elements
+        } else {
+            panic!("Cannot extract elements from non-tuple");
+        }
+    })
 }
 
 pub fn init(context: &PyContext) {

vm/src/stdlib/json.rs

@@ -35,10 +35,10 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> {
         S: serde::Serializer,
     {
         let serialize_seq_elements =
-            |serializer: S, elements: Vec<PyObjectRef>| -> Result<S::Ok, S::Error> {
+            |serializer: S, elements: &Vec<PyObjectRef>| -> Result<S::Ok, S::Error> {
                 let mut seq = serializer.serialize_seq(Some(elements.len()))?;
-                for e in elements {
-                    seq.serialize_element(&self.clone_with_object(&e))?;
+                for e in elements.iter() {
+                    seq.serialize_element(&self.clone_with_object(e))?;
                 }
                 seq.end()
             };
@@ -55,10 +55,10 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> {
         // v.serialize(serializer)
         } else if objtype::isinstance(self.pyobject, &self.ctx.list_type()) {
             let elements = objlist::get_elements(self.pyobject);
-            serialize_seq_elements(serializer, elements)
+            serialize_seq_elements(serializer, &elements)
         } else if objtype::isinstance(self.pyobject, &self.ctx.tuple_type()) {
             let elements = objtuple::get_elements(self.pyobject);
-            serialize_seq_elements(serializer, elements)
+            serialize_seq_elements(serializer, &elements)
         } else if objtype::isinstance(self.pyobject, &self.ctx.dict_type()) {
             let elements = objdict::get_elements(self.pyobject);
             let mut map = serializer.serialize_map(Some(elements.len()))?;