Skip to content

Commit fac6e31

Browse files
DimitrisJimDimitrisJim
committed
Use SaturatedIndices instead of convert_slice.
1 parent 79ca53d commit fac6e31

File tree

3 files changed

+23
-121
lines changed

3 files changed

+23
-121
lines changed

‎stdlib/src/array.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ mod array {
2323
BufferInternal, BufferOptions, BufferResizeGuard, PyBuffer, PyIterReturn,
2424
PyMappingMethods,
2525
},
26-
sliceable::{saturate_index, PySliceableSequence, PySliceableSequenceMut, SequenceIndex},
26+
sliceable::{PySliceableSequence, PySliceableSequenceMut, SequenceIndex},
2727
slots::{
2828
AsBuffer, AsMapping, Comparable, Iterable, IteratorIterable, PyComparisonOp,
2929
SlotConstructor, SlotIterator,
@@ -122,14 +122,14 @@ mod array {
122122

123123
fn insert(
124124
&mut self,
125-
i: usize,
125+
i: isize,
126126
obj: PyObjectRef,
127127
vm: &VirtualMachine
128128
) -> PyResult<()> {
129129
match self {
130130
$(ArrayContentType::$n(v) => {
131131
let val = <$t>::try_into_from_object(vm, obj)?;
132-
v.insert(i, val);
132+
v.insert(v.saturate_index(i), val);
133133
})*
134134
}
135135
Ok(())
@@ -895,7 +895,6 @@ mod array {
895895
vm: &VirtualMachine,
896896
) -> PyResult<()> {
897897
let mut w = zelf.try_resizable(vm)?;
898-
let i = saturate_index(i, w.len());
899898
w.insert(i, x, vm)
900899
}
901900

‎vm/src/builtins/memory.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
use super::{PyBytes, PyBytesRef, PyList, PyListRef, PySliceRef, PyStr, PyStrRef, PyTypeRef};
1+
use super::{
2+
slice::SaturatedIndices, PyBytes, PyBytesRef, PyList, PyListRef, PySliceRef, PyStr, PyStrRef,
3+
PyTypeRef,
4+
};
25
use crate::common::{
36
borrow::{BorrowedValue, BorrowedValueMut},
47
hash::PyHash,
@@ -9,7 +12,7 @@ use crate::{
912
bytesinner::bytes_to_hex,
1013
function::{FuncArgs, IntoPyObject, OptionalArg},
1114
protocol::{BufferInternal, BufferOptions, PyBuffer, PyMappingMethods},
12-
sliceable::{convert_slice, wrap_index, SequenceIndex},
15+
sliceable::{wrap_index, SequenceIndex},
1316
slots::{AsBuffer, AsMapping, Comparable, Hashable, PyComparisonOp, SlotConstructor},
1417
stdlib::pystruct::FormatSpec,
1518
utils::Either,
@@ -253,7 +256,7 @@ impl PyMemoryView {
253256
fn getitem_by_slice(zelf: PyRef<Self>, slice: PySliceRef, vm: &VirtualMachine) -> PyResult {
254257
// slicing a memoryview return a new memoryview
255258
let len = zelf.buffer.options.len;
256-
let (range, step, is_negative_step) = convert_slice(&slice, len, vm)?;
259+
let (range, step, is_negative_step) = SaturatedIndices::new(&slice, vm)?.adjust_indices(len);
257260
let abs_step = step.unwrap();
258261
let step = if is_negative_step {
259262
-(abs_step as isize)
@@ -393,7 +396,8 @@ impl PyMemoryView {
393396
return diff_err();
394397
}
395398

396-
let (range, step, is_negative_step) = convert_slice(&slice, zelf.buffer.options.len, vm)?;
399+
let (range, step, is_negative_step) =
400+
SaturatedIndices::new(&slice, vm)?.adjust_indices(zelf.buffer.options.len);
397401

398402
let bytes = items.to_contiguous();
399403
assert_eq!(bytes.len(), len * itemsize);

‎vm/src/sliceable.rs

Lines changed: 12 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
use num_bigint::BigInt;
2-
use num_traits::{One, Signed, ToPrimitive, Zero};
1+
use num_traits::ToPrimitive;
32
use std::ops::Range;
43

54
use crate::builtins::int::PyInt;
6-
use crate::builtins::slice::{PySlice, PySliceRef};
5+
use crate::builtins::slice::{saturate_index, PySlice, PySliceRef, SaturatedIndices};
76
use crate::utils::Either;
87
use crate::VirtualMachine;
98
use crate::{PyObjectRef, PyResult, TypeProtocol};
@@ -27,7 +26,8 @@ pub trait PySliceableSequenceMut {
2726
slice: &PySlice,
2827
items: &[Self::Item],
2928
) -> PyResult<()> {
30-
let (range, step, is_negative_step) = convert_slice(slice, self.as_slice().len(), vm)?;
29+
let (range, step, is_negative_step) =
30+
SaturatedIndices::new(slice, vm)?.adjust_indices(self.as_slice().len());
3131
if !is_negative_step && step == Some(1) {
3232
return if range.end - range.start == items.len() {
3333
self.do_set_range(range, items);
@@ -85,7 +85,8 @@ pub trait PySliceableSequenceMut {
8585
slice: &PySlice,
8686
items: &[Self::Item],
8787
) -> PyResult<()> {
88-
let (range, step, is_negative_step) = convert_slice(slice, self.as_slice().len(), vm)?;
88+
let (range, step, is_negative_step) =
89+
SaturatedIndices::new(slice, vm)?.adjust_indices(self.as_slice().len());
8990
if !is_negative_step && step == Some(1) {
9091
self.do_set_range(range, items);
9192
return Ok(());
@@ -136,7 +137,8 @@ pub trait PySliceableSequenceMut {
136137
}
137138

138139
fn delete_slice(&mut self, vm: &VirtualMachine, slice: &PySlice) -> PyResult<()> {
139-
let (range, step, is_negative_step) = convert_slice(slice, self.as_slice().len(), vm)?;
140+
let (range, step, is_negative_step) =
141+
SaturatedIndices::new(slice, vm)?.adjust_indices(self.as_slice().len());
140142
if range.start >= range.end {
141143
return Ok(());
142144
}
@@ -228,19 +230,12 @@ pub trait PySliceableSequence {
228230
}
229231

230232
fn saturate_index(&self, p: isize) -> usize {
231-
saturate_index(p, self.len())
232-
}
233-
234-
fn saturate_big_index(&self, slice_pos: &BigInt) -> usize {
235-
saturate_big_index(slice_pos, self.len())
236-
}
237-
238-
fn saturate_range(&self, start: &Option<BigInt>, stop: &Option<BigInt>) -> Range<usize> {
239-
saturate_range(start, stop, self.len())
233+
saturate_index(p, self.len() as isize)
240234
}
241235

242236
fn get_slice_items(&self, vm: &VirtualMachine, slice: &PySlice) -> PyResult<Self::Sliced> {
243-
let (range, step, is_negative_step) = convert_slice(slice, self.len(), vm)?;
237+
let (range, step, is_negative_step) =
238+
SaturatedIndices::new(slice, vm)?.adjust_indices(self.len());
244239
if range.start >= range.end {
245240
return Ok(Self::empty());
246241
}
@@ -395,100 +390,4 @@ pub(crate) fn wrap_index(p: isize, len: usize) -> Option<usize> {
395390
} else {
396391
Some(p)
397392
}
398-
}
399-
400-
// return pos is in range [0, len] inclusive
401-
pub fn saturate_index(p: isize, len: usize) -> usize {
402-
let mut p = p;
403-
let len = len.to_isize().unwrap();
404-
if p < 0 {
405-
p += len;
406-
if p < 0 {
407-
p = 0;
408-
}
409-
}
410-
if p > len {
411-
p = len;
412-
}
413-
p as usize
414-
}
415-
416-
fn saturate_big_index(slice_pos: &BigInt, len: usize) -> usize {
417-
if let Some(pos) = slice_pos.to_isize() {
418-
saturate_index(pos, len)
419-
} else if slice_pos.is_negative() {
420-
// slice past start bound, round to start
421-
0
422-
} else {
423-
// slice past end bound, round to end
424-
len
425-
}
426-
}
427-
428-
pub(crate) fn saturate_range(
429-
start: &Option<BigInt>,
430-
stop: &Option<BigInt>,
431-
len: usize,
432-
) -> Range<usize> {
433-
let start = start.as_ref().map_or(0, |x| saturate_big_index(x, len));
434-
let stop = stop.as_ref().map_or(len, |x| saturate_big_index(x, len));
435-
436-
start..stop
437-
}
438-
439-
pub(crate) fn convert_slice(
440-
slice: &PySlice,
441-
len: usize,
442-
vm: &VirtualMachine,
443-
) -> PyResult<(Range<usize>, Option<usize>, bool)> {
444-
let start = slice.start_index(vm)?;
445-
let stop = slice.stop_index(vm)?;
446-
let step = slice.step_index(vm)?.unwrap_or_else(BigInt::one);
447-
448-
if step.is_zero() {
449-
return Err(vm.new_value_error("slice step cannot be zero".to_owned()));
450-
}
451-
452-
let (start, stop, step, is_negative_step) = if step.is_negative() {
453-
(
454-
stop.map(|x| {
455-
if x == -BigInt::one() {
456-
len + BigInt::one()
457-
} else {
458-
x + 1
459-
}
460-
}),
461-
start.map(|x| {
462-
if x == -BigInt::one() {
463-
BigInt::from(len)
464-
} else {
465-
x + 1
466-
}
467-
}),
468-
-step,
469-
true,
470-
)
471-
} else {
472-
(start, stop, step, false)
473-
};
474-
475-
let step = step.to_usize();
476-
477-
let range = saturate_range(&start, &stop, len);
478-
let range = if range.start >= range.end {
479-
range.start..range.start
480-
} else {
481-
// step overflow
482-
if step.is_none() {
483-
if is_negative_step {
484-
(range.end - 1)..range.end
485-
} else {
486-
range.start..(range.start + 1)
487-
}
488-
} else {
489-
range
490-
}
491-
};
492-
493-
Ok((range, step, is_negative_step))
494-
}
393+
}

0 commit comments

Comments
 (0)