From 89cfdbe3ddf03e66e3db000ebd7b9fe671b75ea1 Mon Sep 17 00:00:00 2001 From: Windel Bouwman Date: Wed, 31 Oct 2018 23:18:26 +0100 Subject: [PATCH] Use __setitem__ to store elements in list. Also refactor get_elements to return a reference to the elements, not a copy. --- vm/src/builtins.rs | 2 +- vm/src/exceptions.rs | 4 +- vm/src/frame.rs | 18 ++------- vm/src/obj/objbytes.rs | 4 +- vm/src/obj/objdict.rs | 10 ++--- vm/src/obj/objlist.rs | 85 ++++++++++++++++++++++----------------- vm/src/obj/objsequence.rs | 4 +- vm/src/obj/objset.rs | 2 +- vm/src/obj/objtuple.rs | 22 +++++----- vm/src/stdlib/json.rs | 10 ++--- vm/src/stdlib/types.rs | 2 +- vm/src/vm.rs | 16 ++++---- 12 files changed, 90 insertions(+), 89 deletions(-) diff --git a/vm/src/builtins.rs b/vm/src/builtins.rs index fcfab37ed..8b7570b95 100644 --- a/vm/src/builtins.rs +++ b/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 diff --git a/vm/src/exceptions.rs b/vm/src/exceptions.rs index 94f917201..401688bd1 100644 --- a/vm/src/exceptions.rs +++ b/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()) { diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 6f4e678df..3112f1666 100644 --- a/vm/src/frame.rs +++ b/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 { diff --git a/vm/src/obj/objbytes.rs b/vm/src/obj/objbytes.rs index 0fce66e18..2df1d76c7 100644 --- a/vm/src/obj/objbytes.rs +++ b/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 diff --git a/vm/src/obj/objdict.rs b/vm/src/obj/objdict.rs index 5e991c6f7..3ae3ed403 100644 --- a/vm/src/obj/objdict.rs +++ b/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(", ")); diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index 4f823ab70..0cbb0dd76 100644 --- a/vm/src/obj/objlist.rs +++ b/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, idx: PyObjectRef, @@ -32,12 +34,26 @@ pub fn set_item( } } -pub fn get_elements(obj: &PyObjectRef) -> Vec { - 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> + '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> + '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)); diff --git a/vm/src/obj/objsequence.rs b/vm/src/obj/objsequence.rs index f98415238..5fb50fd2a 100644 --- a/vm/src/obj/objsequence.rs +++ b/vm/src/obj/objsequence.rs @@ -104,8 +104,8 @@ pub fn get_item( pub fn seq_equal( vm: &mut VirtualMachine, - zelf: Vec, - other: Vec, + zelf: &Vec, + other: &Vec, ) -> Result { if zelf.len() == other.len() { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { diff --git a/vm/src/obj/objset.rs b/vm/src/obj/objset.rs index 2ca2d9c64..2b60e7300 100644 --- a/vm/src/obj/objset.rs +++ b/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)); } diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index 0c64f5387..8dab5068c 100644 --- a/vm/src/obj/objtuple.rs +++ b/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 { - 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> + '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) { diff --git a/vm/src/stdlib/json.rs b/vm/src/stdlib/json.rs index 63015d68a..39be42387 100644 --- a/vm/src/stdlib/json.rs +++ b/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| -> Result { + |serializer: S, elements: &Vec| -> Result { 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()))?; diff --git a/vm/src/stdlib/types.rs b/vm/src/stdlib/types.rs index bb263d167..1706b3cec 100644 --- a/vm/src/stdlib/types.rs +++ b/vm/src/stdlib/types.rs @@ -22,7 +22,7 @@ fn types_new_class(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { let bases = match bases { Some(b) => { if objtype::isinstance(b, &vm.ctx.tuple_type()) { - objtuple::get_elements(b) + objtuple::get_elements(b).to_vec() } else { return Err(vm.new_type_error("Bases must be a tuple".to_string())); } diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 1b4125967..50fab0c51 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -17,10 +17,7 @@ use super::obj::objlist; use super::obj::objobject; use super::obj::objtuple; use super::obj::objtype; -use super::pyobject::{ - AttributeProtocol, DictProtocol, PyContext, PyFuncArgs, PyObjectKind, PyObjectRef, PyResult, - TypeProtocol, -}; +use super::pyobject::{DictProtocol, PyContext, PyFuncArgs, PyObjectKind, PyObjectRef, PyResult}; use super::stdlib; use super::sysmodule; @@ -146,8 +143,8 @@ impl VirtualMachine { self.call_method(&obj, "__str__", vec![]) } - pub fn to_repr(&mut self, obj: PyObjectRef) -> PyResult { - self.call_method(&obj, "__repr__", vec![]) + pub fn to_repr(&mut self, obj: &PyObjectRef) -> PyResult { + self.call_method(obj, "__repr__", vec![]) } pub fn call_method( @@ -363,9 +360,9 @@ impl VirtualMachine { ) -> Result, PyObjectRef> { // Extract elements from item, if possible: let elements = if objtype::isinstance(value, &self.ctx.tuple_type()) { - objtuple::get_elements(value) + objtuple::get_elements(value).to_vec() } else if objtype::isinstance(value, &self.ctx.list_type()) { - objlist::get_elements(value) + objlist::get_elements(value).to_vec() } else { let iter = objiter::get_iter(self, value)?; objiter::get_all(self, &iter)? @@ -379,6 +376,8 @@ impl VirtualMachine { pub fn _sub(&mut self, a: PyObjectRef, b: PyObjectRef) -> PyResult { // Try __sub__, next __rsub__, next, give up + self.call_method(&a, "__sub__", vec![b]) + /* if a.has_attr("__sub__") { self.call_method(&a, "__sub__", vec![b]) } else if b.has_attr("__rsub__") { @@ -392,6 +391,7 @@ impl VirtualMachine { a_type_name, b_type_name ))) } + */ } pub fn _add(&mut self, a: PyObjectRef, b: PyObjectRef) -> PyResult {