put wrap_index to SequenceIndexOp

stdlib/src/mmap.rs

@@ -14,7 +14,7 @@ mod mmap {
         protocol::{
             BufferDescriptor, BufferMethods, PyBuffer, PyMappingMethods, PySequenceMethods,
         },
-        sliceable::{wrap_index, SaturatedSlice, SequenceIndex, SequenceIndexOp},
+        sliceable::{SaturatedSlice, SequenceIndex, SequenceIndexOp},
         types::{AsBuffer, AsMapping, AsSequence, Constructor},
         AsObject, FromArgs, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult,
         TryFromBorrowedObject, VirtualMachine,
@@ -918,7 +918,8 @@ mod mmap {
         }
 
         fn getitem_by_index(&self, i: isize, vm: &VirtualMachine) -> PyResult<PyObjectRef> {
-            let i = wrap_index(i, self.len())
+            let i = i
+                .wrapped_at(self.len())
                 .ok_or_else(|| vm.new_index_error("mmap index out of range".to_owned()))?;
 
             let b = match self.check_valid(vm)?.deref().as_ref().unwrap() {
@@ -999,7 +1000,8 @@ mod mmap {
             value: PyObjectRef,
             vm: &VirtualMachine,
         ) -> PyResult<()> {
-            let i = wrap_index(i, zelf.len())
+            let i = i
+                .wrapped_at(zelf.len())
                 .ok_or_else(|| vm.new_index_error("mmap index out of range".to_owned()))?;
 
             let b = value_from_object(vm, &value)?;

vm/src/builtins/memory.rs

@@ -17,7 +17,7 @@ use crate::{
     protocol::{
         BufferDescriptor, BufferMethods, PyBuffer, PyMappingMethods, PySequenceMethods, VecBuffer,
     },
-    sliceable::wrap_index,
+    sliceable::SequenceIndexOp,
     types::{AsBuffer, AsMapping, AsSequence, Comparable, Constructor, Hashable, PyComparisonOp},
     AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult,
     TryFromBorrowedObject, TryFromObject, VirtualMachine,
@@ -243,7 +243,8 @@ impl PyMemoryView {
             ));
         }
         let (shape, stride, suboffset) = self.desc.dim_desc[0];
-        let index = wrap_index(i, shape)
+        let index = i
+            .wrapped_at(shape)
             .ok_or_else(|| vm.new_index_error("index out of range".to_owned()))?;
         let index = index as isize * stride + suboffset;
         let pos = (index + self.start as isize) as usize;
@@ -292,7 +293,8 @@ impl PyMemoryView {
             return Err(vm.new_not_implemented_error("sub-views are not implemented".to_owned()));
         }
         let (shape, stride, suboffset) = self.desc.dim_desc[0];
-        let index = wrap_index(i, shape)
+        let index = i
+            .wrapped_at(shape)
             .ok_or_else(|| vm.new_index_error("index out of range".to_owned()))?;
         let index = index as isize * stride + suboffset;
         let pos = (index + self.start as isize) as usize;

vm/src/protocol/buffer.rs

@@ -7,7 +7,7 @@ use crate::{
         lock::{MapImmutable, PyMutex, PyMutexGuard},
     },
     object::PyObjectPayload,
-    sliceable::wrap_index,
+    sliceable::SequenceIndexOp,
     types::{Constructor, Unconstructible},
     AsObject, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromBorrowedObject,
     VirtualMachine,
@@ -255,7 +255,7 @@ impl BufferDescriptor {
             .cloned()
             .zip_eq(self.dim_desc.iter().cloned())
         {
-            let i = wrap_index(i, shape).ok_or_else(|| {
+            let i = i.wrapped_at(shape).ok_or_else(|| {
                 vm.new_index_error(format!("index out of bounds on dimension {}", i))
             })?;
             pos += i as isize * stride + suboffset;

vm/src/sliceable.rs

@@ -176,7 +176,7 @@ pub trait SliceableSequenceOp {
     fn len(&self) -> usize;
 
     fn wrap_index(&self, p: isize) -> Option<usize> {
-        wrap_index(p, self.len())
+        p.wrapped_at(self.len())
     }
 
     fn saturate_index(&self, p: isize) -> usize {
@@ -292,28 +292,33 @@ impl SequenceIndex {
     }
 }
 
-// Use PySliceableSequence::wrap_index for implementors
-pub fn wrap_index(p: isize, len: usize) -> Option<usize> {
-    let neg = p.is_negative();
-    let p = p.wrapping_abs() as usize;
-    if neg {
-        len.checked_sub(p)
-    } else if p >= len {
-        None
-    } else {
-        Some(p)
-    }
-}
-
 pub trait SequenceIndexOp {
     // Saturate p in range [0, len] inclusive
     fn saturated_at(&self, len: usize) -> usize;
+    // Use PySliceableSequence::wrap_index for implementors
+    fn wrapped_at(&self, len: usize) -> Option<usize>;
 }
 
 impl SequenceIndexOp for isize {
     fn saturated_at(&self, len: usize) -> usize {
         let len = len.to_isize().unwrap_or(Self::MAX);
-        *self.clamp(&0, &len) as usize
+        let mut p = *self;
+        if p < 0 {
+            p += len;
+        }
+        p.clamp(0, len) as usize
+    }
+
+    fn wrapped_at(&self, len: usize) -> Option<usize> {
+        let neg = self.is_negative();
+        let p = self.unsigned_abs();
+        if neg {
+            len.checked_sub(p)
+        } else if p >= len {
+            None
+        } else {
+            Some(p)
+        }
     }
 }
 
@@ -327,6 +332,9 @@ impl SequenceIndexOp for BigInt {
             self.try_into().unwrap_or(len)
         }
     }
+    fn wrapped_at(&self, _len: usize) -> Option<usize> {
+        unimplemented!("please add one once we need it")
+    }
 }
 
 /// A saturated slice with values ranging in [isize::MIN, isize::MAX]. Used for

vm/src/stdlib/collections.rs

@@ -13,7 +13,7 @@ mod _collections {
         protocol::{PyIterReturn, PySequenceMethods},
         recursion::ReprGuard,
         sequence::MutObjectSequenceOp,
-        sliceable::{self, SequenceIndexOp},
+        sliceable::SequenceIndexOp,
         types::{
             AsSequence, Comparable, Constructor, Hashable, Initializer, IterNext, IterNextIterable,
             Iterable, PyComparisonOp, Unhashable,
@@ -279,7 +279,7 @@ mod _collections {
         #[pymethod(magic)]
         fn getitem(&self, idx: isize, vm: &VirtualMachine) -> PyResult {
             let deque = self.borrow_deque();
-            sliceable::wrap_index(idx, deque.len())
+            idx.wrapped_at(deque.len())
                 .and_then(|i| deque.get(i).cloned())
                 .ok_or_else(|| vm.new_index_error("deque index out of range".to_owned()))
         }
@@ -287,7 +287,7 @@ mod _collections {
         #[pymethod(magic)]
         fn setitem(&self, idx: isize, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
             let mut deque = self.borrow_deque_mut();
-            sliceable::wrap_index(idx, deque.len())
+            idx.wrapped_at(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()))
@@ -296,7 +296,7 @@ mod _collections {
         #[pymethod(magic)]
         fn delitem(&self, idx: isize, vm: &VirtualMachine) -> PyResult<()> {
             let mut deque = self.borrow_deque_mut();
-            sliceable::wrap_index(idx, deque.len())
+            idx.wrapped_at(deque.len())
                 .and_then(|i| deque.remove(i).map(drop))
                 .ok_or_else(|| vm.new_index_error("deque index out of range".to_owned()))
         }