From 7791165fb36ecca2398ac81e9b6bc0248821262c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 22 Jan 2016 12:33:12 +0100 Subject: 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. --- Include/code.h | 11 +++- Lib/test/test_compile.py | 59 +++++++++++++++++++ Misc/NEWS | 6 ++ Objects/codeobject.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++- Python/compile.c | 53 ++--------------- 5 files changed, 224 insertions(+), 49 deletions(-) diff --git a/Include/code.h b/Include/code.h index 38b2958..7456fd6 100644 --- a/Include/code.h +++ b/Include/code.h @@ -70,7 +70,7 @@ PyAPI_DATA(PyTypeObject) PyCode_Type; /* Public interface */ PyAPI_FUNC(PyCodeObject *) PyCode_New( int, int, int, int, PyObject *, PyObject *, PyObject *, PyObject *, - PyObject *, PyObject *, PyObject *, PyObject *, int, PyObject *); + PyObject *, PyObject *, PyObject *, PyObject *, int, PyObject *); /* same as struct above */ /* Creates a new empty code object with the specified source location. */ @@ -98,6 +98,15 @@ typedef struct _addr_pair { 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); + PyAPI_FUNC(PyObject*) PyCode_Optimize(PyObject *code, PyObject* consts, PyObject *names, PyObject *lineno_obj); diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index c9f2835..e954a0c 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -623,6 +623,65 @@ class TestStackSize(unittest.TestCase): code += " x and x\n" * self.N self.check_stack_size(code) + 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. + @test_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') + + 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)) + def test_main(): test_support.run_unittest(__name__) diff --git a/Misc/NEWS b/Misc/NEWS index 995ad1b..549d22c 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,12 @@ What's New in Python 2.7.12? Core and Builtins ----------------- +- 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 #22995: [UPDATE] Remove the one of the pickleability tests in _PyObject_GetState() due to regressions observed in Cython-based projects. diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 9879df5..7e74607 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -376,11 +376,140 @@ code_compare(PyCodeObject *co, PyCodeObject *cp) return 0; } +PyObject* +_PyCode_ConstantKey(PyObject *op) +{ + PyObject *key; + + /* Py_None is a singleton */ + if (op == Py_None + || PyInt_CheckExact(op) + || PyLong_CheckExact(op) + || PyBool_Check(op) + || PyBytes_CheckExact(op) +#ifdef Py_USING_UNICODE + || PyUnicode_CheckExact(op) +#endif + /* 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); + } +#ifndef WITHOUT_COMPLEX + 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); + } + } +#endif + 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; + long 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) || @@ -413,8 +542,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 01b9fe0..bad1f50 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -338,7 +338,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); @@ -399,7 +399,7 @@ dictbytype(PyObject *src, int scope_type, int flag, int 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); @@ -944,49 +944,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., int and long) */ - /* _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); - } -#ifndef WITHOUT_COMPLEX - 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); - } - } -#endif /* WITHOUT_COMPLEX */ - else { - t = PyTuple_Pack(2, o, o->ob_type); - } + + t = _PyCode_ConstantKey(o); if (t == NULL) return -1; @@ -1287,7 +1246,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); @@ -3794,7 +3753,7 @@ dict_keys_inorder(PyObject *dict, int offset) i = PyInt_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); + k = PyTuple_GET_ITEM(k, 1); Py_INCREF(k); assert((i - offset) < size); assert((i - offset) >= 0); -- cgit v0.12