summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMichael Droettboom <mdboom@gmail.com>2024-11-07 16:39:23 (GMT)
committerGitHub <noreply@github.com>2024-11-07 16:39:23 (GMT)
commita38e82bd8c249c126ab033c078170b6dea27a619 (patch)
tree30d94d6b3ac4f1c8ada50988a33bc3ae6656a4f3
parent9357fdcaf0b08dac9396c17e8695b420fad887f8 (diff)
downloadcpython-a38e82bd8c249c126ab033c078170b6dea27a619.zip
cpython-a38e82bd8c249c126ab033c078170b6dea27a619.tar.gz
cpython-a38e82bd8c249c126ab033c078170b6dea27a619.tar.bz2
gh-126298: Don't deduplicate slice constants based on equality (#126398)
* gh-126298: Don't deduplicated slice constants based on equality * NULL check for PySlice_New * Fix refcounting * Fix refcounting some more * Fix refcounting * Make tests more complete * Fix tests
-rw-r--r--Lib/test/test_compile.py78
-rw-r--r--Objects/codeobject.c35
2 files changed, 93 insertions, 20 deletions
diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py
index 85ae71c..519a120 100644
--- a/Lib/test/test_compile.py
+++ b/Lib/test/test_compile.py
@@ -2,6 +2,7 @@ import contextlib
import dis
import io
import itertools
+import marshal
import math
import opcode
import os
@@ -1385,52 +1386,91 @@ class TestSpecifics(unittest.TestCase):
self.assertEqual(actual, expected)
def check_consts(func, typ, expected):
- slice_consts = 0
+ expected = set([repr(x) for x in expected])
+ all_consts = set()
consts = func.__code__.co_consts
for instr in dis.Bytecode(func):
if instr.opname == "LOAD_CONST" and isinstance(consts[instr.oparg], typ):
- slice_consts += 1
- self.assertEqual(slice_consts, expected)
+ all_consts.add(repr(consts[instr.oparg]))
+ self.assertEqual(all_consts, expected)
def load():
return x[a:b] + x [a:] + x[:b] + x[:]
+ check_op_count(load, "BINARY_SLICE", 3)
+ check_op_count(load, "BUILD_SLICE", 0)
+ check_consts(load, slice, [slice(None, None, None)])
+ check_op_count(load, "BINARY_SUBSCR", 1)
+
def store():
x[a:b] = y
x [a:] = y
x[:b] = y
x[:] = y
+ check_op_count(store, "STORE_SLICE", 3)
+ check_op_count(store, "BUILD_SLICE", 0)
+ check_op_count(store, "STORE_SUBSCR", 1)
+ check_consts(store, slice, [slice(None, None, None)])
+
def long_slice():
return x[a:b:c]
+ check_op_count(long_slice, "BUILD_SLICE", 1)
+ check_op_count(long_slice, "BINARY_SLICE", 0)
+ check_consts(long_slice, slice, [])
+ check_op_count(long_slice, "BINARY_SUBSCR", 1)
+
def aug():
x[a:b] += y
+ check_op_count(aug, "BINARY_SLICE", 1)
+ check_op_count(aug, "STORE_SLICE", 1)
+ check_op_count(aug, "BUILD_SLICE", 0)
+ check_op_count(aug, "BINARY_SUBSCR", 0)
+ check_op_count(aug, "STORE_SUBSCR", 0)
+ check_consts(aug, slice, [])
+
def aug_const():
x[1:2] += y
+ check_op_count(aug_const, "BINARY_SLICE", 0)
+ check_op_count(aug_const, "STORE_SLICE", 0)
+ check_op_count(aug_const, "BINARY_SUBSCR", 1)
+ check_op_count(aug_const, "STORE_SUBSCR", 1)
+ check_consts(aug_const, slice, [slice(1, 2)])
+
def compound_const_slice():
x[1:2:3, 4:5:6] = y
- check_op_count(load, "BINARY_SLICE", 3)
- check_op_count(load, "BUILD_SLICE", 0)
- check_consts(load, slice, 1)
- check_op_count(store, "STORE_SLICE", 3)
- check_op_count(store, "BUILD_SLICE", 0)
- check_consts(store, slice, 1)
- check_op_count(long_slice, "BUILD_SLICE", 1)
- check_op_count(long_slice, "BINARY_SLICE", 0)
- check_op_count(aug, "BINARY_SLICE", 1)
- check_op_count(aug, "STORE_SLICE", 1)
- check_op_count(aug, "BUILD_SLICE", 0)
- check_op_count(aug_const, "BINARY_SLICE", 0)
- check_op_count(aug_const, "STORE_SLICE", 0)
- check_consts(aug_const, slice, 1)
check_op_count(compound_const_slice, "BINARY_SLICE", 0)
check_op_count(compound_const_slice, "BUILD_SLICE", 0)
- check_consts(compound_const_slice, slice, 0)
- check_consts(compound_const_slice, tuple, 1)
+ check_op_count(compound_const_slice, "STORE_SLICE", 0)
+ check_op_count(compound_const_slice, "STORE_SUBSCR", 1)
+ check_consts(compound_const_slice, slice, [])
+ check_consts(compound_const_slice, tuple, [(slice(1, 2, 3), slice(4, 5, 6))])
+
+ def mutable_slice():
+ x[[]:] = y
+
+ check_consts(mutable_slice, slice, {})
+
+ def different_but_equal():
+ x[:0] = y
+ x[:0.0] = y
+ x[:False] = y
+ x[:None] = y
+
+ check_consts(
+ different_but_equal,
+ slice,
+ [
+ slice(None, 0, None),
+ slice(None, 0.0, None),
+ slice(None, False, None),
+ slice(None, None, None)
+ ]
+ )
def test_compare_positions(self):
for opname_prefix, op in [
diff --git a/Objects/codeobject.c b/Objects/codeobject.c
index 1cf9740..dba43d5 100644
--- a/Objects/codeobject.c
+++ b/Objects/codeobject.c
@@ -2388,7 +2388,6 @@ _PyCode_ConstantKey(PyObject *op)
if (op == Py_None || op == Py_Ellipsis
|| PyLong_CheckExact(op)
|| PyUnicode_CheckExact(op)
- || PySlice_Check(op)
/* code_richcompare() uses _PyCode_ConstantKey() internally */
|| PyCode_Check(op))
{
@@ -2496,6 +2495,40 @@ _PyCode_ConstantKey(PyObject *op)
Py_DECREF(set);
return key;
}
+ else if (PySlice_Check(op)) {
+ PySliceObject *slice = (PySliceObject *)op;
+ PyObject *start_key = NULL;
+ PyObject *stop_key = NULL;
+ PyObject *step_key = NULL;
+ key = NULL;
+
+ start_key = _PyCode_ConstantKey(slice->start);
+ if (start_key == NULL) {
+ goto slice_exit;
+ }
+
+ stop_key = _PyCode_ConstantKey(slice->stop);
+ if (stop_key == NULL) {
+ goto slice_exit;
+ }
+
+ step_key = _PyCode_ConstantKey(slice->step);
+ if (step_key == NULL) {
+ goto slice_exit;
+ }
+
+ PyObject *slice_key = PySlice_New(start_key, stop_key, step_key);
+ if (slice_key == NULL) {
+ goto slice_exit;
+ }
+
+ key = PyTuple_Pack(2, slice_key, op);
+ Py_DECREF(slice_key);
+ slice_exit:
+ Py_XDECREF(start_key);
+ Py_XDECREF(stop_key);
+ Py_XDECREF(step_key);
+ }
else {
/* for other types, use the object identifier as a unique identifier
* to ensure that they are seen as unequal. */