Fix #746: invalid slice with start or stop =-1 when step<0

tests/snippets/builtin_slice.py

@@ -2,24 +2,24 @@ from testutils import assert_raises
 
 a = []
 assert a[:] == []
-assert a[:2**100] == []
-assert a[-2**100:] == []
-assert a[::2**100] == []
+assert a[: 2 ** 100] == []
+assert a[-2 ** 100 :] == []
+assert a[:: 2 ** 100] == []
 assert a[10:20] == []
 assert a[-20:-10] == []
 
 b = [1, 2]
 
 assert b[:] == [1, 2]
-assert b[:2**100] == [1, 2]
-assert b[-2**100:] == [1, 2]
-assert b[2**100:] == []
-assert b[::2**100] == [1]
+assert b[: 2 ** 100] == [1, 2]
+assert b[-2 ** 100 :] == [1, 2]
+assert b[2 ** 100 :] == []
+assert b[:: 2 ** 100] == [1]
 assert b[-10:1] == [1]
 assert b[0:0] == []
 assert b[1:0] == []
 
-assert_raises(ValueError, lambda: b[::0], 'zero step slice')
+assert_raises(ValueError, lambda: b[::0], "zero step slice")
 
 assert b[::-1] == [2, 1]
 assert b[1::-1] == [2, 1]
@@ -33,7 +33,7 @@ c = list(range(10))
 assert c[9:6:-3] == [9]
 assert c[9::-3] == [9, 6, 3, 0]
 assert c[9::-4] == [9, 5, 1]
-assert c[8::-2**100] == [8]
+assert c[8 :: -2 ** 100] == [8]
 
 assert c[7:7:-2] == []
 assert c[7:8:-2] == []
@@ -43,6 +43,7 @@ d = "123456"
 assert d[3::-1] == "4321"
 assert d[4::-3] == "52"
 
+assert [1, 2, 3, 5, 6][-1:-5:-1] == [6, 5, 3, 2]  # #746
 
 slice_a = slice(5)
 assert slice_a.start is None
@@ -71,3 +72,59 @@ class SubScript(object):
 ss = SubScript()
 _ = ss[:]
 ss[:1] = 1
+
+
+def test_all_slices():
+    """
+    test all possible slices except big number
+    """
+    MODE = None  # set to "build" to rebuild slice_res.py
+    ll = [0, 1, 2, 3]
+    start = list(range(-7, 7))
+    end = list(range(-7, 7))
+    step = list(range(-5, 5))
+    step.pop(step.index(0))
+
+    for i in [start, end, step]:
+        i.append(None)
+
+    def build():
+        # loop used to build slices_res.py with cpython
+        with open("slice_res.py", "wt") as f:
+            for s in start:
+                for e in end:
+                    for t in step:
+                        f.write(str(ll[s:e:t]) + "\n")
+
+    def run():
+        # test utility
+        from slice_res import SLICES_RES
+
+        count = 0
+        failures = []
+        for s in start:
+            for e in end:
+                for t in step:
+                    lhs = ll[s:e:t]
+                    try:
+                        assert lhs == SLICES_RES[count]
+                    except AssertionError:
+                        failures.append(
+                            "start: {} ,stop: {}, step {}. Expected: {}, found: {}".format(
+                                s, e, t, lhs, SLICES_RES[count]
+                            )
+                        )
+                    count += 1
+
+        if failures:
+            for f in failures:
+                print(f)
+            print(len(failures), "slices failed")
+
+    if MODE == "build":
+        build()
+    else:
+        run()
+
+
+test_all_slices()

vm/src/obj/objsequence.rs

@@ -2,11 +2,10 @@ use std::cell::RefCell;
 use std::marker::Sized;
 use std::ops::{Deref, DerefMut, Range};
 
-use num_bigint::BigInt;
-use num_traits::{One, Signed, ToPrimitive, Zero};
-
 use crate::pyobject::{IdProtocol, PyObject, PyObjectRef, PyResult, TypeProtocol};
 use crate::vm::VirtualMachine;
+use num_bigint::{BigInt, ToBigInt};
+use num_traits::{One, Signed, ToPrimitive, Zero};
 
 use super::objbool;
 use super::objint::PyInt;
@@ -86,8 +85,20 @@ pub trait PySliceableSequence {
                 } else {
                     // calculate the range for the reverse slice, first the bounds needs to be made
                     // exclusive around stop, the lower number
-                    let start = start.as_ref().map(|x| x + 1);
-                    let stop = stop.as_ref().map(|x| x + 1);
+                    let start = start.as_ref().map(|x| {
+                        if *x == (-1).to_bigint().unwrap() {
+                            self.len() + BigInt::one() //.to_bigint().unwrap()
+                        } else {
+                            x + 1
+                        }
+                    });
+                    let stop = stop.as_ref().map(|x| {
+                        if *x == (-1).to_bigint().unwrap() {
+                            self.len().to_bigint().unwrap()
+                        } else {
+                            x + 1
+                        }
+                    });
                     let range = self.get_slice_range(&stop, &start);
                     if range.start < range.end {
                         match (-step).to_i32() {