code_richcompare() now uses the constants types

Issue #25843: When compiling code, don't merge constants if they are equal but
have a different types. For example, "f1, f2 = lambda: 1, lambda: 1.0" is now
correctly compiled to two different functions: f1() returns 1 (int) and f2()
returns 1.0 (int), even if 1 and 1.0 are equal.

Add a new _PyCode_ConstantKey() private function.

5 files changed, 246 insertions, 50 deletions

diff --git a/Include/code.h b/Include/code.h
index 56e6ec1..8ecf38a 100644
--- a/Include/code.h
+++ b/Include/code.h
@@ -108,12 +108,21 @@ typedef struct _addr_pair {
         int ap_upper;
 } PyAddrPair;
 
+#ifndef Py_LIMITED_API
 /* Update *bounds to describe the first and one-past-the-last instructions in the
    same line as lasti.  Return the number of that line.
 */
-#ifndef Py_LIMITED_API
 PyAPI_FUNC(int) _PyCode_CheckLineNumber(PyCodeObject* co,
                                         int lasti, PyAddrPair *bounds);
+
+/* Create a comparable key used to compare constants taking in account the
+ * object type. It is used to make sure types are not coerced (e.g., float and
+ * complex) _and_ to distinguish 0.0 from -0.0 e.g. on IEEE platforms
+ *
+ * Return (type(obj), obj, ...): a tuple with variable size (at least 2 items)
+ * depending on the type and the value. The type is the first item to not
+ * compare bytes and str which can raise a BytesWarning exception. */
+PyAPI_FUNC(PyObject*) _PyCode_ConstantKey(PyObject *obj);
 #endif
 
 PyAPI_FUNC(PyObject*) PyCode_Optimize(PyObject *code, PyObject* consts,
diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py
index 8935c65..e0fdee3 100644
--- a/Lib/test/test_compile.py
+++ b/Lib/test/test_compile.py
@@ -572,6 +572,88 @@ if 1:
         exec(memoryview(b"ax = 123")[1:-1], namespace)
         self.assertEqual(namespace['x'], 12)
 
+    def check_constant(self, func, expected):
+        for const in func.__code__.co_consts:
+            if repr(const) == repr(expected):
+                break
+        else:
+            self.fail("unable to find constant %r in %r"
+                      % (expected, func.__code__.co_consts))
+
+    # Merging equal constants is not a strict requirement for the Python
+    # semantics, it's a more an implementation detail.
+    @support.cpython_only
+    def test_merge_constants(self):
+        # Issue #25843: compile() must merge constants which are equal
+        # and have the same type.
+
+        def check_same_constant(const):
+            ns = {}
+            code = "f1, f2 = lambda: %r, lambda: %r" % (const, const)
+            exec(code, ns)
+            f1 = ns['f1']
+            f2 = ns['f2']
+            self.assertIs(f1.__code__, f2.__code__)
+            self.check_constant(f1, const)
+            self.assertEqual(repr(f1()), repr(const))
+
+        check_same_constant(None)
+        check_same_constant(0)
+        check_same_constant(0.0)
+        check_same_constant(b'abc')
+        check_same_constant('abc')
+
+        # Note: "lambda: ..." emits "LOAD_CONST Ellipsis",
+        # whereas "lambda: Ellipsis" emits "LOAD_GLOBAL Ellipsis"
+        f1, f2 = lambda: ..., lambda: ...
+        self.assertIs(f1.__code__, f2.__code__)
+        self.check_constant(f1, Ellipsis)
+        self.assertEqual(repr(f1()), repr(Ellipsis))
+
+        # {0} is converted to a constant frozenset({0}) by the peephole
+        # optimizer
+        f1, f2 = lambda x: x in {0}, lambda x: x in {0}
+        self.assertIs(f1.__code__, f2.__code__)
+        self.check_constant(f1, frozenset({0}))
+        self.assertTrue(f1(0))
+
+    def test_dont_merge_constants(self):
+        # Issue #25843: compile() must not merge constants which are equal
+        # but have a different type.
+
+        def check_different_constants(const1, const2):
+            ns = {}
+            exec("f1, f2 = lambda: %r, lambda: %r" % (const1, const2), ns)
+            f1 = ns['f1']
+            f2 = ns['f2']
+            self.assertIsNot(f1.__code__, f2.__code__)
+            self.check_constant(f1, const1)
+            self.check_constant(f2, const2)
+            self.assertEqual(repr(f1()), repr(const1))
+            self.assertEqual(repr(f2()), repr(const2))
+
+        check_different_constants(0, 0.0)
+        check_different_constants(+0.0, -0.0)
+        check_different_constants((0,), (0.0,))
+
+        # check_different_constants() cannot be used because repr(-0j) is
+        # '(-0-0j)', but when '(-0-0j)' is evaluated to 0j: we loose the sign.
+        f1, f2 = lambda: +0.0j, lambda: -0.0j
+        self.assertIsNot(f1.__code__, f2.__code__)
+        self.check_constant(f1, +0.0j)
+        self.check_constant(f2, -0.0j)
+        self.assertEqual(repr(f1()), repr(+0.0j))
+        self.assertEqual(repr(f2()), repr(-0.0j))
+
+        # {0} is converted to a constant frozenset({0}) by the peephole
+        # optimizer
+        f1, f2 = lambda x: x in {0}, lambda x: x in {0.0}
+        self.assertIsNot(f1.__code__, f2.__code__)
+        self.check_constant(f1, frozenset({0}))
+        self.check_constant(f2, frozenset({0.0}))
+        self.assertTrue(f1(0))
+        self.assertTrue(f2(0.0))
+
 
 class TestStackSize(unittest.TestCase):
     # These tests check that the computed stack size for a code object
diff --git a/Misc/NEWS b/Misc/NEWS
index dc375d0..75df532 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -17,6 +17,12 @@ Core and Builtins
   Python 3.5.1 to hide the exact implementation of atomic C types, to avoid
   compiler issues.
 
+- Issue #25843: When compiling code, don't merge constants if they are equal
+  but have a different types. For example, ``f1, f2 = lambda: 1, lambda: 1.0``
+  is now correctly compiled to two different functions: ``f1()`` returns ``1``
+  (``int``) and ``f2()`` returns ``1.0`` (``int``), even if ``1`` and ``1.0``
+  are equal.
+
 - Issue #25731: Fix set and deleting __new__ on a class.
 
 - Issue #22995: [UPDATE] Comment out the one of the pickleability tests in
diff --git a/Objects/codeobject.c b/Objects/codeobject.c
index b0e3446..3ad0e99 100644
--- a/Objects/codeobject.c
+++ b/Objects/codeobject.c
@@ -409,11 +409,135 @@ code_repr(PyCodeObject *co)
     }
 }
 
