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

Author: mdboom

Lib/test/test_compile.py

@@ -2,6 +2,7 @@
 import dis
 import io
 import itertools
+import marshal
 import math
 import opcode
 import os
@@ -1385,52 +1386,91 @@ def check_op_count(func, op, expected):
             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 [

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. */