Skip to content

Commit 2c88790

Browse files
coolreader18coolreader18
authored
Merge pull request RustPython#766 from jgirardet/seq
Fix RustPython#746: invalid slice with start or stop =-1 when step<0
2 parents 2220397 + 8692811 commit 2c88790

4 files changed

Lines changed: 132 additions & 45 deletions

File tree

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@ __pycache__
99
.vscode
1010
wasm-pack.log
1111
.idea/
12+
tests/snippets/resources

‎tests/snippets/builtin_slice.py‎

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,24 @@
22

33
a = []
44
assert a[:] == []
5-
assert a[:2**100] == []
6-
assert a[-2**100:] == []
7-
assert a[::2**100] == []
5+
assert a[: 2 ** 100] == []
6+
assert a[-2 ** 100 :] == []
7+
assert a[:: 2 ** 100] == []
88
assert a[10:20] == []
99
assert a[-20:-10] == []
1010

1111
b = [1, 2]
1212

1313
assert b[:] == [1, 2]
14-
assert b[:2**100] == [1, 2]
15-
assert b[-2**100:] == [1, 2]
16-
assert b[2**100:] == []
17-
assert b[::2**100] == [1]
14+
assert b[: 2 ** 100] == [1, 2]
15+
assert b[-2 ** 100 :] == [1, 2]
16+
assert b[2 ** 100 :] == []
17+
assert b[:: 2 ** 100] == [1]
1818
assert b[-10:1] == [1]
1919
assert b[0:0] == []
2020
assert b[1:0] == []
2121

22-
assert_raises(ValueError, lambda: b[::0], 'zero step slice')
22+
assert_raises(ValueError, lambda: b[::0], "zero step slice")
2323

2424
assert b[::-1] == [2, 1]
2525
assert b[1::-1] == [2, 1]
@@ -33,7 +33,7 @@
3333
assert c[9:6:-3] == [9]
3434
assert c[9::-3] == [9, 6, 3, 0]
3535
assert c[9::-4] == [9, 5, 1]
36-
assert c[8::-2**100] == [8]
36+
assert c[8 :: -2 ** 100] == [8]
3737

3838
assert c[7:7:-2] == []
3939
assert c[7:8:-2] == []
@@ -43,6 +43,7 @@
4343
assert d[3::-1] == "4321"
4444
assert d[4::-3] == "52"
4545

46+
assert [1, 2, 3, 5, 6][-1:-5:-1] == [6, 5, 3, 2] # #746
4647

4748
slice_a = slice(5)
4849
assert slice_a.start is None
@@ -71,3 +72,41 @@ def __setitem__(self, key, value):
7172
ss = SubScript()
7273
_ = ss[:]
7374
ss[:1] = 1
75+
76+
77+
def test_all_slices():
78+
"""
79+
test all possible slices except big number
80+
"""
81+
82+
mod = __import__('cpython_generated_slices')
83+
84+
ll = mod.LL
85+
start = mod.START
86+
end = mod.END
87+
step = mod.STEP
88+
slices_res = mod.SLICES_RES
89+
90+
count = 0
91+
failures = []
92+
for s in start:
93+
for e in end:
94+
for t in step:
95+
lhs = ll[s:e:t]
96+
try:
97+
assert lhs == slices_res[count]
98+
except AssertionError:
99+
failures.append(
100+
"start: {} ,stop: {}, step {}. Expected: {}, found: {}".format(
101+
s, e, t, lhs, slices_res[count]
102+
)
103+
)
104+
count += 1
105+
106+
if failures:
107+
for f in failures:
108+
print(f)
109+
print(len(failures), "slices failed")
110+
111+
112+
test_all_slices()

‎tests/test_snippets.py‎

Lines changed: 66 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
# This is a python unittest class automatically populating with all tests
32
# in the tests folder.
43

@@ -11,6 +10,8 @@
1110
import subprocess
1211
import contextlib
1312
import enum
13+
from pathlib import Path
14+
import shutil
1415

1516
import compile_code
1617

@@ -19,13 +20,11 @@ class _TestType(enum.Enum):
1920
functional = 1
2021

2122