+PyObject*
+_PyCode_ConstantKey(PyObject *op)
+{
+    PyObject *key;
+
+    /* Py_None and Py_Ellipsis are singleton */
+    if (op == Py_None || op == Py_Ellipsis
+       || PyLong_CheckExact(op)
+       || PyBool_Check(op)
+       || PyBytes_CheckExact(op)
+       || PyUnicode_CheckExact(op)
+          /* code_richcompare() uses _PyCode_ConstantKey() internally */
+       || PyCode_Check(op)) {
+        key = PyTuple_Pack(2, Py_TYPE(op), op);
+    }
+    else if (PyFloat_CheckExact(op)) {
+        double d = PyFloat_AS_DOUBLE(op);
+        /* all we need is to make the tuple different in either the 0.0
+         * or -0.0 case from all others, just to avoid the "coercion".
+         */
+        if (d == 0.0 && copysign(1.0, d) < 0.0)
+            key = PyTuple_Pack(3, Py_TYPE(op), op, Py_None);
+        else
+            key = PyTuple_Pack(2, Py_TYPE(op), op);
+    }
+    else if (PyComplex_CheckExact(op)) {
+        Py_complex z;
+        int real_negzero, imag_negzero;
+        /* For the complex case we must make complex(x, 0.)
+           different from complex(x, -0.) and complex(0., y)
+           different from complex(-0., y), for any x and y.
+           All four complex zeros must be distinguished.*/
+        z = PyComplex_AsCComplex(op);
+        real_negzero = z.real == 0.0 && copysign(1.0, z.real) < 0.0;
+        imag_negzero = z.imag == 0.0 && copysign(1.0, z.imag) < 0.0;
+        /* use True, False and None singleton as tags for the real and imag
+         * sign, to make tuples different */
+        if (real_negzero && imag_negzero) {
+            key = PyTuple_Pack(3, Py_TYPE(op), op, Py_True);
+        }
+        else if (imag_negzero) {
+            key = PyTuple_Pack(3, Py_TYPE(op), op, Py_False);
+        }
+        else if (real_negzero) {
+            key = PyTuple_Pack(3, Py_TYPE(op), op, Py_None);
+        }
+        else {
+            key = PyTuple_Pack(2, Py_TYPE(op), op);
+        }
+    }
+    else if (PyTuple_CheckExact(op)) {
+        Py_ssize_t i, len;
+        PyObject *tuple;
+
+        len = PyTuple_GET_SIZE(op);
+        tuple = PyTuple_New(len);
+        if (tuple == NULL)
+            return NULL;
+
+        for (i=0; i < len; i++) {
+            PyObject *item, *item_key;
+
+            item = PyTuple_GET_ITEM(op, i);
+            item_key = _PyCode_ConstantKey(item);
+            if (item_key == NULL) {
+                Py_DECREF(tuple);
+                return NULL;
+            }
+
+            PyTuple_SET_ITEM(tuple, i, item_key);
+        }
+
+        key = PyTuple_Pack(3, Py_TYPE(op), op, tuple);
+        Py_DECREF(tuple);
+    }
+    else if (PyFrozenSet_CheckExact(op)) {
+        Py_ssize_t pos = 0;
+        PyObject *item;
+        Py_hash_t hash;
+        Py_ssize_t i, len;
+        PyObject *tuple, *set;
+
+        len = PySet_GET_SIZE(op);
+        tuple = PyTuple_New(len);
+        if (tuple == NULL)
+            return NULL;
+
+        i = 0;
+        while (_PySet_NextEntry(op, &pos, &item, &hash)) {
+            PyObject *item_key;
+
+            item_key = _PyCode_ConstantKey(item);
+            if (item_key == NULL) {
+                Py_DECREF(tuple);
+                return NULL;
+            }
+
+            assert(i < len);
+            PyTuple_SET_ITEM(tuple, i, item_key);
+            i++;
+        }
+        set = PyFrozenSet_New(tuple);
+        Py_DECREF(tuple);
+        if (set == NULL)
+            return NULL;
+
+        key = PyTuple_Pack(3, Py_TYPE(op), op, set);
+        Py_DECREF(set);
+        return key;
+    }
+    else {
+        /* for other types, use the object identifier as an unique identifier
+         * to ensure that they are seen as unequal. */
+        PyObject *obj_id = PyLong_FromVoidPtr(op);
+        if (obj_id == NULL)
+            return NULL;
+
+        key = PyTuple_Pack(3, Py_TYPE(op), op, obj_id);
+        Py_DECREF(obj_id);
+    }
+    return key;
+}
+
 static PyObject *
 code_richcompare(PyObject *self, PyObject *other, int op)
 {
     PyCodeObject *co, *cp;
     int eq;
+    PyObject *consts1, *consts2;
     PyObject *res;
 
     if ((op != Py_EQ && op != Py_NE) ||
@@ -439,8 +563,21 @@ code_richcompare(PyObject *self, PyObject *other, int op)
     if (!eq) goto unequal;
     eq = PyObject_RichCompareBool(co->co_code, cp->co_code, Py_EQ);
     if (eq <= 0) goto unequal;
-    eq = PyObject_RichCompareBool(co->co_consts, cp->co_consts, Py_EQ);
+
+    /* compare constants */
+    consts1 = _PyCode_ConstantKey(co->co_consts);
+    if (!consts1)
+        return NULL;
+    consts2 = _PyCode_ConstantKey(cp->co_consts);
+    if (!consts2) {
+        Py_DECREF(consts1);
+        return NULL;
+    }
+    eq = PyObject_RichCompareBool(consts1, consts2, Py_EQ);
+    Py_DECREF(consts1);
+    Py_DECREF(consts2);
     if (eq <= 0) goto unequal;
+
     eq = PyObject_RichCompareBool(co->co_names, cp->co_names, Py_EQ);
     if (eq <= 0) goto unequal;
     eq = PyObject_RichCompareBool(co->co_varnames, cp->co_varnames, Py_EQ);
diff --git a/Python/compile.c b/Python/compile.c
index 7631f4e..2c4f7d7 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -393,7 +393,7 @@ list2dict(PyObject *list)
             return NULL;
         }
         k = PyList_GET_ITEM(list, i);
-        k = PyTuple_Pack(2, k, k->ob_type);
+        k = _PyCode_ConstantKey(k);
         if (k == NULL || PyDict_SetItem(dict, k, v) < 0) {
             Py_XDECREF(k);
             Py_DECREF(v);
@@ -456,7 +456,7 @@ dictbytype(PyObject *src, int scope_type, int flag, Py_ssize_t offset)
                 return NULL;
             }
             i++;
-            tuple = PyTuple_Pack(2, k, k->ob_type);
+            tuple = _PyCode_ConstantKey(k);
             if (!tuple || PyDict_SetItem(dest, tuple, item) < 0) {
                 Py_DECREF(sorted_keys);
                 Py_DECREF(item);
@@ -559,7 +559,7 @@ compiler_enter_scope(struct compiler *c, identifier name,
             compiler_unit_free(u);
             return 0;
         }
-        tuple = PyTuple_Pack(2, name, Py_TYPE(name));
+        tuple = _PyCode_ConstantKey(name);
         if (!tuple) {
             compiler_unit_free(u);
             return 0;
@@ -1100,47 +1100,8 @@ compiler_add_o(struct compiler *c, PyObject *dict, PyObject *o)
 {
     PyObject *t, *v;
     Py_ssize_t arg;
-    double d;
-
-    /* necessary to make sure types aren't coerced (e.g., float and complex) */
-    /* _and_ to distinguish 0.0 from -0.0 e.g. on IEEE platforms */
-    if (PyFloat_Check(o)) {
-        d = PyFloat_AS_DOUBLE(o);
-        /* all we need is to make the tuple different in either the 0.0
-         * or -0.0 case from all others, just to avoid the "coercion".
-         */
-        if (d == 0.0 && copysign(1.0, d) < 0.0)
-            t = PyTuple_Pack(3, o, o->ob_type, Py_None);
-        else
-            t = PyTuple_Pack(2, o, o->ob_type);
-    }
-    else if (PyComplex_Check(o)) {
-        Py_complex z;
-        int real_negzero, imag_negzero;
-        /* For the complex case we must make complex(x, 0.)
-           different from complex(x, -0.) and complex(0., y)
-           different from complex(-0., y), for any x and y.
-           All four complex zeros must be distinguished.*/
-        z = PyComplex_AsCComplex(o);
-        real_negzero = z.real == 0.0 && copysign(1.0, z.real) < 0.0;
-        imag_negzero = z.imag == 0.0 && copysign(1.0, z.imag) < 0.0;
-        if (real_negzero && imag_negzero) {
-            t = PyTuple_Pack(5, o, o->ob_type,
-                             Py_None, Py_None, Py_None);
-        }
-        else if (imag_negzero) {
-            t = PyTuple_Pack(4, o, o->ob_type, Py_None, Py_None);
-        }
-        else if (real_negzero) {
-            t = PyTuple_Pack(3, o, o->ob_type, Py_None);
-        }
-        else {
-            t = PyTuple_Pack(2, o, o->ob_type);
-        }
-    }
-    else {
-        t = PyTuple_Pack(2, o, o->ob_type);
-    }
+
+    t = _PyCode_ConstantKey(o);
     if (t == NULL)
         return -1;
 
@@ -1454,7 +1415,7 @@ static int
 compiler_lookup_arg(PyObject *dict, PyObject *name)
 {
     PyObject *k, *v;
-    k = PyTuple_Pack(2, name, name->ob_type);
+    k = _PyCode_ConstantKey(name);
     if (k == NULL)
         return -1;
     v = PyDict_GetItem(dict, k);
@@ -4562,9 +4523,10 @@ dict_keys_inorder(PyObject *dict, Py_ssize_t offset)
         return NULL;
     while (PyDict_Next(dict, &pos, &k, &v)) {
         i = PyLong_AS_LONG(v);
-        /* The keys of the dictionary are tuples. (see compiler_add_o)
-           The object we want is always first, though. */
-        k = PyTuple_GET_ITEM(k, 0);
+        /* The keys of the dictionary are tuples. (see compiler_add_o
+         * and _PyCode_ConstantKey). The object we want is always second,
+         * though. */
+        k = PyTuple_GET_ITEM(k, 1);
         Py_INCREF(k);
         assert((i - offset) < size);
         assert((i - offset) >= 0);