Skip to content

Commit 0539d33

Browse files
DimitrisJimDimitrisJim
committed
Export saturated index via slicable, use better naming conventions.
1 parent 72ad712 commit 0539d33

File tree

7 files changed

+37
-39
lines changed

7 files changed

+37
-39
lines changed

‎extra_tests/snippets/bad_indexing.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,24 +12,21 @@ def __index__(self):
1212
return 1
1313

1414

15-
def run_setitem():
15+
def run_setslice():
1616
with assert_raises(IndexError):
1717
e[BadIndex()] = 42
1818
e[BadIndex():0:-1] = e
1919
e[0:BadIndex():1] = e
2020
e[0:10:BadIndex()] = e
2121

2222

23-
def run_delitem():
23+
def run_delslice():
2424
del e[BadIndex():0:-1]
2525
del e[0:BadIndex():1]
2626
del e[0:10:BadIndex()]
2727

2828
# Check types
2929
instances = [list(), bytearray(), array('b')]
3030
for e in instances:
31-
print("Trying for ", type(e).__name__)
32-
run_setitem()
33-
print("setitem ok")
34-
run_delitem()
35-
print("delitem ok")
31+
run_setslice()
32+
run_delslice()

‎stdlib/src/array.rs

Lines changed: 7 additions & 7 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::{PySliceableSequence, PySliceableSequenceMut, SaturatedIndices, SequenceIndex},
26+
sliceable::{PySliceableSequence, PySliceableSequenceMut, SaturatedSlice, SequenceIndex},
2727
slots::{
2828
AsBuffer, AsMapping, Comparable, Iterable, IteratorIterable, PyComparisonOp,
2929
SlotConstructor, SlotIterator,
@@ -278,7 +278,7 @@ mod array {
278278
SequenceIndex::Slice(slice) => {
279279
// TODO: Use interface similar to set/del item. This can
280280
// still hang.
281-
let slice = slice.to_saturated_indices(vm)?;
281+
let slice = slice.to_saturated(vm)?;
282282
let elements = v.get_slice_items(vm, slice)?;
283283
let array: PyArray = ArrayContentType::$n(elements).into();
284284
Ok(array.into_object(vm))
@@ -290,7 +290,7 @@ mod array {
290290

291291
fn setitem_by_slice(
292292
&mut self,
293-
slice: SaturatedIndices,
293+
slice: SaturatedSlice,
294294
items: &ArrayContentType,
295295
vm: &VirtualMachine
296296
) -> PyResult<()> {
@@ -307,7 +307,7 @@ mod array {
307307

308308
fn setitem_by_slice_no_resize(
309309
&mut self,
310-
slice: SaturatedIndices,
310+
slice: SaturatedSlice,
311311
items: &ArrayContentType,
312312
vm: &VirtualMachine
313313
) -> PyResult<()> {
@@ -353,7 +353,7 @@ mod array {
353353

354354
fn delitem_by_slice(
355355
&mut self,
356-
slice: SaturatedIndices,
356+
slice: SaturatedSlice,
357357
vm: &VirtualMachine
358358
) -> PyResult<()> {
359359
match self {
@@ -985,7 +985,7 @@ mod array {
985985
match SequenceIndex::try_from_object_for(vm, needle, "array")? {
986986
SequenceIndex::Int(i) => zelf.write().setitem_by_idx(i, obj, vm),
987987
SequenceIndex::Slice(slice) => {
988-
let slice = slice.to_saturated_indices(vm)?;
988+
let slice = slice.to_saturated(vm)?;
989989
let cloned;
990990
let guard;
991991
let items = if zelf.is(&obj) {
@@ -1019,7 +1019,7 @@ mod array {
10191019
match SequenceIndex::try_from_object_for(vm, needle, "array")? {
10201020
SequenceIndex::Int(i) => zelf.try_resizable(vm)?.delitem_by_idx(i, vm),
10211021
SequenceIndex::Slice(slice) => {
1022-
let slice = slice.to_saturated_indices(vm)?;
1022+
let slice = slice.to_saturated(vm)?;
10231023
zelf.try_resizable(vm)?.delitem_by_slice(slice, vm)
10241024
}
10251025
}

‎vm/src/builtins/bytearray.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ impl PyByteArray {
182182
}
183183
}
184184
SequenceIndex::Slice(slice) => {
185-
let slice = slice.to_saturated_indices(vm)?;
185+
let slice = slice.to_saturated(vm)?;
186186
let items = if zelf.is(&value) {
187187
zelf.borrow_buf().to_vec()
188188
} else {
@@ -224,7 +224,7 @@ impl PyByteArray {
224224
}
225225
}
226226
SequenceIndex::Slice(slice) => {
227-
let slice = slice.to_saturated_indices(vm)?;
227+
let slice = slice.to_saturated(vm)?;
228228
let elements = &mut self.try_resizable(vm)?.elements;
229229
elements.delete_slice(vm, slice)
230230
}

‎vm/src/builtins/list.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ impl PyList {
213213
fn setslice(&self, slice: PySliceRef, sec: ArgIterable, vm: &VirtualMachine) -> PyResult<()> {
214214
let items: Result<Vec<PyObjectRef>, _> = sec.iter(vm)?.collect();
215215
let items = items?;
216-
let slice = slice.to_saturated_indices(vm)?;
216+
let slice = slice.to_saturated(vm)?;
217217
let mut elements = self.borrow_vec_mut();
218218
elements.set_slice_items(vm, slice, items.as_slice())
219219
}
@@ -374,7 +374,7 @@ impl PyList {
374374
}
375375

376376
fn delslice(&self, slice: PySliceRef, vm: &VirtualMachine) -> PyResult<()> {
377-
let slice = slice.to_saturated_indices(vm)?;
377+
let slice = slice.to_saturated(vm)?;
378378
self.borrow_vec_mut().delete_slice(vm, slice)
379379
}
380380

‎vm/src/builtins/memory.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::{
99
bytesinner::bytes_to_hex,
1010
function::{FuncArgs, IntoPyObject, OptionalArg},
1111
protocol::{BufferInternal, BufferOptions, PyBuffer, PyMappingMethods},
12-
sliceable::{wrap_index, SaturatedIndices, SequenceIndex},
12+
sliceable::{wrap_index, SaturatedSlice, SequenceIndex},
1313
slots::{AsBuffer, AsMapping, Comparable, Hashable, PyComparisonOp, SlotConstructor},
1414
stdlib::pystruct::FormatSpec,
1515
utils::Either,
@@ -254,7 +254,7 @@ impl PyMemoryView {
254254
// slicing a memoryview return a new memoryview
255255
let len = zelf.buffer.options.len;
256256
let (range, step, is_negative_step) =
257-
SaturatedIndices::new(&slice, vm)?.adjust_indices(len);
257+
SaturatedSlice::with_slice(&slice, vm)?.adjust_indices(len);
258258
let abs_step = step.unwrap();
259259
let step = if is_negative_step {
260260
-(abs_step as isize)
@@ -395,7 +395,7 @@ impl PyMemoryView {
395395
}
396396

397397
let (range, step, is_negative_step) =
398-
SaturatedIndices::new(&slice, vm)?.adjust_indices(zelf.buffer.options.len);
398+
SaturatedSlice::with_slice(&slice, vm)?.adjust_indices(zelf.buffer.options.len);
399399

400400
let bytes = items.to_contiguous();
401401
assert_eq!(bytes.len(), len * itemsize);

‎vm/src/builtins/slice.rs

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ impl PySlice {
7171
))
7272
}
7373

74-
pub fn to_saturated_indices(&self, vm: &VirtualMachine) -> PyResult<SaturatedIndices> {
75-
SaturatedIndices::new(self, vm)
74+
pub fn to_saturated(&self, vm: &VirtualMachine) -> PyResult<SaturatedSlice> {
75+
SaturatedSlice::with_slice(self, vm)
7676
}
7777

7878
#[pyslot]
@@ -255,15 +255,15 @@ impl Unhashable for PySlice {}
255255
/// sequence length. The reason this is important is due to the fact that an objects
256256
/// `__index__` might get a lock on the sequence and cause a deadlock.
257257
#[derive(Copy, Clone, Debug)]
258-
pub struct SaturatedIndices {
258+
pub struct SaturatedSlice {
259259
start: isize,
260260
stop: isize,
261261
step: isize,
262262
}
263263

264-
impl SaturatedIndices {
264+
impl SaturatedSlice {
265265
// Equivalent to PySlice_Unpack.
266-
pub fn new(slice: &PySlice, vm: &VirtualMachine) -> PyResult<Self> {
266+
pub fn with_slice(slice: &PySlice, vm: &VirtualMachine) -> PyResult<Self> {
267267
let step = to_isize_index(vm, slice.step_ref(vm))?.unwrap_or(1);
268268
if step == 0 {
269269
return Err(vm.new_value_error("slice step cannot be zero".to_owned()));
@@ -290,18 +290,18 @@ impl SaturatedIndices {
290290
/// Convert for usage in indexing the underlying rust collections. Called *after*
291291
/// __index__ has been called on the Slice which might mutate the collection.
292292
pub fn adjust_indices(&self, len: usize) -> (Range<usize>, Option<usize>, bool) {
293-
// todo: handle this
294-
let len = len as isize;
293+
// len should always be <= isize::MAX
294+
let ilen = len.to_isize().unwrap_or(isize::MAX);
295295
let (start, stop, step) = (self.start, self.stop, self.step);
296296
let (start, stop, step, is_negative_step) = if step.is_negative() {
297297
(
298298
if stop == -1 {
299-
len.saturating_add(1)
299+
ilen.saturating_add(1)
300300
} else {
301301
stop.saturating_add(1)
302302
},
303303
if start == -1 {
304-
len
304+
ilen
305305
} else {
306306
start.saturating_add(1)
307307
},
@@ -357,7 +357,8 @@ fn to_isize_index(vm: &VirtualMachine, obj: &PyObjectRef) -> PyResult<Option<isi
357357
}
358358

359359
// Saturate p in range [0, len] inclusive
360-
pub fn saturate_index(p: isize, len: isize) -> usize {
360+
pub fn saturate_index(p: isize, len: usize) -> usize {
361+
let len = len.to_isize().unwrap_or(isize::MAX);
361362
let mut p = p;
362363
if p < 0 {
363364
p += len;

‎vm/src/sliceable.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ use std::ops::Range;
33

44
use crate::builtins::int::PyInt;
55
// export through slicable module, not slice.
6-
pub use crate::builtins::slice::SaturatedIndices;
7-
use crate::builtins::slice::{saturate_index, PySlice, PySliceRef};
6+
pub use crate::builtins::slice::{saturate_index, SaturatedSlice};
7+
use crate::builtins::slice::{PySlice, PySliceRef};
88
use crate::utils::Either;
99
use crate::VirtualMachine;
1010
use crate::{PyObjectRef, PyResult, TypeProtocol};
@@ -25,7 +25,7 @@ pub trait PySliceableSequenceMut {
2525
fn set_slice_items_no_resize(
2626
&mut self,
2727
vm: &VirtualMachine,
28-
slice: SaturatedIndices,
28+
slice: SaturatedSlice,
2929
items: &[Self::Item],
3030
) -> PyResult<()> {
3131
let (range, step, is_negative_step) = slice.adjust_indices(self.as_slice().len());
@@ -83,7 +83,7 @@ pub trait PySliceableSequenceMut {
8383
fn set_slice_items(
8484
&mut self,
8585
vm: &VirtualMachine,
86-
slice: SaturatedIndices,
86+
slice: SaturatedSlice,
8787
items: &[Self::Item],
8888
) -> PyResult<()> {
8989
let (range, step, is_negative_step) = slice.adjust_indices(self.as_slice().len());
@@ -136,7 +136,7 @@ pub trait PySliceableSequenceMut {
136136
}
137137
}
138138

139-
fn delete_slice(&mut self, _vm: &VirtualMachine, slice: SaturatedIndices) -> PyResult<()> {
139+
fn delete_slice(&mut self, _vm: &VirtualMachine, slice: SaturatedSlice) -> PyResult<()> {
140140
let (range, step, is_negative_step) = slice.adjust_indices(self.as_slice().len());
141141
if range.start >= range.end {
142142
return Ok(());
@@ -229,13 +229,13 @@ pub trait PySliceableSequence {
229229
}
230230

231231
fn saturate_index(&self, p: isize) -> usize {
232-
saturate_index(p, self.len() as isize)
232+
saturate_index(p, self.len())
233233
}
234234

235235
fn get_slice_items(
236236
&self,
237237
_vm: &VirtualMachine,
238-
slice: SaturatedIndices,
238+
slice: SaturatedSlice,
239239
) -> PyResult<Self::Sliced> {
240240
let (range, step, is_negative_step) = slice.adjust_indices(self.len());
241241
if range.start >= range.end {
@@ -276,7 +276,7 @@ pub trait PySliceableSequence {
276276
Ok(Either::A(self.do_get(pos_index)))
277277
}
278278
SequenceIndex::Slice(slice) => {
279-
let slice = slice.to_saturated_indices(vm)?;
279+
let slice = slice.to_saturated(vm)?;
280280
Ok(Either::B(self.get_slice_items(vm, slice)?))
281281
}
282282
}

0 commit comments

Comments
 (0)