[Python-checkins] cpython: code_richcompare() now uses the constants types
victor.stinner
python-checkins at python.org
Fri Jan 22 06:37:47 EST 2016
https://hg.python.org/cpython/rev/6c33d4cc9b8f
changeset: 100039:6c33d4cc9b8f
user: Victor Stinner <victor.stinner at gmail.com>
date: Fri Jan 22 12:33:12 2016 +0100
summary:
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.
files:
Include/code.h | 11 +-
Lib/test/test_compile.py | 82 +++++++++++++++
Misc/NEWS | 6 +
Objects/codeobject.c | 139 ++++++++++++++++++++++++++-
Python/compile.c | 58 +---------
5 files changed, 246 insertions(+), 50 deletions(-)
diff --git a/Include/code.h b/Include/code.h
--- a/Include/code.h
+++ b/Include/code.h
@@ -108,12 +108,21 @@
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
--- a/Lib/test/test_compile.py
+++ b/Lib/test/test_compile.py
@@ -572,6 +572,88 @@
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
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,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 #26107: The format of the ``co_lnotab`` attribute of code objects
changes to support negative line number delta.
diff --git a/Objects/codeobject.c b/Objects/codeobject.c
--- a/Objects/codeobject.c
+++ b/Objects/codeobject.c
@@ -409,11 +409,135 @@
}
}
+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 @@
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
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -393,7 +393,7 @@
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 @@
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_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;
@@ -1105,47 +1105,8 @@
{
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;
@@ -1459,7 +1420,7 @@
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);
@@ -4657,9 +4618,10 @@
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);
--
Repository URL: https://hg.python.org/cpython
More information about the Python-checkins
mailing list