Skip to content

Commit 699c801

Browse files
youknowoneyouknowone
committed
Fix sequence index utilities not to use get_ prefix
1 parent 6fda0d3 commit 699c801

File tree

5 files changed

+52
-56
lines changed

5 files changed

+52
-56
lines changed

‎vm/src/bytesinner.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@ use crate::pyobject::{
1818
BorrowValue, Either, PyComparisonValue, PyIterable, PyIterator, PyObjectRef, PyResult,
1919
TryFromObject, TypeProtocol,
2020
};
21-
use crate::sliceable::{
22-
get_saturated_pos, PySliceableSequence, PySliceableSequenceMut, SequenceIndex,
23-
};
21+
use crate::sliceable::{PySliceableSequence, PySliceableSequenceMut, SequenceIndex};
2422
use crate::slots::PyComparisonOp;
2523
use crate::vm::VirtualMachine;
2624
use rustpython_common::hash;
@@ -300,7 +298,7 @@ impl PyBytesInner {
300298
pub fn getitem(&self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult {
301299
match needle {
302300
SequenceIndex::Int(int) => {
303-
if let Some(idx) = self.elements.get_pos(int) {
301+
if let Some(idx) = self.elements.wrap_index(int) {
304302
Ok(vm.ctx.new_int(self.elements[idx]))
305303
} else {
306304
Err(vm.new_index_error("index out of range".to_owned()))
@@ -318,7 +316,7 @@ impl PyBytesInner {
318316
object: PyObjectRef,
319317
vm: &VirtualMachine,
320318
) -> PyResult<()> {
321-
if let Some(idx) = self.elements.get_pos(int) {
319+
if let Some(idx) = self.elements.wrap_index(int) {
322320
let value = Self::value_try_from_object(vm, object)?;
323321
self.elements[idx] = value;
324322
Ok(())
@@ -374,7 +372,7 @@ impl PyBytesInner {
374372
pub fn delitem(&mut self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult<()> {
375373
match needle {
376374
SequenceIndex::Int(int) => {
377-
if let Some(idx) = self.elements.get_pos(int) {
375+
if let Some(idx) = self.elements.wrap_index(int) {
378376
self.elements.remove(idx);
379377
Ok(())
380378
} else {
@@ -390,7 +388,7 @@ impl PyBytesInner {
390388
}
391389

392390
pub fn pop(&mut self, index: isize, vm: &VirtualMachine) -> PyResult<u8> {
393-
if let Some(index) = self.elements.get_pos(index) {
391+
if let Some(index) = self.elements.wrap_index(index) {
394392
Ok(self.elements.remove(index))
395393
} else {
396394
Err(vm.new_index_error("index out of range".to_owned()))
@@ -404,7 +402,7 @@ impl PyBytesInner {
404402
vm: &VirtualMachine,
405403
) -> PyResult<()> {
406404
let value = Self::value_try_from_object(vm, object)?;
407-
let index = get_saturated_pos(index, self.len());
405+
let index = self.elements.saturate_index(index);
408406
self.elements.insert(index, value);
409407
Ok(())
410408
}

‎vm/src/obj/objlist.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@ use crate::pyobject::{
1818
PyResult, PyValue, TryFromObject, TypeProtocol,
1919
};
2020
use crate::sequence::{self, SimpleSeq};
21-
use crate::sliceable::{
22-
get_item, get_saturated_pos, PySliceableSequence, PySliceableSequenceMut, SequenceIndex,
23-
};
21+
use crate::sliceable::{get_item, PySliceableSequence, PySliceableSequenceMut, SequenceIndex};
2422
use crate::slots::{Comparable, Hashable, PyComparisonOp, Unhashable};
2523
use crate::vm::{ReprGuard, VirtualMachine};
2624

@@ -120,7 +118,7 @@ impl PyList {
120118
#[pymethod]
121119
pub(crate) fn insert(&self, position: isize, element: PyObjectRef) {
122120
let mut elements = self.borrow_value_mut();
123-
let position = get_saturated_pos(position, elements.len());
121+
let position = elements.saturate_index(position);
124122
elements.insert(position, element);
125123
}
126124

@@ -228,7 +226,7 @@ impl PyList {
228226

229227
fn setindex(&self, index: isize, mut value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
230228
let mut elements = self.borrow_value_mut();
231-
if let Some(pos_index) = elements.get_pos(index) {
229+
if let Some(pos_index) = elements.wrap_index(index) {
232230
std::mem::swap(&mut elements[pos_index], &mut value);
233231
Ok(())
234232
} else {
@@ -361,7 +359,7 @@ impl PyList {
361359
fn delindex(&self, index: isize, vm: &VirtualMachine) -> PyResult<()> {
362360
let removed = {
363361
let mut elements = self.borrow_value_mut();
364-
if let Some(pos_index) = elements.get_pos(index) {
362+
if let Some(pos_index) = elements.wrap_index(index) {
365363
// defer delete out of borrow
366364
Ok(elements.remove(pos_index))
367365
} else {

‎vm/src/sliceable.rs

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ pub trait PySliceableSequenceMut {
3434
return Err(vm.new_value_error("slice step cannot be zero".to_owned()));
3535
}
3636
if step == BigInt::one() {
37-
let range = self.as_slice().get_slice_range(&start, &stop);
37+
let range = self.as_slice().saturate_range(&start, &stop);
3838
let range = if range.end < range.start {
3939
range.start..range.start
4040
} else {
@@ -67,7 +67,7 @@ pub trait PySliceableSequenceMut {
6767
(start, stop, step, false)
6868
};
6969

70-
let range = self.as_slice().get_slice_range(&start, &stop);
70+
let range = self.as_slice().saturate_range(&start, &stop);
7171
let range = if range.end < range.start {
7272
range.start..range.start
7373
} else {
@@ -137,7 +137,7 @@ pub trait PySliceableSequenceMut {
137137
}
138138

139139
if step == BigInt::one() {
140-
let range = self.as_slice().get_slice_range(&start, &stop);
140+
let range = self.as_slice().saturate_range(&start, &stop);
141141
if range.start < range.end {
142142
self.do_delete_range(range);
143143
}
@@ -167,7 +167,7 @@ pub trait PySliceableSequenceMut {
167167
(start, stop, step, false)
168168
};
169169

170-
let range = self.as_slice().get_slice_range(&start, &stop);
170+
let range = self.as_slice().saturate_range(&start, &stop);
171171
if range.start >= range.end {
172172
return Ok(());
173173
}
@@ -251,19 +251,35 @@ pub trait PySliceableSequence {
251251
fn len(&self) -> usize;
252252
fn is_empty(&self) -> bool;
253253

254-
fn get_pos(&self, p: isize) -> Option<usize> {
255-
get_pos(p, self.len())
254+
fn wrap_index(&self, p: isize) -> Option<usize> {
255+
wrap_index(p, self.len())
256256
}
257257

258-
fn get_slice_pos(&self, slice_pos: &BigInt) -> usize {
259-
get_slice_pos(slice_pos, self.len())
258+
fn saturate_index(&self, p: isize) -> usize {
259+
saturate_index(p, self.len())
260260
}
261261

262-
fn get_slice_range(&self, start: &Option<BigInt>, stop: &Option<BigInt>) -> Range<usize> {
263-
let start = start.as_ref().map(|x| self.get_slice_pos(x)).unwrap_or(0);
262+
fn saturate_big_index(&self, slice_pos: &BigInt) -> usize {
263+
let len = self.len();
264+
if let Some(pos) = slice_pos.to_isize() {
265+
self.saturate_index(pos)
266+
} else if slice_pos.is_negative() {
267+
// slice past start bound, round to start
268+
0
269+
} else {
270+
// slice past end bound, round to end
271+
len
272+
}
273+
}
274+
275+
fn saturate_range(&self, start: &Option<BigInt>, stop: &Option<BigInt>) -> Range<usize> {
276+
let start = start
277+
.as_ref()
278+
.map(|x| self.saturate_big_index(x))
279+
.unwrap_or(0);
264280
let stop = stop
265281
.as_ref()
266-
.map(|x| self.get_slice_pos(x))
282+
.map(|x| self.saturate_big_index(x))
267283
.unwrap_or_else(|| self.len());
268284

269285
start..stop
@@ -276,7 +292,7 @@ pub trait PySliceableSequence {
276292
if step.is_zero() {
277293
Err(vm.new_value_error("slice step cannot be zero".to_owned()))
278294
} else if step.is_positive() {
279-
let range = self.get_slice_range(&start, &stop);
295+
let range = self.saturate_range(&start, &stop);
280296
if range.start < range.end {
281297
#[allow(clippy::range_plus_one)]
282298
match step.to_i32() {
@@ -304,7 +320,7 @@ pub trait PySliceableSequence {
304320
x + 1
305321
}
306322
});
307-
let range = self.get_slice_range(&stop, &start);
323+
let range = self.saturate_range(&stop, &start);
308324
if range.start < range.end {
309325
match (-step).to_i32() {
310326
Some(1) => Ok(self.do_slice_reverse(range)),
@@ -403,7 +419,8 @@ impl TryFromObject for SequenceIndex {
403419
// }
404420
// }
405421

406-
pub fn get_pos(p: isize, len: usize) -> Option<usize> {
422+
// Use PySliceableSequence::wrap_index for implementors
423+
pub(crate) fn wrap_index(p: isize, len: usize) -> Option<usize> {
407424
let neg = p.is_negative();
408425
let p = p.abs().to_usize()?;
409426
if neg {
@@ -416,7 +433,7 @@ pub fn get_pos(p: isize, len: usize) -> Option<usize> {
416433
}
417434

418435
// return pos is in range [0, len] inclusive
419-
pub fn get_saturated_pos(p: isize, len: usize) -> usize {
436+
pub(crate) fn saturate_index(p: isize, len: usize) -> usize {
420437
let mut p = p;
421438
let len = len.to_isize().unwrap();
422439
if p < 0 {
@@ -431,26 +448,9 @@ pub fn get_saturated_pos(p: isize, len: usize) -> usize {
431448
p as usize
432449
}
433450

434-
pub fn get_slice_pos(slice_pos: &BigInt, len: usize) -> usize {
435-
if let Some(pos) = slice_pos.to_isize() {
436-
if let Some(index) = get_pos(pos, len) {
437-
// within bounds
438-
return index;
439-
}
440-
}
441-
442-
if slice_pos.is_negative() {
443-
// slice past start bound, round to start
444-
0
445-
} else {
446-
// slice past end bound, round to end
447-
len
448-
}
449-
}
450-
451-
// pub fn get_slice_range(start: &Option<BigInt>, stop: &Option<BigInt>, len: usize) -> Range<usize> {
452-
// let start = start.as_ref().map_or(0, |x| get_slice_pos(x, len));
453-
// let stop = stop.as_ref().map_or(len, |x| get_slice_pos(x, len));
451+
// pub fn saturate_range(start: &Option<BigInt>, stop: &Option<BigInt>, len: usize) -> Range<usize> {
452+
// let start = start.as_ref().map_or(0, |x| saturate_big_index(x, len));
453+
// let stop = stop.as_ref().map_or(len, |x| saturate_big_index(x, len));
454454

455455
// start..stop
456456
// }
@@ -466,7 +466,7 @@ pub fn get_item(
466466
vm.new_index_error("cannot fit 'int' into an index-sized integer".to_owned())
467467
})?;
468468
let pos_index = elements
469-
.get_pos(value)
469+
.wrap_index(value)
470470
.ok_or_else(|| vm.new_index_error("Index out of bounds!".to_owned()))?;
471471
Ok(Either::A(elements[pos_index].clone()))
472472
} else {
@@ -480,7 +480,7 @@ pub fn get_item(
480480
}
481481
}
482482

483-
//Check if given arg could be used with PySliceableSequence.get_slice_range()
483+
//Check if given arg could be used with PySliceableSequence.saturate_range()
484484
// pub fn is_valid_slice_arg(
485485
// arg: OptionalArg<PyObjectRef>,
486486
// vm: &VirtualMachine,

‎vm/src/stdlib/array.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use crate::pyobject::{
1414
BorrowValue, Either, IdProtocol, IntoPyObject, PyClassImpl, PyComparisonValue, PyIterable,
1515
PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol,
1616
};
17-
use crate::sliceable::{get_saturated_pos, PySliceableSequence, PySliceableSequenceMut};
17+
use crate::sliceable::{saturate_index, PySliceableSequence, PySliceableSequenceMut};
1818
use crate::slots::{Comparable, PyComparisonOp};
1919
use crate::VirtualMachine;
2020
use crossbeam_utils::atomic::AtomicCell;
@@ -581,7 +581,7 @@ impl PyArray {
581581

582582
#[pymethod]
583583
fn insert(&self, i: isize, x: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
584-
let i = get_saturated_pos(i, self.len());
584+
let i = saturate_index(i, self.len());
585585
self.borrow_value_mut().insert(i, x, vm)
586586
}
587587

‎vm/src/stdlib/collections.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -248,15 +248,15 @@ mod _collections {
248248
#[pymethod(magic)]
249249
fn getitem(&self, idx: isize, vm: &VirtualMachine) -> PyResult {
250250
let deque = self.borrow_deque();
251-
sliceable::get_pos(idx, deque.len())
251+
sliceable::wrap_index(idx, deque.len())
252252
.and_then(|i| deque.get(i).cloned())
253253
.ok_or_else(|| vm.new_index_error("deque index out of range".to_owned()))
254254
}
255255

256256
#[pymethod(magic)]
257257
fn setitem(&self, idx: isize, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
258258
let mut deque = self.borrow_deque_mut();
259-
sliceable::get_pos(idx, deque.len())
259+
sliceable::wrap_index(idx, deque.len())
260260
.and_then(|i| deque.get_mut(i))
261261
.map(|x| *x = value)
262262
.ok_or_else(|| vm.new_index_error("deque index out of range".to_owned()))
@@ -265,7 +265,7 @@ mod _collections {
265265
#[pymethod(magic)]
266266
fn delitem(&self, idx: isize, vm: &VirtualMachine) -> PyResult<()> {
267267
let mut deque = self.borrow_deque_mut();
268-
sliceable::get_pos(idx, deque.len())
268+
sliceable::wrap_index(idx, deque.len())
269269
.and_then(|i| deque.remove(i).map(drop))
270270
.ok_or_else(|| vm.new_index_error("deque index out of range".to_owned()))
271271
}

0 commit comments

Comments
 (0)