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.
This commit is contained in:
Victor Stinner 2016-01-22 12:33:12 +01:00
parent e3560a7dc9
commit efb2413ce8
5 changed files with 245 additions and 49 deletions

View File

@ -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,

View File

@ -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

View File

@ -10,6 +10,12 @@ Release date: tba
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.

View File

@ -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);

View File

@ -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;
@ -1105,47 +1105,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;
@ -1459,7 +1420,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);
@ -4657,9 +4618,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);