Skip to content

Commit 098a709

Browse files
youknowoneyouknowone
authored
Merge pull request #3944 from youknowone/refactor-range
Refactor repetive optional range pattern to OptionalRangeArgs
2 parents 722f8fd + 90affcd commit 098a709

File tree

5 files changed

+39
-46
lines changed

5 files changed

+39
-46
lines changed

‎stdlib/src/array.rs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ mod array {
2323
BufferDescriptor, BufferMethods, BufferResizeGuard, PyBuffer, PyIterReturn,
2424
PyMappingMethods,
2525
},
26-
sequence::{SequenceExt, SequenceMutExt},
26+
sequence::{OptionalRangeArgs, SequenceExt, SequenceMutExt},
2727
sliceable::{
2828
SaturatedSlice, SequenceIndex, SequenceIndexOp, SliceableSequenceMutOp,
2929
SliceableSequenceOp,
@@ -867,17 +867,10 @@ mod array {
867867
fn index(
868868
&self,
869869
x: PyObjectRef,
870-
start: OptionalArg<PyObjectRef>,
871-
stop: OptionalArg<PyObjectRef>,
870+
range: OptionalRangeArgs,
872871
vm: &VirtualMachine,
873872
) -> PyResult<usize> {
874-
let len = self.len();
875-
let saturate = |obj: PyObjectRef, len| -> PyResult<_> {
876-
obj.try_into_value(vm)
877-
.map(|int: PyIntRef| int.as_bigint().saturated_at(len))
878-
};
879-
let start = start.map_or(Ok(0), |obj| saturate(obj, len))?;
880-
let stop = stop.map_or(Ok(len), |obj| saturate(obj, len))?;
873+
let (start, stop) = range.saturate(self.len(), vm)?;
881874
self.read().index(x, start, stop, vm)
882875
}
883876

‎vm/src/builtins/list.rs

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::{PositionIterInternal, PyGenericAlias, PyIntRef, PyTupleRef, PyType, PyTypeRef};
1+
use super::{PositionIterInternal, PyGenericAlias, PyTupleRef, PyType, PyTypeRef};
22
use crate::common::lock::{
33
PyMappedRwLockReadGuard, PyMutex, PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard,
44
};
@@ -9,8 +9,8 @@ use crate::{
99
iter::PyExactSizeIterator,
1010
protocol::{PyIterReturn, PyMappingMethods, PySequence, PySequenceMethods},
1111
recursion::ReprGuard,
12-
sequence::{MutObjectSequenceOp, SequenceExt, SequenceMutExt},
13-
sliceable::{SequenceIndex, SequenceIndexOp, SliceableSequenceMutOp, SliceableSequenceOp},
12+
sequence::{MutObjectSequenceOp, OptionalRangeArgs, SequenceExt, SequenceMutExt},
13+
sliceable::{SequenceIndex, SliceableSequenceMutOp, SliceableSequenceOp},
1414
types::{
1515
AsMapping, AsSequence, Comparable, Constructor, Hashable, Initializer, IterNext,
1616
IterNextIterable, Iterable, PyComparisonOp, Unconstructible, Unhashable,
@@ -269,17 +269,10 @@ impl PyList {
269269
fn index(
270270
&self,
271271
needle: PyObjectRef,
272-
start: OptionalArg<PyObjectRef>,
273-
stop: OptionalArg<PyObjectRef>,
272+
range: OptionalRangeArgs,
274273
vm: &VirtualMachine,
275274
) -> PyResult<usize> {
276-
let len = self.len();
277-
let saturate = |obj: PyObjectRef, len| -> PyResult<_> {
278-
obj.try_into_value(vm)
279-
.map(|int: PyIntRef| int.as_bigint().saturated_at(len))
280-
};
281-
let start = start.map_or(Ok(0), |obj| saturate(obj, len))?;
282-
let stop = stop.map_or(Ok(len), |obj| saturate(obj, len))?;
275+
let (start, stop) = range.saturate(self.len(), vm)?;
283276
let index = self.mut_index_range(vm, &needle, start..stop)?;
284277
if let Some(index) = index.into() {
285278
Ok(index)

‎vm/src/builtins/tuple.rs

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::{PositionIterInternal, PyGenericAlias, PyIntRef, PyType, PyTypeRef};
1+
use super::{PositionIterInternal, PyGenericAlias, PyType, PyTypeRef};
22
use crate::common::{hash::PyHash, lock::PyMutex};
33
use crate::{
44
class::PyClassImpl,
@@ -7,8 +7,7 @@ use crate::{
77
iter::PyExactSizeIterator,
88
protocol::{PyIterReturn, PyMappingMethods, PySequenceMethods},
99
recursion::ReprGuard,
10-
sequence::SequenceExt,
11-
sliceable::SequenceIndexOp,
10+
sequence::{OptionalRangeArgs, SequenceExt},
1211
sliceable::{SequenceIndex, SliceableSequenceOp},
1312
types::{
1413
AsMapping, AsSequence, Comparable, Constructor, Hashable, IterNext, IterNextIterable,
@@ -281,17 +280,10 @@ impl PyTuple {
281280
fn index(
282281
&self,
283282
needle: PyObjectRef,
284-
start: OptionalArg<PyObjectRef>,
285-
stop: OptionalArg<PyObjectRef>,
283+
range: OptionalRangeArgs,
286284
vm: &VirtualMachine,
287285
) -> PyResult<usize> {
288-
let len = self.len();
289-
let saturate = |obj: PyObjectRef, len| -> PyResult<_> {
290-
obj.try_into_value(vm)
291-
.map(|int: PyIntRef| int.as_bigint().saturated_at(len))
292-
};
293-
let start = start.map_or(Ok(0), |i| saturate(i, len))?;
294-
let stop = stop.map_or(Ok(len), |i| saturate(i, len))?;
286+
let (start, stop) = range.saturate(self.len(), vm)?;
295287
for (index, element) in self
296288
.elements
297289
.iter()

‎vm/src/sequence.rs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use crate::{
2-
function::{Either, PyComparisonValue},
2+
builtins::PyIntRef,
3+
function::{Either, OptionalArg, PyComparisonValue},
4+
sliceable::SequenceIndexOp,
35
types::{richcompare_wrapper, PyComparisonOp, RichCompareFunc},
46
vm::VirtualMachine,
57
AsObject, PyObject, PyObjectRef, PyResult,
@@ -225,3 +227,23 @@ impl<T: Clone> SequenceMutExt<T> for Vec<T> {
225227
self
226228
}
227229
}
230+
231+
#[derive(FromArgs)]
232+
pub struct OptionalRangeArgs {
233+
#[pyarg(positional, optional)]
234+
start: OptionalArg<PyObjectRef>,
235+
#[pyarg(positional, optional)]
236+
stop: OptionalArg<PyObjectRef>,
237+
}
238+
239+
impl OptionalRangeArgs {
240+
pub fn saturate(self, len: usize, vm: &VirtualMachine) -> PyResult<(usize, usize)> {
241+
let saturate = |obj: PyObjectRef| -> PyResult<_> {
242+
obj.try_into_value(vm)
243+
.map(|int: PyIntRef| int.as_bigint().saturated_at(len))
244+
};
245+
let start = self.start.map_or(Ok(0), saturate)?;
246+
let stop = self.stop.map_or(Ok(len), saturate)?;
247+
Ok((start, stop))
248+
}
249+
}

‎vm/src/stdlib/collections.rs

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ mod _collections {
55
use crate::{
66
builtins::{
77
IterStatus::{Active, Exhausted},
8-
PositionIterInternal, PyGenericAlias, PyInt, PyIntRef, PyTypeRef,
8+
PositionIterInternal, PyGenericAlias, PyInt, PyTypeRef,
99
},
1010
common::lock::{PyMutex, PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard},
1111
function::{FuncArgs, KwArgs, OptionalArg, PyComparisonValue},
1212
iter::PyExactSizeIterator,
1313
protocol::{PyIterReturn, PySequenceMethods},
1414
recursion::ReprGuard,
15-
sequence::MutObjectSequenceOp,
15+
sequence::{MutObjectSequenceOp, OptionalRangeArgs},
1616
sliceable::SequenceIndexOp,
1717
types::{
1818
AsSequence, Comparable, Constructor, Hashable, Initializer, IterNext, IterNextIterable,
@@ -157,19 +157,12 @@ mod _collections {
157157
fn index(
158158
&self,
159159
needle: PyObjectRef,
160-
start: OptionalArg<PyObjectRef>,
161-
stop: OptionalArg<PyObjectRef>,
160+
range: OptionalRangeArgs,
162161
vm: &VirtualMachine,
163162
) -> PyResult<usize> {
164163
let start_state = self.state.load();
165164

166-
let len = self.len();
167-
let saturate = |obj: PyObjectRef, len| -> PyResult<_> {
168-
obj.try_into_value(vm)
169-
.map(|int: PyIntRef| int.as_bigint().saturated_at(len))
170-
};
171-
let start = start.map_or(Ok(0), |i| saturate(i, len))?;
172-
let stop = stop.map_or(Ok(len), |i| saturate(i, len))?;
165+
let (start, stop) = range.saturate(self.len(), vm)?;
173166
let index = self.mut_index_range(vm, &needle, start..stop)?;
174167
if start_state != self.state.load() {
175168
Err(vm.new_runtime_error("deque mutated during iteration".to_owned()))

0 commit comments

Comments
 (0)