Skip to content

Commit a3c2eea

Browse files
coolreader18coolreader18
authored
Merge pull request RustPython#1410 from dralley/fix-range
Fix range slicing behavior
2 parents 0160cd1 + c6bebae commit a3c2eea

File tree

5 files changed

+75
-62
lines changed

5 files changed

+75
-62
lines changed

‎tests/snippets/builtin_range.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@
3535
assert range(10)[:] == range(10)
3636
assert range(10, 0, -2)[0:5:2] == range(10, 0, -4)
3737
assert range(10)[10:11] == range(10,10)
38+
assert range(0, 10, -1)[::-1] == range(1, 1)
39+
assert range(0, 10)[::-1] == range(9, -1, -1)
40+
assert range(0, -10)[::-1] == range(-1, -1, -1)
41+
assert range(0, -10)[::-1][::-1] == range(0, 0)
42+
assert_raises(ValueError, lambda: range(0, 10)[::0], _msg='slice step cannot be zero')
43+
assert_raises(TypeError, lambda: range(0, 10)['a':], _msg='slice indices must be integers or None or have an __index__ method')
3844

3945
# count tests
4046
assert range(10).count(2) == 1

‎vm/src/builtins.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -380,11 +380,8 @@ fn builtin_max(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
380380

381381
if candidates.is_empty() {
382382
let default = args.get_optional_kwarg("default");
383-
if default.is_none() {
384-
return Err(vm.new_value_error("max() arg is an empty sequence".to_string()));
385-
} else {
386-
return Ok(default.unwrap());
387-
}
383+
return default
384+
.ok_or_else(|| vm.new_value_error("max() arg is an empty sequence".to_string()));
388385
}
389386

390387
let key_func = args.get_optional_kwarg("key");
@@ -429,11 +426,8 @@ fn builtin_min(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
429426

430427
if candidates.is_empty() {
431428
let default = args.get_optional_kwarg("default");
432-
if default.is_none() {
433-
return Err(vm.new_value_error("min() arg is an empty sequence".to_string()));
434-
} else {
435-
return Ok(default.unwrap());
436-
}
429+
return default
430+
.ok_or_else(|| vm.new_value_error("min() arg is an empty sequence".to_string()));
437431
}
438432

439433
let key_func = args.get_optional_kwarg("key");

‎vm/src/obj/objbytes.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ impl PyBytesRef {
509509
}
510510
}
511511
None => {
512-
if replacing_char.is_none() {
512+
if replacing_char.is_some() {
513513
decode_content.push(replacing_char.unwrap())
514514
}
515515
}

‎vm/src/obj/objrange.rs

Lines changed: 63 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,15 @@ impl PyRange {
8282
return None;
8383
}
8484

85-
let length = if start < stop {
85+
let length: BigInt = if start < stop {
8686
(stop - start - 1) / step + 1
8787
} else {
8888
(start - stop - 1) / (-step) + 1
8989
};
9090

91-
let index = if index < BigInt::zero() {
92-
let new_index = &length + &index;
93-
if new_index < BigInt::zero() {
91+
let index = if index.is_negative() {
92+
let new_index: BigInt = &length + &index;
93+
if new_index.is_negative() {
9494
return None;
9595
}
9696
length + index
@@ -320,73 +320,86 @@ impl PyRange {
320320
fn getitem(&self, subscript: RangeIndex, vm: &VirtualMachine) -> PyResult {
321321
match subscript {
322322
RangeIndex::Slice(slice) => {
323-
let start = self.start.as_bigint();
324-
let stop = self.stop.as_bigint();
325-
let step = self.step.as_bigint();
326-
327-
let new_start = if let Some(int) = slice.start_index(vm)? {
328-
let int = &int;
329-
if let Some(i) = self.get(int) {
330-
PyInt::new(i).into_ref(vm)
331-
} else if start < stop {
332-
if stop <= int {
333-
self.stop.clone()
334-
} else {
335-
self.start.clone()
336-
}
337-
} else if int < stop {
338-
self.stop.clone()
339-
} else {
340-
self.start.clone()
323+
let range_start = self.start.as_bigint();
324+
let range_step = self.step.as_bigint();
325+
let _tmp_len = self.length();
326+
let range_length = _tmp_len.as_bigint();
327+
328+
let substep = if let Some(slice_step) = slice.step_index(vm)? {
329+
if slice_step.is_zero() {
330+
return Err(vm.new_value_error("slice step cannot be zero".to_string()));
341331
}
332+
slice_step
342333
} else {
343-
self.start.clone()
334+
BigInt::one()
344335
};
345336

346-
let new_end = if let Some(int) = slice.stop_index(vm)? {
347-
let int = &int;
348-
if let Some(i) = self.get(int) {
349-
PyInt::new(i).into_ref(vm)
350-
} else if start < stop {
351-
if int < start {
352-
self.start.clone()
337+
let negative_step = substep.is_negative();
338+
let lower_bound = if negative_step {
339+
-BigInt::one()
340+
} else {
341+
BigInt::zero()
342+
};
343+
let upper_bound = if negative_step {
344+
&lower_bound + range_length
345+
} else {
346+
range_length.clone()
347+
};
348+
349+
let substart = if let Some(slice_start) = slice.start_index(vm)? {
350+
if slice_start.is_negative() {
351+
let tmp = slice_start + range_length;
352+
if tmp < lower_bound {
353+
lower_bound.clone()
353354
} else {
354-
self.stop.clone()
355+
tmp.clone()
355356
}
356-
} else if start < int {
357-
self.start.clone()
357+
} else if slice_start > upper_bound {
358+
upper_bound.clone()
358359
} else {
359-
self.stop.clone()
360+
slice_start.clone()
360361
}
362+
} else if negative_step {
363+
upper_bound.clone()
361364
} else {
362-
self.stop.clone()
365+
lower_bound.clone()
363366
};
364367

365-
let new_step = if let Some(int) = slice.step_index(vm)? {
366-
if step.is_zero() {
367-
return Err(vm.new_value_error("slice step cannot be zero".to_string()));
368+
let substop = if let Some(slice_stop) = slice.stop_index(vm)? {
369+
if slice_stop.is_negative() {
370+
let tmp = slice_stop + range_length;
371+
if tmp < lower_bound {
372+
lower_bound.clone()
373+
} else {
374+
tmp.clone()
375+
}
376+
} else if slice_stop > upper_bound {
377+
upper_bound.clone()
368378
} else {
369-
PyInt::new(int * self.step.as_bigint()).into_ref(vm)
379+
slice_stop.clone()
370380
}
381+
} else if negative_step {
382+
lower_bound.clone()
371383
} else {
372-
self.step.clone()
384+
upper_bound.clone()
373385
};
374386

387+
let step = range_step * &substep;
388+
let start = range_start + (&substart * range_step);
389+
let stop = range_start + (&substop * range_step);
390+
375391
Ok(PyRange {
376-
start: new_start,
377-
stop: new_end,
378-
step: new_step,
392+
start: PyInt::new(start).into_ref(vm),
393+
stop: PyInt::new(stop).into_ref(vm),
394+
step: PyInt::new(step).into_ref(vm),
379395
}
380396
.into_ref(vm)
381397
.into_object())
382398
}
383-
RangeIndex::Int(index) => {
384-
if let Some(value) = self.get(index.as_bigint()) {
385-
Ok(PyInt::new(value).into_ref(vm).into_object())
386-
} else {
387-
Err(vm.new_index_error("range object index out of range".to_string()))
388-
}
389-
}
399+
RangeIndex::Int(index) => match self.get(index.as_bigint()) {
400+
Some(value) => Ok(PyInt::new(value).into_ref(vm).into_object()),
401+
None => Err(vm.new_index_error("range object index out of range".to_string())),
402+
},
390403
}
391404
}
392405

‎vm/src/obj/objsequence.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ pub fn get_mut_elements<'a>(obj: &'a PyObjectRef) -> impl DerefMut<Target = Vec<
465465
panic!("Cannot extract elements from non-sequence");
466466
}
467467

468-
//Check if given arg could be used with PySciceableSequance.get_slice_range()
468+
//Check if given arg could be used with PySliceableSequence.get_slice_range()
469469
pub fn is_valid_slice_arg(
470470
arg: OptionalArg<PyObjectRef>,
471471
vm: &VirtualMachine,

0 commit comments

Comments
 (0)