22-
logger = logging.getLogger('tests')
23-
ROOT_DIR = '..'
24-
TEST_ROOT = os.path.abspath(os.path.join(ROOT_DIR, 'tests'))
25-
TEST_DIRS = {
26-
_TestType.functional: os.path.join(TEST_ROOT, 'snippets'),
27-
}
28-
CPYTHON_RUNNER_DIR = os.path.abspath(os.path.join(ROOT_DIR, 'py_code_object'))
23+
logger = logging.getLogger("tests")
24+
ROOT_DIR = ".."
25+
TEST_ROOT = os.path.abspath(os.path.join(ROOT_DIR, "tests"))
26+
TEST_DIRS = {_TestType.functional: os.path.join(TEST_ROOT, "snippets")}
27+
CPYTHON_RUNNER_DIR = os.path.abspath(os.path.join(ROOT_DIR, "py_code_object"))
2928
RUSTPYTHON_RUNNER_DIR = os.path.abspath(os.path.join(ROOT_DIR))
3029
RUSTPYTHON_LIB_DIR = os.path.abspath(os.path.join(ROOT_DIR, "Lib"))
3130

@@ -38,12 +37,12 @@ def pushd(path):
3837

3938

4039
def perform_test(filename, method, test_type):
41-
logger.info('Running %s via %s', filename, method)
42-
if method == 'cpython':
40+
logger.info("Running %s via %s", filename, method)
41+
if method == "cpython":
4342
run_via_cpython(filename)
44-
elif method == 'cpython_bytecode':
43+
elif method == "cpython_bytecode":
4544
run_via_cpython_bytecode(filename, test_type)
46-
elif method == 'rustpython':
45+
elif method == "rustpython":
4746
run_via_rustpython(filename, test_type)
4847
else:
4948
raise NotImplementedError(method)
@@ -57,16 +56,16 @@ def run_via_cpython(filename):
5756

5857
def run_via_cpython_bytecode(filename, test_type):
5958
# Step1: Create bytecode file:
60-
bytecode_filename = filename + '.bytecode'
61-
with open(bytecode_filename, 'w') as f:
59+
bytecode_filename = filename + ".bytecode"
60+
with open(bytecode_filename, "w") as f:
6261
compile_code.compile_to_bytecode(filename, out_file=f)
6362

6463
# Step2: run cpython bytecode:
6564
env = os.environ.copy()
66-
env['RUST_LOG'] = 'info,cargo=error,jobserver=error'
67-
env['RUST_BACKTRACE'] = '1'
65+
env["RUST_LOG"] = "info,cargo=error,jobserver=error"
66+
env["RUST_BACKTRACE"] = "1"
6867
with pushd(CPYTHON_RUNNER_DIR):
69-
subprocess.check_call(['cargo', 'run', bytecode_filename], env=env)
68+
subprocess.check_call(["cargo", "run", bytecode_filename], env=env)
7069

7170

7271
def run_via_rustpython(filename, test_type):
@@ -75,26 +74,26 @@ def run_via_rustpython(filename, test_type):
7574
env['RUST_BACKTRACE'] = '1'
7675
env['PYTHONPATH'] = RUSTPYTHON_LIB_DIR
7776

78-
target = 'release'
79-
if env.get('CODE_COVERAGE', 'false') == 'true':
80-
target = 'debug'
81-
binary = os.path.abspath(os.path.join(ROOT_DIR, 'target', target, 'rustpython'))
77+
target = "release"
78+
if env.get("CODE_COVERAGE", "false") == "true":
79+
target = "debug"
80+
binary = os.path.abspath(os.path.join(ROOT_DIR, "target", target, "rustpython"))
8281

8382
subprocess.check_call([binary, filename], env=env)
8483

8584

8685
def create_test_function(cls, filename, method, test_type):
8786
""" Create a test function for a single snippet """
8887
core_test_directory, snippet_filename = os.path.split(filename)
89-
test_function_name = 'test_{}_'.format(method) \
90-
+ os.path.splitext(snippet_filename)[0] \
91-
.replace('.', '_').replace('-', '_')
88+
test_function_name = "test_{}_".format(method) + os.path.splitext(snippet_filename)[
89+
0
90+
].replace(".", "_").replace("-", "_")
9291

9392
def test_function(self):
9493
perform_test(filename, method, test_type)
9594

9695
if hasattr(cls, test_function_name):
97-
raise ValueError('Duplicate test case {}'.format(test_function_name))
96+
raise ValueError("Duplicate test case {}".format(test_function_name))
9897
setattr(cls, test_function_name, test_function)
9998

