Merge pull request #2247 from youknowone/vm-len

vm._len and objsequence.rs -> sliceable.rs

Lib/test/test_bytes.py

@@ -934,8 +934,6 @@ class BaseBytesTest:
 class BytesTest(BaseBytesTest, unittest.TestCase):
     type2test = bytes
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_getitem_error(self):
         b = b'python'
         msg = "byte indices must be integers or slices"
@@ -1133,8 +1131,6 @@ class BytesTest(BaseBytesTest, unittest.TestCase):
 class ByteArrayTest(BaseBytesTest, unittest.TestCase):
     type2test = bytearray
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_getitem_error(self):
         b = bytearray(b'python')
         msg = "bytearray indices must be integers or slices"

vm/src/builtins.rs

@@ -28,7 +28,6 @@ mod decl {
     use crate::obj::objint::{self, PyIntRef};
     use crate::obj::objiter;
     use crate::obj::objlist::{PyList, SortOptions};
-    use crate::obj::objsequence;
     use crate::obj::objstr::{PyStr, PyStrRef};
     use crate::obj::objtype::{self, PyTypeRef};
     use crate::pyobject::{
@@ -37,6 +36,7 @@ mod decl {
     };
     use crate::readline::{Readline, ReadlineResult};
     use crate::scope::Scope;
+    use crate::sliceable;
     use crate::slots::PyComparisonOp;
     #[cfg(feature = "rustpython-parser")]
     use crate::stdlib::ast;
@@ -444,7 +444,12 @@ mod decl {
 
     #[pyfunction]
     fn len(obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<usize> {
-        objsequence::len(&obj, vm)
+        vm._len(&obj).unwrap_or_else(|| {
+            Err(vm.new_type_error(format!(
+                "object of type '{}' has no len()",
+                obj.lease_class().name
+            )))
+        })
     }
 
     #[pyfunction]

vm/src/bytesinner.rs

@@ -11,9 +11,6 @@ use crate::obj::objbytes::PyBytes;
 use crate::obj::objint::{self, PyInt, PyIntRef};
 use crate::obj::objlist::PyList;
 use crate::obj::objmemory::PyMemoryView;
-use crate::obj::objsequence::{
-    get_saturated_pos, PySliceableSequence, PySliceableSequenceMut, SequenceIndex,
-};
 use crate::obj::objsingletons::PyNoneRef;
 use crate::obj::objslice::PySliceRef;
 use crate::obj::objstr::{self, PyStr, PyStrRef};
@@ -21,6 +18,7 @@ use crate::pyobject::{
     BorrowValue, Either, PyComparisonValue, PyIterable, PyIterator, PyObjectRef, PyResult,
     TryFromObject, TypeProtocol,
 };
+use crate::sliceable::{PySliceableSequence, PySliceableSequenceMut, SequenceIndex};
 use crate::slots::PyComparisonOp;
 use crate::vm::VirtualMachine;
 use rustpython_common::hash;
@@ -297,19 +295,17 @@ impl PyBytesInner {
         })
     }
 
-    pub fn getitem(&self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult {
-        match needle {
-            SequenceIndex::Int(int) => {
-                if let Some(idx) = self.elements.get_pos(int) {
-                    Ok(vm.ctx.new_int(self.elements[idx]))
-                } else {
-                    Err(vm.new_index_error("index out of range".to_owned()))
-                }
-            }
-            SequenceIndex::Slice(slice) => {
-                Ok(vm.ctx.new_bytes(self.elements.get_slice_items(vm, &slice)?))
-            }
-        }
+    pub fn getitem(
+        &self,
+        name: &'static str,
+        needle: PyObjectRef,
+        vm: &VirtualMachine,
+    ) -> PyResult {
+        let obj = match self.elements.get_item(vm, needle, name)? {
+            Either::A(byte) => vm.new_pyobj(byte),
+            Either::B(bytes) => vm.ctx.new_bytes(bytes),
+        };
+        Ok(obj)
     }
 
     pub fn setindex(
@@ -318,7 +314,7 @@ impl PyBytesInner {
         object: PyObjectRef,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
-        if let Some(idx) = self.elements.get_pos(int) {
+        if let Some(idx) = self.elements.wrap_index(int) {
             let value = Self::value_try_from_object(vm, object)?;
             self.elements[idx] = value;
             Ok(())
@@ -374,7 +370,7 @@ impl PyBytesInner {
     pub fn delitem(&mut self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult<()> {
         match needle {
             SequenceIndex::Int(int) => {
-                if let Some(idx) = self.elements.get_pos(int) {
+                if let Some(idx) = self.elements.wrap_index(int) {
                     self.elements.remove(idx);
                     Ok(())
                 } else {
@@ -390,7 +386,7 @@ impl PyBytesInner {
     }
 
     pub fn pop(&mut self, index: isize, vm: &VirtualMachine) -> PyResult<u8> {
-        if let Some(index) = self.elements.get_pos(index) {
+        if let Some(index) = self.elements.wrap_index(index) {
             Ok(self.elements.remove(index))
         } else {
             Err(vm.new_index_error("index out of range".to_owned()))
@@ -404,7 +400,7 @@ impl PyBytesInner {
         vm: &VirtualMachine,
     ) -> PyResult<()> {
         let value = Self::value_try_from_object(vm, object)?;
-        let index = get_saturated_pos(index, self.len());
+        let index = self.elements.saturate_index(index);
         self.elements.insert(index, value);
         Ok(())
     }

vm/src/coroutine.rs

@@ -1,6 +1,6 @@
-use super::objtype::{self, PyTypeRef};
 use crate::exceptions::{self, PyBaseExceptionRef};
 use crate::frame::{ExecutionResult, FrameRef};
+use crate::obj::objtype::{self, PyTypeRef};
 use crate::pyobject::{PyObjectRef, PyResult};
 use crate::vm::VirtualMachine;
 

vm/src/frame.rs

@@ -7,11 +7,11 @@ use itertools::Itertools;
 use crate::builtins::builtin_isinstance;
 use crate::bytecode;
 use crate::common::lock::PyMutex;
+use crate::coroutine::Coro;
 use crate::exceptions::{self, ExceptionCtor, PyBaseExceptionRef};
 use crate::function::PyFuncArgs;
 use crate::obj::objasyncgenerator::PyAsyncGenWrappedValue;
 use crate::obj::objcode::PyCodeRef;
-use crate::obj::objcoroinner::Coro;
 use crate::obj::objcoroutine::PyCoroutine;
 use crate::obj::objdict::{PyDict, PyDictRef};
 use crate::obj::objgenerator::PyGenerator;

vm/src/lib.rs

@@ -48,6 +48,7 @@ mod builtins;
 mod bytesinner;
 pub mod byteslike;
 pub mod cformat;
+mod coroutine;
 mod dictdatatype;
 #[cfg(feature = "rustpython-compiler")]
 pub mod eval;
@@ -65,6 +66,7 @@ mod pyobjectrc;
 pub mod readline;
 pub mod scope;
 mod sequence;
+mod sliceable;
 pub mod slots;
 pub mod stdlib;
 mod sysmodule;

vm/src/obj/mod.rs

@@ -8,7 +8,6 @@ pub mod objbytes;
 pub mod objclassmethod;
 pub mod objcode;
 pub mod objcomplex;
-pub mod objcoroinner;
 pub mod objcoroutine;
 pub mod objdict;
 pub mod objenumerate;
@@ -29,7 +28,6 @@ pub mod objnamespace;
 pub mod objobject;
 pub mod objproperty;
 pub mod objrange;
-pub mod objsequence;
 pub mod objset;
 pub mod objsingletons;
 pub mod objslice;

vm/src/obj/objasyncgenerator.rs

@@ -1,6 +1,6 @@
 use super::objcode::PyCodeRef;
-use super::objcoroinner::{Coro, Variant};
 use super::objtype::{self, PyTypeRef};
+use crate::coroutine::{Coro, Variant};
 use crate::exceptions::PyBaseExceptionRef;
 use crate::frame::FrameRef;
 use crate::function::OptionalArg;

vm/src/obj/objbytearray.rs

@@ -5,7 +5,6 @@ use std::mem::size_of;
 
 use super::objint::PyIntRef;
 use super::objiter;
-use super::objsequence::SequenceIndex;
 use super::objstr::PyStrRef;
 use super::objtype::PyTypeRef;
 use crate::anystr::{self, AnyStr};
@@ -22,6 +21,7 @@ use crate::pyobject::{
     BorrowValue, Either, IdProtocol, IntoPyObject, PyClassImpl, PyComparisonValue, PyContext,
     PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject,
 };
+use crate::sliceable::SequenceIndex;
 use crate::slots::{Comparable, Hashable, PyComparisonOp, Unhashable};
 use crate::vm::VirtualMachine;
 
@@ -152,8 +152,8 @@ impl PyByteArray {
     }
 
     #[pymethod(name = "__getitem__")]
-    fn getitem(&self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult {
-        self.borrow_value().getitem(needle, vm)
+    fn getitem(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult {
+        self.borrow_value().getitem("bytearray", needle, vm)
     }
 
     #[pymethod(name = "__setitem__")]

vm/src/obj/objbytes.rs

@@ -5,7 +5,6 @@ use std::ops::Deref;
 
 use super::objint::PyIntRef;
 use super::objiter;
-use super::objsequence::SequenceIndex;
 use super::objstr::PyStrRef;
 use super::objtype::PyTypeRef;
 use crate::anystr::{self, AnyStr};
@@ -139,8 +138,8 @@ impl PyBytes {
     }
 
     #[pymethod(name = "__getitem__")]
-    fn getitem(&self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult {
-        self.inner.getitem(needle, vm)
+    fn getitem(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult {
+        self.inner.getitem("byte", needle, vm)
     }
 
     #[pymethod(name = "isalnum")]

vm/src/obj/objcoroutine.rs

@@ -1,7 +1,7 @@
 use super::objcode::PyCodeRef;
-use super::objcoroinner::{Coro, Variant};
 use super::objstr::PyStrRef;
 use super::objtype::PyTypeRef;
+use crate::coroutine::{Coro, Variant};
 use crate::frame::FrameRef;
 use crate::function::OptionalArg;
 use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue};

vm/src/obj/objgenerator.rs

@@ -3,8 +3,8 @@
  */
 
 use super::objcode::PyCodeRef;
-use super::objcoroinner::{Coro, Variant};
 use super::objtype::PyTypeRef;
+use crate::coroutine::{Coro, Variant};
 use crate::frame::FrameRef;
 use crate::function::OptionalArg;
 use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue};

vm/src/obj/objiter.rs

@@ -6,7 +6,6 @@ use crossbeam_utils::atomic::AtomicCell;
 use num_traits::Signed;
 
 use super::objint::{self, PyInt};
-use super::objsequence;
 use super::objtype::{self, PyTypeRef};
 use crate::exceptions::PyBaseExceptionRef;
 use crate::pyobject::{
@@ -103,7 +102,7 @@ pub fn stop_iter_value(vm: &VirtualMachine, exc: &PyBaseExceptionRef) -> PyResul
 }
 
 pub fn length_hint(vm: &VirtualMachine, iter: PyObjectRef) -> PyResult<Option<usize>> {
-    if let Some(len) = objsequence::opt_len(&iter, vm) {
+    if let Some(len) = vm._len(&iter) {
         match len {
             Ok(len) => return Ok(Some(len)),
             Err(e) => {
@@ -203,7 +202,7 @@ impl PySequenceIterator {
         let hint = if self.reversed {
             pos + 1
         } else {
-            let len = objsequence::opt_len(&self.obj, vm).unwrap_or_else(|| {
+            let len = vm._len(&self.obj).unwrap_or_else(|| {
                 Err(vm.new_type_error("sequence has no __len__ method".to_owned()))
             })?;
             len as isize - pos

vm/src/obj/objlist.rs

@@ -8,9 +8,6 @@ use num_traits::ToPrimitive;
 
 use super::objint::PyIntRef;
 use super::objiter;
-use super::objsequence::{
-    get_item, get_pos, get_saturated_pos, PySliceableSequenceMut, SequenceIndex,
-};
 use super::objslice::PySliceRef;
 use super::objtype::PyTypeRef;
 use crate::bytesinner;
@@ -21,6 +18,7 @@ use crate::pyobject::{
     PyResult, PyValue, TryFromObject, TypeProtocol,
 };
 use crate::sequence::{self, SimpleSeq};
+use crate::sliceable::{PySliceableSequence, PySliceableSequenceMut, SequenceIndex};
 use crate::slots::{Comparable, Hashable, PyComparisonOp, Unhashable};
 use crate::vm::{ReprGuard, VirtualMachine};
 
@@ -120,7 +118,7 @@ impl PyList {
     #[pymethod]
     pub(crate) fn insert(&self, position: isize, element: PyObjectRef) {
         let mut elements = self.borrow_value_mut();
-        let position = get_saturated_pos(position, elements.len());
+        let position = elements.saturate_index(position);
         elements.insert(position, element);
     }
 
@@ -192,12 +190,11 @@ impl PyList {
 
     #[pymethod(name = "__getitem__")]
     fn getitem(zelf: PyRef<Self>, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        Ok(
-            match get_item(vm, zelf.as_object(), &zelf.borrow_value(), needle)? {
-                Either::A(obj) => obj,
-                Either::B(vec) => vm.ctx.new_list(vec),
-            },
-        )
+        let result = match zelf.borrow_value().get_item(vm, needle, "list")? {
+            Either::A(obj) => obj,
+            Either::B(vec) => vm.ctx.new_list(vec),
+        };
+        Ok(result)
     }
 
     #[pymethod(name = "__iter__")]
@@ -228,7 +225,7 @@ impl PyList {
 
     fn setindex(&self, index: isize, mut value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
         let mut elements = self.borrow_value_mut();
-        if let Some(pos_index) = get_pos(index, elements.len()) {
+        if let Some(pos_index) = elements.wrap_index(index) {
             std::mem::swap(&mut elements[pos_index], &mut value);
             Ok(())
         } else {
@@ -361,7 +358,7 @@ impl PyList {
     fn delindex(&self, index: isize, vm: &VirtualMachine) -> PyResult<()> {
         let removed = {
             let mut elements = self.borrow_value_mut();
-            if let Some(pos_index) = get_pos(index, elements.len()) {
+            if let Some(pos_index) = elements.wrap_index(index) {
                 // defer delete out of borrow
                 Ok(elements.remove(pos_index))
             } else {

vm/src/obj/objstr.rs

@@ -16,16 +16,17 @@ use super::objbytes::{PyBytes, PyBytesRef};
 use super::objdict::PyDict;
 use super::objint::{PyInt, PyIntRef};
 use super::objiter;
-use super::objsequence::{PySliceableSequence, SequenceIndex};
 use super::objtype::{self, PyTypeRef};
 use crate::anystr::{self, adjust_indices, AnyStr, AnyStrContainer, AnyStrWrapper};
 use crate::exceptions::IntoPyException;
 use crate::format::{FormatSpec, FormatString, FromTemplate};
 use crate::function::{OptionalArg, OptionalOption, PyFuncArgs};
 use crate::pyobject::{
-    BorrowValue, IdProtocol, IntoPyObject, ItemProtocol, PyClassImpl, PyComparisonValue, PyContext,
-    PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TryIntoRef, TypeProtocol,
+    BorrowValue, Either, IdProtocol, IntoPyObject, ItemProtocol, PyClassImpl, PyComparisonValue,
+    PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TryIntoRef,
+    TypeProtocol,
 };
+use crate::sliceable::PySliceableSequence;
 use crate::slots::{Comparable, Hashable, PyComparisonOp};
 use crate::VirtualMachine;
 use rustpython_common::hash;
@@ -258,26 +259,12 @@ impl PyStr {
     }
 
     #[pymethod(name = "__getitem__")]
-    fn getitem(&self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult {
-        match needle {
-            SequenceIndex::Int(pos) => {
-                let index: usize = if pos.is_negative() {
-                    (self.value.chars().count() as isize + pos) as usize
-                } else {
-                    pos.abs() as usize
-                };
-
-                if let Some(character) = self.value.chars().nth(index) {
-                    Ok(vm.ctx.new_str(character.to_string()))
-                } else {
-                    Err(vm.new_index_error("string index out of range".to_owned()))
-                }
-            }
-            SequenceIndex::Slice(slice) => {
-                let string = self.get_slice_items(vm, &slice)?;
-                Ok(vm.ctx.new_str(string))
-            }
-        }
+    fn getitem(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult {
+        let s = match self.get_item(vm, needle, "string")? {
+            Either::A(ch) => ch.to_string(),
+            Either::B(s) => s,
+        };
+        Ok(vm.ctx.new_str(s))
     }
 
     pub(crate) fn hash(&self, vm: &VirtualMachine) -> hash::PyHash {
@@ -1202,8 +1189,13 @@ pub fn borrow_value(obj: &PyObjectRef) -> &str {
 }
 
 impl PySliceableSequence for PyStr {
+    type Item = char;
     type Sliced = String;
 
+    fn do_get(&self, index: usize) -> Self::Item {
+        self.value.chars().nth(index).unwrap()
+    }
+
     fn do_slice(&self, range: Range<usize>) -> Self::Sliced {
         self.value
             .chars()

vm/src/obj/objtuple.rs

@@ -2,7 +2,6 @@ use crossbeam_utils::atomic::AtomicCell;
 use std::fmt;
 
 use super::objiter;
-use super::objsequence::get_item;
 use super::objtype::PyTypeRef;
 use crate::function::OptionalArg;
 use crate::pyobject::{
@@ -10,6 +9,7 @@ use crate::pyobject::{
     PyComparisonValue, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol,
 };
 use crate::sequence::{self, SimpleSeq};
+use crate::sliceable::PySliceableSequence;
 use crate::slots::{Comparable, Hashable, PyComparisonOp};
 use crate::vm::{ReprGuard, VirtualMachine};
 use rustpython_common::hash::PyHash;
@@ -184,12 +184,11 @@ impl PyTuple {
 
     #[pymethod(name = "__getitem__")]
     fn getitem(zelf: PyRef<Self>, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        Ok(
-            match get_item(vm, zelf.as_object(), &zelf.elements, needle)? {
-                Either::A(obj) => obj,
-                Either::B(vec) => vm.ctx.new_tuple(vec),
-            },
-        )
+        let result = match zelf.elements.as_ref().get_item(vm, needle, "tuple")? {
+            Either::A(obj) => obj,
+            Either::B(vec) => vm.ctx.new_tuple(vec),
+        };
+        Ok(result)
     }
 
     #[pymethod(name = "index")]

vm/src/sliceable.rs

@@ -1,12 +1,9 @@
-use std::ops::Range;
-
 use num_bigint::BigInt;
 use num_traits::{One, Signed, ToPrimitive, Zero};
+use std::ops::Range;
 
-use super::objint::{PyInt, PyIntRef};
-use super::objsingletons::PyNone;
-use super::objslice::{PySlice, PySliceRef};
-use crate::function::OptionalArg;
+use crate::obj::objint::PyInt;
+use crate::obj::objslice::{PySlice, PySliceRef};
 use crate::pyobject::{BorrowValue, Either, PyObjectRef, PyResult, TryFromObject, TypeProtocol};
 use crate::vm::VirtualMachine;
 
@@ -37,7 +34,7 @@ pub trait PySliceableSequenceMut {
             return Err(vm.new_value_error("slice step cannot be zero".to_owned()));
         }
         if step == BigInt::one() {
-            let range = self.as_slice().get_slice_range(&start, &stop);
+            let range = self.as_slice().saturate_range(&start, &stop);
             let range = if range.end < range.start {
                 range.start..range.start
             } else {
@@ -70,7 +67,7 @@ pub trait PySliceableSequenceMut {
             (start, stop, step, false)
         };
 
-        let range = self.as_slice().get_slice_range(&start, &stop);
+        let range = self.as_slice().saturate_range(&start, &stop);
         let range = if range.end < range.start {
             range.start..range.start
         } else {
@@ -140,7 +137,7 @@ pub trait PySliceableSequenceMut {
         }
 
         if step == BigInt::one() {
-            let range = self.as_slice().get_slice_range(&start, &stop);
+            let range = self.as_slice().saturate_range(&start, &stop);
             if range.start < range.end {
                 self.do_delete_range(range);
             }
@@ -170,7 +167,7 @@ pub trait PySliceableSequenceMut {
             (start, stop, step, false)
         };
 
-        let range = self.as_slice().get_slice_range(&start, &stop);
+        let range = self.as_slice().saturate_range(&start, &stop);
         if range.start >= range.end {
             return Ok(());
         }
@@ -243,8 +240,10 @@ impl<T: Clone> PySliceableSequenceMut for Vec<T> {
 }
 
 pub trait PySliceableSequence {
+    type Item;
     type Sliced;
 
+    fn do_get(&self, index: usize) -> Self::Item;
     fn do_slice(&self, range: Range<usize>) -> Self::Sliced;
     fn do_slice_reverse(&self, range: Range<usize>) -> Self::Sliced;
     fn do_stepped_slice(&self, range: Range<usize>, step: usize) -> Self::Sliced;
@@ -254,19 +253,35 @@ pub trait PySliceableSequence {
     fn len(&self) -> usize;
     fn is_empty(&self) -> bool;
 
-    fn get_pos(&self, p: isize) -> Option<usize> {
-        get_pos(p, self.len())
+    fn wrap_index(&self, p: isize) -> Option<usize> {
+        wrap_index(p, self.len())
     }
 
-    fn get_slice_pos(&self, slice_pos: &BigInt) -> usize {
-        get_slice_pos(slice_pos, self.len())
+    fn saturate_index(&self, p: isize) -> usize {
+        saturate_index(p, self.len())
     }
 
-    fn get_slice_range(&self, start: &Option<BigInt>, stop: &Option<BigInt>) -> Range<usize> {
-        let start = start.as_ref().map(|x| self.get_slice_pos(x)).unwrap_or(0);
+    fn saturate_big_index(&self, slice_pos: &BigInt) -> usize {
+        let len = self.len();
+        if let Some(pos) = slice_pos.to_isize() {
+            self.saturate_index(pos)
+        } else if slice_pos.is_negative() {
+            // slice past start bound, round to start
+            0
+        } else {
+            // slice past end bound, round to end
+            len
+        }
+    }
+
+    fn saturate_range(&self, start: &Option<BigInt>, stop: &Option<BigInt>) -> Range<usize> {
+        let start = start
+            .as_ref()
+            .map(|x| self.saturate_big_index(x))
+            .unwrap_or(0);
         let stop = stop
             .as_ref()
-            .map(|x| self.get_slice_pos(x))
+            .map(|x| self.saturate_big_index(x))
             .unwrap_or_else(|| self.len());
 
         start..stop
@@ -279,7 +294,7 @@ pub trait PySliceableSequence {
         if step.is_zero() {
             Err(vm.new_value_error("slice step cannot be zero".to_owned()))
         } else if step.is_positive() {
-            let range = self.get_slice_range(&start, &stop);
+            let range = self.saturate_range(&start, &stop);
             if range.start < range.end {
                 #[allow(clippy::range_plus_one)]
                 match step.to_i32() {
@@ -307,7 +322,7 @@ pub trait PySliceableSequence {
                     x + 1
                 }
             });
-            let range = self.get_slice_range(&stop, &start);
+            let range = self.saturate_range(&stop, &start);
             if range.start < range.end {
                 match (-step).to_i32() {
                     Some(1) => Ok(self.do_slice_reverse(range)),
@@ -319,25 +334,53 @@ pub trait PySliceableSequence {
             }
         }
     }
+
+    fn get_item(
+        &self,
+        vm: &VirtualMachine,
+        needle: PyObjectRef,
+        owner_type: &'static str,
+    ) -> PyResult<Either<Self::Item, Self::Sliced>> {
+        let needle = SequenceIndex::try_from_object_for(vm, needle, owner_type)?;
+        match needle {
+            SequenceIndex::Int(value) => {
+                let pos_index = self.wrap_index(value).ok_or_else(|| {
+                    vm.new_index_error(format!("{} index out of range", owner_type))
+                })?;
+                Ok(Either::A(self.do_get(pos_index)))
+            }
+            SequenceIndex::Slice(slice) => Ok(Either::B(self.get_slice_items(vm, &slice)?)),
+        }
+    }
 }
 
 impl<T: Clone> PySliceableSequence for [T] {
+    type Item = T;
     type Sliced = Vec<T>;
 
+    #[inline]
+    fn do_get(&self, index: usize) -> Self::Item {
+        self[index].clone()
+    }
+
+    #[inline]
     fn do_slice(&self, range: Range<usize>) -> Self::Sliced {
         self[range].to_vec()
     }
 
+    #[inline]
     fn do_slice_reverse(&self, range: Range<usize>) -> Self::Sliced {
         let mut slice = self[range].to_vec();
         slice.reverse();
         slice
     }
 
+    #[inline]
     fn do_stepped_slice(&self, range: Range<usize>, step: usize) -> Self::Sliced {
         self[range].iter().step_by(step).cloned().collect()
     }
 
+    #[inline]
     fn do_stepped_slice_reverse(&self, range: Range<usize>, step: usize) -> Self::Sliced {
         self[range].iter().rev().step_by(step).cloned().collect()
     }
@@ -363,8 +406,12 @@ pub enum SequenceIndex {
     Slice(PySliceRef),
 }
 
-impl TryFromObject for SequenceIndex {
-    fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<Self> {
+impl SequenceIndex {
+    fn try_from_object_for(
+        vm: &VirtualMachine,
+        obj: PyObjectRef,
+        owner_type: &'static str,
+    ) -> PyResult<Self> {
         match_class!(match obj {
             i @ PyInt => i
                 .borrow_value()
@@ -374,39 +421,47 @@ impl TryFromObject for SequenceIndex {
                     .new_index_error("cannot fit 'int' into an index-sized integer".to_owned())),
             s @ PySlice => Ok(SequenceIndex::Slice(s)),
             obj => Err(vm.new_type_error(format!(
-                "sequence indices be integers or slices, not {}",
-                obj.class(),
+                "{} indices must be integers or slices, not {}",
+                owner_type,
+                obj.lease_class().name,
             ))),
         })
     }
 }
 
-/// Get the index into a sequence like type. Get it from a python integer
-/// object, accounting for negative index, and out of bounds issues.
-pub fn get_sequence_index(vm: &VirtualMachine, index: &PyIntRef, length: usize) -> PyResult<usize> {
-    if let Some(value) = index.borrow_value().to_i64() {
-        if value < 0 {
-            let from_end: usize = -value as usize;
-            if from_end > length {
-                Err(vm.new_index_error("Index out of bounds!".to_owned()))
-            } else {
-                let index = length - from_end;
-                Ok(index)
-            }
-        } else {
-            let index = value as usize;
-            if index >= length {
-                Err(vm.new_index_error("Index out of bounds!".to_owned()))
-            } else {
-                Ok(index)
-            }
-        }
-    } else {
-        Err(vm.new_index_error("cannot fit 'int' into an index-sized integer".to_owned()))
+impl TryFromObject for SequenceIndex {
+    fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<Self> {
+        Self::try_from_object_for(vm, obj, "sequence")
     }
 }
 
-pub fn get_pos(p: isize, len: usize) -> Option<usize> {
+/// Get the index into a sequence like type. Get it from a python integer
+/// object, accounting for negative index, and out of bounds issues.
+// pub fn get_sequence_index(vm: &VirtualMachine, index: &PyIntRef, length: usize) -> PyResult<usize> {
+//     if let Some(value) = index.borrow_value().to_i64() {
+//         if value < 0 {
+//             let from_end: usize = -value as usize;
+//             if from_end > length {
+//                 Err(vm.new_index_error("Index out of bounds!".to_owned()))
+//             } else {
+//                 let index = length - from_end;
+//                 Ok(index)
+//             }
+//         } else {
+//             let index = value as usize;
+//             if index >= length {
+//                 Err(vm.new_index_error("Index out of bounds!".to_owned()))
+//             } else {
+//                 Ok(index)
+//             }
+//         }
+//     } else {
+//         Err(vm.new_index_error("cannot fit 'int' into an index-sized integer".to_owned()))
+//     }
+// }
+
+// Use PySliceableSequence::wrap_index for implementors
+pub(crate) fn wrap_index(p: isize, len: usize) -> Option<usize> {
     let neg = p.is_negative();
     let p = p.abs().to_usize()?;
     if neg {
@@ -419,7 +474,7 @@ pub fn get_pos(p: isize, len: usize) -> Option<usize> {
 }
 
 // return pos is in range [0, len] inclusive
-pub fn get_saturated_pos(p: isize, len: usize) -> usize {
+pub(crate) fn saturate_index(p: isize, len: usize) -> usize {
     let mut p = p;
     let len = len.to_isize().unwrap();
     if p < 0 {
@@ -434,99 +489,27 @@ pub fn get_saturated_pos(p: isize, len: usize) -> usize {
     p as usize
 }
 
-pub fn get_slice_pos(slice_pos: &BigInt, len: usize) -> usize {
-    if let Some(pos) = slice_pos.to_isize() {
-        if let Some(index) = get_pos(pos, len) {
-            // within bounds
-            return index;
-        }
-    }
+// pub fn saturate_range(start: &Option<BigInt>, stop: &Option<BigInt>, len: usize) -> Range<usize> {
+//     let start = start.as_ref().map_or(0, |x| saturate_big_index(x, len));
+//     let stop = stop.as_ref().map_or(len, |x| saturate_big_index(x, len));
 
-    if slice_pos.is_negative() {
-        // slice past start bound, round to start
-        0
-    } else {
-        // slice past end bound, round to end
-        len
-    }
-}
+//     start..stop
+// }
 
-pub fn get_slice_range(start: &Option<BigInt>, stop: &Option<BigInt>, len: usize) -> Range<usize> {
-    let start = start.as_ref().map_or(0, |x| get_slice_pos(x, len));
-    let stop = stop.as_ref().map_or(len, |x| get_slice_pos(x, len));
-
-    start..stop
-}
-
-pub fn get_item(
-    vm: &VirtualMachine,
-    sequence: &PyObjectRef,
-    elements: &[PyObjectRef],
-    subscript: PyObjectRef,
-) -> PyResult<Either<PyObjectRef, Vec<PyObjectRef>>> {
-    if let Some(i) = subscript.payload::<PyInt>() {
-        let value = i.borrow_value().to_isize().ok_or_else(|| {
-            vm.new_index_error("cannot fit 'int' into an index-sized integer".to_owned())
-        })?;
-        let pos_index = get_pos(value, elements.len())
-            .ok_or_else(|| vm.new_index_error("Index out of bounds!".to_owned()))?;
-        Ok(Either::A(elements[pos_index].clone()))
-    } else {
-        let slice = subscript.payload::<PySlice>().ok_or_else(|| {
-            vm.new_type_error(format!(
-                "{} indices must be integers or slices",
-                sequence.lease_class().name
-            ))
-        })?;
-        Ok(Either::B(elements.get_slice_items(vm, slice)?))
-    }
-}
-
-//Check if given arg could be used with PySliceableSequence.get_slice_range()
-pub fn is_valid_slice_arg(
-    arg: OptionalArg<PyObjectRef>,
-    vm: &VirtualMachine,
-) -> PyResult<Option<BigInt>> {
-    if let OptionalArg::Present(value) = arg {
-        match_class!(match value {
-            i @ PyInt => Ok(Some(i.borrow_value().clone())),
-            _obj @ PyNone => Ok(None),
-            _ => Err(vm.new_type_error(
-                "slice indices must be integers or None or have an __index__ method".to_owned()
-            )), // TODO: check for an __index__ method
-        })
-    } else {
-        Ok(None)
-    }
-}
-
-pub fn opt_len(obj: &PyObjectRef, vm: &VirtualMachine) -> Option<PyResult<usize>> {
-    vm.get_method(obj.clone(), "__len__").map(|len| {
-        let len = vm.invoke(&len?, vec![])?;
-        let len = len
-            .payload_if_subclass::<PyInt>(vm)
-            .ok_or_else(|| {
-                vm.new_type_error(format!(
-                    "'{}' object cannot be interpreted as an integer",
-                    len.lease_class().name
-                ))
-            })?
-            .borrow_value();
-        if len.is_negative() {
-            return Err(vm.new_value_error("__len__() should return >= 0".to_owned()));
-        }
-        let len = len.to_isize().ok_or_else(|| {
-            vm.new_overflow_error("cannot fit 'int' into an index-sized integer".to_owned())
-        })?;
-        Ok(len as usize)
-    })
-}
-
-pub fn len(obj: &PyObjectRef, vm: &VirtualMachine) -> PyResult<usize> {
-    opt_len(obj, vm).unwrap_or_else(|| {
-        Err(vm.new_type_error(format!(
-            "object of type '{}' has no len()",
-            obj.lease_class().name
-        )))
-    })
-}
+//Check if given arg could be used with PySliceableSequence.saturate_range()
+// pub fn is_valid_slice_arg(
+//     arg: OptionalArg<PyObjectRef>,
+//     vm: &VirtualMachine,
+// ) -> PyResult<Option<BigInt>> {
+//     if let OptionalArg::Present(value) = arg {
+//         match_class!(match value {
+//             i @ PyInt => Ok(Some(i.borrow_value().clone())),
+//             _obj @ PyNone => Ok(None),
+//             _ => Err(vm.new_type_error(
+//                 "slice indices must be integers or None or have an __index__ method".to_owned()
+//             )), // TODO: check for an __index__ method
+//         })
+//     } else {
+//         Ok(None)
+//     }
+// }

vm/src/stdlib/array.rs

@@ -7,7 +7,6 @@ use crate::obj::objbytes::PyBytesRef;
 use crate::obj::objfloat::try_float;
 use crate::obj::objiter;
 use crate::obj::objlist::PyList;
-use crate::obj::objsequence::{get_saturated_pos, PySliceableSequence, PySliceableSequenceMut};
 use crate::obj::objslice::PySliceRef;
 use crate::obj::objstr::PyStrRef;
 use crate::obj::objtype::PyTypeRef;
@@ -15,6 +14,7 @@ use crate::pyobject::{
     BorrowValue, Either, IdProtocol, IntoPyObject, PyClassImpl, PyComparisonValue, PyIterable,
     PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol,
 };
+use crate::sliceable::{saturate_index, PySliceableSequence, PySliceableSequenceMut};
 use crate::slots::{Comparable, PyComparisonOp};
 use crate::VirtualMachine;
 use crossbeam_utils::atomic::AtomicCell;
@@ -581,7 +581,7 @@ impl PyArray {
 
     #[pymethod]
     fn insert(&self, i: isize, x: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        let i = get_saturated_pos(i, self.len());
+        let i = saturate_index(i, self.len());
         self.borrow_value_mut().insert(i, x, vm)
     }
 

vm/src/stdlib/collections.rs

@@ -4,12 +4,12 @@ pub(crate) use _collections::make_module;
 mod _collections {
     use crate::common::lock::{PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard};
     use crate::function::OptionalArg;
-    use crate::obj::{objiter, objsequence, objtype::PyTypeRef};
+    use crate::obj::{objiter, objtype::PyTypeRef};
     use crate::pyobject::{PyComparisonValue, PyIterable, PyObjectRef, PyRef, PyResult, PyValue};
-    use crate::sequence;
     use crate::slots::{Comparable, PyComparisonOp};
     use crate::vm::ReprGuard;
     use crate::VirtualMachine;
+    use crate::{sequence, sliceable};
     use itertools::Itertools;
     use std::collections::VecDeque;
 
@@ -248,7 +248,7 @@ mod _collections {
         #[pymethod(magic)]
         fn getitem(&self, idx: isize, vm: &VirtualMachine) -> PyResult {
             let deque = self.borrow_deque();
-            objsequence::get_pos(idx, deque.len())
+            sliceable::wrap_index(idx, deque.len())
                 .and_then(|i| deque.get(i).cloned())
                 .ok_or_else(|| vm.new_index_error("deque index out of range".to_owned()))
         }
@@ -256,7 +256,7 @@ mod _collections {
         #[pymethod(magic)]
         fn setitem(&self, idx: isize, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
             let mut deque = self.borrow_deque_mut();
-            objsequence::get_pos(idx, deque.len())
+            sliceable::wrap_index(idx, deque.len())
                 .and_then(|i| deque.get_mut(i))
                 .map(|x| *x = value)
                 .ok_or_else(|| vm.new_index_error("deque index out of range".to_owned()))
@@ -265,7 +265,7 @@ mod _collections {
         #[pymethod(magic)]
         fn delitem(&self, idx: isize, vm: &VirtualMachine) -> PyResult<()> {
             let mut deque = self.borrow_deque_mut();
-            objsequence::get_pos(idx, deque.len())
+            sliceable::wrap_index(idx, deque.len())
                 .and_then(|i| deque.remove(i).map(drop))
                 .ok_or_else(|| vm.new_index_error("deque index out of range".to_owned()))
         }

vm/src/vm.rs

@@ -11,11 +11,7 @@ use std::fmt;
 
 use arr_macro::arr;
 use crossbeam_utils::atomic::AtomicCell;
-#[cfg(feature = "rustpython-compiler")]
-use rustpython_compiler::{
-    compile::{self, CompileOpts},
-    error::CompileError,
-};
+use num_traits::{Signed, ToPrimitive};
 
 use crate::builtins::{self, to_ascii};
 use crate::bytecode;
@@ -28,7 +24,7 @@ use crate::import;
 use crate::obj::objbool;
 use crate::obj::objcode::{PyCode, PyCodeRef};
 use crate::obj::objdict::PyDictRef;
-use crate::obj::objint::PyIntRef;
+use crate::obj::objint::{PyInt, PyIntRef};
 use crate::obj::objiter;
 use crate::obj::objlist::PyList;
 use crate::obj::objmodule::{self, PyModule};
@@ -44,6 +40,11 @@ use crate::scope::Scope;
 use crate::slots::PyComparisonOp;
 use crate::stdlib;
 use crate::sysmodule;
+#[cfg(feature = "rustpython-compiler")]
+use rustpython_compiler::{
+    compile::{self, CompileOpts},
+    error::CompileError,
+};
 
 // use objects::objects;
 
@@ -1494,6 +1495,28 @@ impl VirtualMachine {
         hash(&obj, self)
     }
 
+    pub fn _len(&self, obj: &PyObjectRef) -> Option<PyResult<usize>> {
+        self.get_method(obj.clone(), "__len__").map(|len| {
+            let len = self.invoke(&len?, vec![])?;
+            let len = len
+                .payload_if_subclass::<PyInt>(self)
+                .ok_or_else(|| {
+                    self.new_type_error(format!(
+                        "'{}' object cannot be interpreted as an integer",
+                        len.lease_class().name
+                    ))
+                })?
+                .borrow_value();
+            if len.is_negative() {
+                return Err(self.new_value_error("__len__() should return >= 0".to_owned()));
+            }
+            let len = len.to_isize().ok_or_else(|| {
+                self.new_overflow_error("cannot fit 'int' into an index-sized integer".to_owned())
+            })?;
+            Ok(len as usize)
+        })
+    }
+
     // https://docs.python.org/3/reference/expressions.html#membership-test-operations
     fn _membership_iter_search(&self, haystack: PyObjectRef, needle: PyObjectRef) -> PyResult {
         let iter = objiter::get_iter(self, &haystack)?;