From 225f29601851a5e3d92543477e91be1a844b0cd0 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 7 Aug 2018 15:42:16 -0400 Subject: [PATCH 1/3] Refactor objlist and objtuple get_item in to objsequence --- vm/src/lib.rs | 1 - vm/src/objlist.rs | 28 ---------------------------- vm/src/objsequence.rs | 42 +++++++++++++++++++++++++++++++++++++++++- vm/src/objtuple.rs | 30 ------------------------------ vm/src/vm.rs | 6 +++--- 5 files changed, 44 insertions(+), 63 deletions(-) delete mode 100644 vm/src/objtuple.rs diff --git a/vm/src/lib.rs b/vm/src/lib.rs index bf6122899..40190a12f 100644 --- a/vm/src/lib.rs +++ b/vm/src/lib.rs @@ -16,7 +16,6 @@ mod objint; mod objlist; mod objsequence; mod objstr; -mod objtuple; mod objtype; mod objbool; pub mod pyobject; diff --git a/vm/src/objlist.rs b/vm/src/objlist.rs index fbe4e1596..a335f0281 100644 --- a/vm/src/objlist.rs +++ b/vm/src/objlist.rs @@ -1,34 +1,6 @@ use super::pyobject::{PyObject, PyObjectKind, PyObjectRef, PyResult}; use super::vm::VirtualMachine; -pub fn get_item(vm: &mut VirtualMachine, l: &Vec, b: PyObjectRef) -> PyResult { - match &(b.borrow()).kind { - PyObjectKind::Integer { value } => { - let pos_index = super::objsequence::get_pos(l, *value); - if pos_index < l.len() { - let obj = l[pos_index].clone(); - Ok(obj) - } else { - Err(vm.new_exception("Index out of bounds!".to_string())) - } - } - PyObjectKind::Slice { - start: _, - stop: _, - step: _, - } => Ok(PyObject::new( - PyObjectKind::List { - elements: super::objsequence::get_slice_items(l, &b), - }, - vm.get_type(), - )), - _ => Err(vm.new_exception(format!( - "TypeError: indexing type {:?} with index {:?} is not supported (yet?)", - l, b - ))), - } -} - // set_item: pub fn set_item( vm: &mut VirtualMachine, diff --git a/vm/src/objsequence.rs b/vm/src/objsequence.rs index c9eaf04e7..ca24848db 100644 --- a/vm/src/objsequence.rs +++ b/vm/src/objsequence.rs @@ -1,4 +1,5 @@ use super::pyobject::{PyObject, PyObjectKind, PyObjectRef, PyResult}; +use super::vm::VirtualMachine; pub fn get_pos(l: &Vec, p: i32) -> usize { if p < 0 { @@ -8,7 +9,7 @@ pub fn get_pos(l: &Vec, p: i32) -> usize { } } -pub fn get_slice_items(l: &Vec, slice: &PyObjectRef) -> Vec { +fn get_slice_items(l: &Vec, slice: &PyObjectRef) -> Vec { // TODO: we could potentially avoid this copy and use slice match &(slice.borrow()).kind { PyObjectKind::Slice { start, stop, step } => { @@ -37,3 +38,42 @@ pub fn get_slice_items(l: &Vec, slice: &PyObjectRef) -> Vec panic!("get_slice_items called with non-slice: {:?}", kind), } } + +pub fn get_item( + vm: &mut VirtualMachine, + a: &PyObjectRef, + l: &Vec, + b: PyObjectRef, +) -> PyResult { + match &(b.borrow()).kind { + PyObjectKind::Integer { value } => { + let pos_index = get_pos(l, *value); + if pos_index < l.len() { + let obj = l[pos_index].clone(); + Ok(obj) + } else { + Err(vm.new_exception("Index out of bounds!".to_string())) + } + } + PyObjectKind::Slice { + start: _, + stop: _, + step: _, + } => Ok(PyObject::new( + match &(a.borrow()).kind { + PyObjectKind::Tuple { elements: _ } => PyObjectKind::Tuple { + elements: get_slice_items(l, &b), + }, + PyObjectKind::List { elements: _ } => PyObjectKind::List { + elements: get_slice_items(l, &b), + }, + ref kind => panic!("sequence get_item called for non-sequence: {:?}", kind), + }, + vm.get_type(), + )), + _ => Err(vm.new_exception(format!( + "TypeError: indexing type {:?} with index {:?} is not supported (yet?)", + l, b + ))), + } +} diff --git a/vm/src/objtuple.rs b/vm/src/objtuple.rs deleted file mode 100644 index 36e4ac4ab..000000000 --- a/vm/src/objtuple.rs +++ /dev/null @@ -1,30 +0,0 @@ -use super::pyobject::{PyObject, PyObjectKind, PyObjectRef, PyResult}; -use super::vm::VirtualMachine; - -pub fn get_item(vm: &mut VirtualMachine, l: &Vec, b: PyObjectRef) -> PyResult { - match &(b.borrow()).kind { - PyObjectKind::Integer { value } => { - let pos_index = super::objsequence::get_pos(l, *value); - if pos_index < l.len() { - let obj = l[pos_index].clone(); - Ok(obj) - } else { - Err(vm.new_exception("Index out of bounds!".to_string())) - } - } - PyObjectKind::Slice { - start: _, - stop: _, - step: _, - } => Ok(PyObject::new( - PyObjectKind::Tuple { - elements: super::objsequence::get_slice_items(l, &b), - }, - vm.get_type(), - )), - _ => Err(vm.new_exception(format!( - "TypeError: indexing type {:?} with index {:?} is not supported (yet?)", - l, b - ))), - } -} diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 98dd1dffa..f55a5f08b 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -14,7 +14,6 @@ use super::frame::{Block, Frame, copy_code}; use super::import::import; use super::objlist; use super::objstr; -use super::objtuple; use super::pyobject::{ DictProtocol, IdProtocol, ParentProtocol, PyContext, PyFuncArgs, PyObject, PyObjectKind, PyObjectRef, PyResult, @@ -238,8 +237,9 @@ impl VirtualMachine { let a2 = &*a.borrow(); match &a2.kind { PyObjectKind::String { ref value } => objstr::subscript(self, value, b), - PyObjectKind::List { ref elements } => objlist::get_item(self, elements, b), - PyObjectKind::Tuple { ref elements } => objtuple::get_item(self, elements, b), + PyObjectKind::List { ref elements } | PyObjectKind::Tuple { ref elements } => { + super::objsequence::get_item(self, &a, elements, b) + } _ => Err(self.new_exception(format!( "TypeError: indexing type {:?} with index {:?} is not supported (yet?)", a, b From 4e0c285741aac64630ed67befdbceeeadd241965 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 7 Aug 2018 15:45:58 -0400 Subject: [PATCH 2/3] Use more descriptive variable names in objsequence::get_item --- vm/src/objsequence.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/vm/src/objsequence.rs b/vm/src/objsequence.rs index ca24848db..a0c168376 100644 --- a/vm/src/objsequence.rs +++ b/vm/src/objsequence.rs @@ -41,15 +41,15 @@ fn get_slice_items(l: &Vec, slice: &PyObjectRef) -> Vec, - b: PyObjectRef, + sequence: &PyObjectRef, + elements: &Vec, + subscript: PyObjectRef, ) -> PyResult { - match &(b.borrow()).kind { + match &(subscript.borrow()).kind { PyObjectKind::Integer { value } => { - let pos_index = get_pos(l, *value); - if pos_index < l.len() { - let obj = l[pos_index].clone(); + let pos_index = get_pos(elements, *value); + if pos_index < elements.len() { + let obj = elements[pos_index].clone(); Ok(obj) } else { Err(vm.new_exception("Index out of bounds!".to_string())) @@ -60,12 +60,12 @@ pub fn get_item( stop: _, step: _, } => Ok(PyObject::new( - match &(a.borrow()).kind { + match &(sequence.borrow()).kind { PyObjectKind::Tuple { elements: _ } => PyObjectKind::Tuple { - elements: get_slice_items(l, &b), + elements: get_slice_items(elements, &subscript), }, PyObjectKind::List { elements: _ } => PyObjectKind::List { - elements: get_slice_items(l, &b), + elements: get_slice_items(elements, &subscript), }, ref kind => panic!("sequence get_item called for non-sequence: {:?}", kind), }, @@ -73,7 +73,7 @@ pub fn get_item( )), _ => Err(vm.new_exception(format!( "TypeError: indexing type {:?} with index {:?} is not supported (yet?)", - l, b + sequence, subscript ))), } } From 086dc127f27ab246177f89ddccff7e25a49719e1 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 7 Aug 2018 16:48:58 -0400 Subject: [PATCH 3/3] Handle slice ends greater than sequence length This is already handled for the string case, so borrow that logic (and add an explanatory comment where it was borrowed from). --- tests/snippets/3.1.3.2.py | 2 ++ vm/src/objsequence.rs | 4 ++++ vm/src/objstr.rs | 2 ++ 3 files changed, 8 insertions(+) diff --git a/tests/snippets/3.1.3.2.py b/tests/snippets/3.1.3.2.py index 393e589ea..ea724528f 100644 --- a/tests/snippets/3.1.3.2.py +++ b/tests/snippets/3.1.3.2.py @@ -8,6 +8,7 @@ assert squares == squares[:] assert [1, 9, 25] == squares[::2] assert [4, 16] == squares[1::2] assert [4] == squares[1:2:2] +assert [25] == squares[4:100] squares_tuple = (1, 4, 9, 16, 25) @@ -19,3 +20,4 @@ assert squares_tuple == squares_tuple[:] assert (1, 9, 25) == squares_tuple[::2] assert (4, 16) == squares_tuple[1::2] assert (4,) == squares_tuple[1:2:2] +assert (25,) == squares_tuple[4:100] diff --git a/vm/src/objsequence.rs b/vm/src/objsequence.rs index a0c168376..e29a5c492 100644 --- a/vm/src/objsequence.rs +++ b/vm/src/objsequence.rs @@ -4,6 +4,10 @@ use super::vm::VirtualMachine; pub fn get_pos(l: &Vec, p: i32) -> usize { if p < 0 { l.len() - ((-p) as usize) + } else if p as usize > l.len() { + // This is for the slicing case where the end element is greater than the length of the + // sequence + l.len() } else { p as usize } diff --git a/vm/src/objstr.rs b/vm/src/objstr.rs index bf220b1ed..ef8431435 100644 --- a/vm/src/objstr.rs +++ b/vm/src/objstr.rs @@ -5,6 +5,8 @@ fn str_pos(s: &String, p: i32) -> usize { if p < 0 { s.len() - ((-p) as usize) } else if p as usize > s.len() { + // This is for the slicing case where the end element is greater than the length of the + // string s.len() } else { p as usize