10099

@@ -104,25 +103,61 @@ def wrapper(cls):
104103
for test_type, filename in get_test_files():
105104
create_test_function(cls, filename, method, test_type)
106105
return cls
106+
107107
return wrapper
108108

109109

110110
def get_test_files():
111111
""" Retrieve test files """
112112
for test_type, test_dir in TEST_DIRS.items():
113-
for filepath in sorted(glob.iglob(os.path.join(test_dir, '*.py'))):
113+
for filepath in sorted(glob.iglob(os.path.join(test_dir, "*.py"))):
114114
filename = os.path.split(filepath)[1]
115-
if filename.startswith('xfail_'):
115+
if filename.startswith("xfail_"):
116116
continue
117117

118118
yield test_type, os.path.abspath(filepath)
119119

120120

121-
@populate('cpython')
121+
def generate_slices(path):
122+
# loop used to build slices_res.py with cpython
123+
ll = [0, 1, 2, 3]
124+
start = list(range(-7, 7))
125+
end = list(range(-7, 7))
126+
step = list(range(-5, 5))
127+
step.pop(step.index(0))
128+
for i in [start, end, step]:
129+
i.append(None)
130+
131+
slices_res = []
132+
for s in start:
133+
for e in end:
134+
for t in step:
135+
slices_res.append(ll[s:e:t])
136+
137+
path.write_text(
138+
"SLICES_RES={}\nSTART= {}\nEND= {}\nSTEP= {}\nLL={}\n".format(
139+
slices_res, start, end, step, ll
140+
)
141+
)
142+
143+
144+
@populate("cpython")
122145
# @populate('cpython_bytecode')
123-
@populate('rustpython')
146+
@populate("rustpython")
124147
class SampleTestCase(unittest.TestCase):
125148
@classmethod
126149
def setUpClass(cls):
127-
subprocess.check_call(['cargo', 'build'])
128-
subprocess.check_call(['cargo', 'build', '--release'])
150+
# Here add resource files
151+
cls.slices_resource_path = Path(TEST_DIRS[_TestType.functional]) / "cpython_generated_slices.py"
152+
if cls.slices_resource_path.exists():
153+
cls.slices_resource_path.unlink()
154+
155+
generate_slices(cls.slices_resource_path)
156+
157+
# cargo stuff
158+
subprocess.check_call(["cargo", "build"])
159+
subprocess.check_call(["cargo", "build", "--release"])
160+
161+
@classmethod
162+
def tearDownClass(cls):
163+
cls.slices_resource_path.unlink()

‎vm/src/obj/objsequence.rs‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ use std::cell::RefCell;
22
use std::marker::Sized;
33
use std::ops::{Deref, DerefMut, Range};
44

5-
use num_bigint::BigInt;
6-
use num_traits::{One, Signed, ToPrimitive, Zero};
7-
85
use crate::pyobject::{IdProtocol, PyObject, PyObjectRef, PyResult, TryFromObject, TypeProtocol};
6+
97
use crate::vm::VirtualMachine;
8+
use num_bigint::{BigInt, ToBigInt};
9+
use num_traits::{One, Signed, ToPrimitive, Zero};
1010

1111
use super::objbool;
1212
use super::objint::PyInt;
@@ -86,8 +86,20 @@ pub trait PySliceableSequence {
8686
} else {
8787
// calculate the range for the reverse slice, first the bounds needs to be made
8888
// exclusive around stop, the lower number
89-
let start = start.as_ref().map(|x| x + 1);
90-
let stop = stop.as_ref().map(|x| x + 1);
89+
let start = start.as_ref().map(|x| {
90+
if *x == (-1).to_bigint().unwrap() {
91+
self.len() + BigInt::one() //.to_bigint().unwrap()
92+
} else {
93+
x + 1
94+
}
95+
});
96+
let stop = stop.as_ref().map(|x| {
97+
if *x == (-1).to_bigint().unwrap() {
98+
self.len().to_bigint().unwrap()
99+
} else {
100+
x + 1
101+
}
102+
});
91103
let range = self.get_slice_range(&stop, &start);
92104
if range.start < range.end {
93105
match (-step).to_i32() {

0 commit comments

Comments
 (0)