diff --git a/stdlib/src/mmap.rs b/stdlib/src/mmap.rs index 4d5ceeec3..dff120498 100644 --- a/stdlib/src/mmap.rs +++ b/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 { - 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)?; diff --git a/vm/src/builtins/memory.rs b/vm/src/builtins/memory.rs index 69bf1cc97..e1ebbb204 100644 --- a/vm/src/builtins/memory.rs +++ b/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; diff --git a/vm/src/protocol/buffer.rs b/vm/src/protocol/buffer.rs index 5e9063013..6e97fe1b7 100644 --- a/vm/src/protocol/buffer.rs +++ b/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; diff --git a/vm/src/sliceable.rs b/vm/src/sliceable.rs index 564db563f..d94c5b930 100644 --- a/vm/src/sliceable.rs +++ b/vm/src/sliceable.rs @@ -176,7 +176,7 @@ pub trait SliceableSequenceOp { fn len(&self) -> usize; fn wrap_index(&self, p: isize) -> Option { - 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 { - 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; } 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 { + 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 { + unimplemented!("please add one once we need it") + } } /// A saturated slice with values ranging in [isize::MIN, isize::MAX]. Used for diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index dc1480937..34cadd9b0 100644 --- a/vm/src/stdlib/collections.rs +++ b/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())) }