Skip to content

Commit af304c8

Browse files
committed
gh-126298: Don't deduplicated slice constants based on equality
1 parent 3032fcd commit af304c8

File tree

2 files changed

+82
-21
lines changed

2 files changed

+82
-21
lines changed

Lib/test/test_compile.py

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,53 +1384,90 @@ def check_op_count(func, op, expected):
13841384
actual += 1
13851385
self.assertEqual(actual, expected)
13861386

1387+
def check_num_consts(func, typ, expected):
1388+
num_consts = 0
1389+
consts = func.__code__.co_consts
1390+
for instr in dis.Bytecode(func):
1391+
if instr.opname == "LOAD_CONST" and isinstance(consts[instr.oparg], typ):
1392+
num_consts += 1
1393+
self.assertEqual(num_consts, expected)
1394+
13871395
def check_consts(func, typ, expected):
1388-
slice_consts = 0
1396+
all_consts = set()
13891397
consts = func.__code__.co_consts
13901398
for instr in dis.Bytecode(func):
13911399
if instr.opname == "LOAD_CONST" and isinstance(consts[instr.oparg], typ):
1392-
slice_consts += 1
1393-
self.assertEqual(slice_consts, expected)
1400+
all_consts.add(consts[instr.oparg])
1401+
self.assertEqual(all_consts, expected)
13941402

13951403
def load():
13961404
return x[a:b] + x [a:] + x[:b] + x[:]
13971405

1406+
check_op_count(load, "BINARY_SLICE", 3)
1407+
check_op_count(load, "BUILD_SLICE", 0)
1408+
check_consts(load, slice, {slice(None, None, None)})
1409+
13981410
def store():
13991411
x[a:b] = y
1400-
x [a:] = y
1412+
x[a:] = y
14011413
x[:b] = y
14021414
x[:] = y
14031415

1416+
check_op_count(store, "STORE_SLICE", 3)
1417+
check_op_count(store, "BUILD_SLICE", 0)
1418+
check_consts(store, slice, {slice(None, None, None)})
1419+
14041420
def long_slice():
14051421
return x[a:b:c]
14061422

1423+
check_op_count(long_slice, "BUILD_SLICE", 1)
1424+
check_op_count(long_slice, "BINARY_SLICE", 0)
1425+
check_num_consts(long_slice, slice, 0)
1426+
14071427
def aug():
14081428
x[a:b] += y
14091429

1430+
check_op_count(aug, "BINARY_SLICE", 1)
1431+
check_op_count(aug, "STORE_SLICE", 1)
1432+
check_op_count(aug, "BUILD_SLICE", 0)
1433+
check_num_consts(long_slice, slice, 0)
1434+
14101435
def aug_const():
14111436
x[1:2] += y
14121437

1438+
check_op_count(aug_const, "BINARY_SLICE", 0)
1439+
check_op_count(aug_const, "STORE_SLICE", 0)
1440+
check_consts(aug_const, slice, {slice(1, 2)})
1441+
14131442
def compound_const_slice():
14141443
x[1:2:3, 4:5:6] = y
14151444

1416-
check_op_count(load, "BINARY_SLICE", 3)
1417-
check_op_count(load, "BUILD_SLICE", 0)
1418-
check_consts(load, slice, 1)
1419-
check_op_count(store, "STORE_SLICE", 3)
1420-
check_op_count(store, "BUILD_SLICE", 0)
1421-
check_consts(store, slice, 1)
1422-
check_op_count(long_slice, "BUILD_SLICE", 1)
1423-
check_op_count(long_slice, "BINARY_SLICE", 0)
1424-
check_op_count(aug, "BINARY_SLICE", 1)
1425-
check_op_count(aug, "STORE_SLICE", 1)
1426-
check_op_count(aug, "BUILD_SLICE", 0)
1427-
check_op_count(aug_const, "BINARY_SLICE", 0)
1428-
check_op_count(aug_const, "STORE_SLICE", 0)
1429-
check_consts(aug_const, slice, 1)
14301445
check_op_count(compound_const_slice, "BINARY_SLICE", 0)
14311446
check_op_count(compound_const_slice, "BUILD_SLICE", 0)
1432-
check_consts(compound_const_slice, slice, 0)
1433-
check_consts(compound_const_slice, tuple, 1)
1447+
check_num_consts(compound_const_slice, slice, 0)
1448+
check_consts(compound_const_slice, tuple, {(slice(1, 2, 3), slice(4, 5, 6))})
1449+
1450+
def mutable_slice():
1451+
x[[]:] = y
1452+
1453+
check_num_consts(mutable_slice, slice, 0)
1454+
1455+
def different_but_equal():
1456+
x[:0] = y
1457+
x[:0.0] = y
1458+
x[:False] = y
1459+
x[:None] = y
1460+
1461+
check_consts(
1462+
different_but_equal,
1463+
slice,
1464+
{
1465+
slice(None, 0, None),
1466+
slice(None, 0.0, None),
1467+
slice(None, False, None),
1468+
slice(None, None, None)
1469+
}
1470+
)
14341471

14351472
def test_compare_positions(self):
14361473
for opname_prefix, op in [

Objects/codeobject.c

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2348,7 +2348,6 @@ _PyCode_ConstantKey(PyObject *op)
23482348
if (op == Py_None || op == Py_Ellipsis
23492349
|| PyLong_CheckExact(op)
23502350
|| PyUnicode_CheckExact(op)
2351-
|| PySlice_Check(op)
23522351
/* code_richcompare() uses _PyCode_ConstantKey() internally */
23532352
|| PyCode_Check(op))
23542353
{
@@ -2456,6 +2455,31 @@ _PyCode_ConstantKey(PyObject *op)
24562455
Py_DECREF(set);
24572456
return key;
24582457
}
2458+
else if (PySlice_Check(op)) {
2459+
PySliceObject *slice = (PySliceObject *)op;
2460+
2461+
PyObject *start = slice->start;
2462+
PyObject *start_key = _PyCode_ConstantKey(start);
2463+
if (start_key == NULL) {
2464+
return NULL;
2465+
}
2466+
2467+
PyObject *stop = slice->stop;
2468+
PyObject *stop_key = _PyCode_ConstantKey(stop);
2469+
if (stop_key == NULL) {
2470+
return NULL;
2471+
}
2472+
2473+
PyObject *step = slice->step;
2474+
PyObject *step_key = _PyCode_ConstantKey(step);
2475+
if (step_key == NULL) {
2476+
return NULL;
2477+
}
2478+
2479+
PyObject *slice_key = PySlice_New(start_key, stop_key, step_key);
2480+
key = PyTuple_Pack(2, slice_key, op);
2481+
Py_DECREF(slice_key);
2482+
}
24592483
else {
24602484
/* for other types, use the object identifier as a unique identifier
24612485
* to ensure that they are seen as unequal. */

0 commit comments

Comments
 (0)