Skip to content

Commit 3cdd5fb

Browse files
vstinnervstinner
committed
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.
1 parent d52513c commit 3cdd5fb

File tree

5 files changed

+246
-50
lines changed

5 files changed

+246
-50
lines changed

‎Include/code.h‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,21 @@ typedef struct _addr_pair {
108108
int ap_upper;
109109
} PyAddrPair;
110110

111+
#ifndef Py_LIMITED_API
111112
/* Update *bounds to describe the first and one-past-the-last instructions in the
112113
same line as lasti. Return the number of that line.
113114
*/
114-
#ifndef Py_LIMITED_API
115115
PyAPI_FUNC(int) _PyCode_CheckLineNumber(PyCodeObject* co,
116116
int lasti, PyAddrPair *bounds);
117+
118+
/* Create a comparable key used to compare constants taking in account the
119+
* object type. It is used to make sure types are not coerced (e.g., float and
120+
* complex) _and_ to distinguish 0.0 from -0.0 e.g. on IEEE platforms
121+
*
122+
* Return (type(obj), obj, ...): a tuple with variable size (at least 2 items)
123+
* depending on the type and the value. The type is the first item to not
124+
* compare bytes and str which can raise a BytesWarning exception. */
125+
PyAPI_FUNC(PyObject*) _PyCode_ConstantKey(PyObject *obj);
117126
#endif
118127

119128
PyAPI_FUNC(PyObject*) PyCode_Optimize(PyObject *code, PyObject* consts,

‎Lib/test/test_compile.py‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,88 @@ def test_null_terminated(self):
572572
exec(memoryview(b"ax = 123")[1:-1], namespace)
573573
self.assertEqual(namespace['x'], 12)
574574

575+
def check_constant(self, func, expected):
576+
for const in func.__code__.co_consts:
577+
if repr(const) == repr(expected):
578+
break
579+
else:
580+
self.fail("unable to find constant %r in %r"
581+
% (expected, func.__code__.co_consts))
582+
583+
# Merging equal constants is not a strict requirement for the Python
584+
# semantics, it's a more an implementation detail.
585+
@support.cpython_only
586+
def test_merge_constants(self):
587+
# Issue #25843: compile() must merge constants which are equal
588+
# and have the same type.
589+
590+
def check_same_constant(const):
591+
ns = {}
592+
code = "f1, f2 = lambda: %r, lambda: %r" % (const, const)
593+
exec(code, ns)
594+
f1 = ns['f1']
595+
f2 = ns['f2']
596+
self.assertIs(f1.__code__, f2.__code__)
597+
self.check_constant(f1, const)
598+
self.assertEqual(repr(f1()), repr(const))
599+
600+
check_same_constant(None)
601+
check_same_constant(0)
602+
check_same_constant(0.0)
603+
check_same_constant(b'abc')
604+
check_same_constant('abc')
605+
606+
# Note: "lambda: ..." emits "LOAD_CONST Ellipsis",
607+
# whereas "lambda: Ellipsis" emits "LOAD_GLOBAL Ellipsis"
608+
f1, f2 = lambda: ..., lambda: ...
609+
self.assertIs(f1.__code__, f2.__code__)
610+
self.check_constant(f1, Ellipsis)
611+
self.assertEqual(repr(f1()), repr(Ellipsis))
612+
613+
# {0} is converted to a constant frozenset({0}) by the peephole
614+
# optimizer
615+
f1, f2 = lambda x: x in {0}, lambda x: x in {0}
616+
self.assertIs(f1.__code__, f2.__code__)
617+
self.check_constant(f1, frozenset({0}))
618+
self.assertTrue(f1(0))
619+
620+
def test_dont_merge_constants(self):
621+
# Issue #25843: compile() must not merge constants which are equal
622+
# but have a different type.
623+
624+
def check_different_constants(const1, const2):
625+
ns = {}
626+
exec("f1, f2 = lambda: %r, lambda: %r" % (const1, const2), ns)
627+
f1 = ns['f1']
628+
f2 = ns['f2']
629+
self.assertIsNot(f1.__code__, f2.__code__)
630+
self.check_constant(f1, const1)
631+
self.check_constant(f2, const2)
632+
self.assertEqual(repr(f1()), repr(const1))
633+
self.assertEqual(repr(f2()), repr(const2))
634+
635+
check_different_constants(0, 0.0)
636+
check_different_constants(+0.0, -0.0)
637+
check_different_constants((0,), (0.0,))
638+
639+
# check_different_constants() cannot be used because repr(-0j) is
640+
# '(-0-0j)', but when '(-0-0j)' is evaluated to 0j: we loose the sign.
641+
f1, f2 = lambda: +0.0j, lambda: -0.0j
642+
self.assertIsNot(f1.__code__, f2.__code__)
643+
self.check_constant(f1, +0.0j)
644+
self.check_constant(f2, -0.0j)
645+
self.assertEqual(repr(f1()), repr(+0.0j))
646+
self.assertEqual(repr(f2()), repr(-0.0j))
647+
648+
# {0} is converted to a constant frozenset({0}) by the peephole
649+
# optimizer
650+
f1, f2 = lambda x: x in {0}, lambda x: x in {0.0}
651+
self.assertIsNot(f1.__code__, f2.__code__)
652+
self.check_constant(f1, frozenset({0}))
653+
self.check_constant(f2, frozenset({0.0}))
654+
self.assertTrue(f1(0))
655+
self.assertTrue(f2(0.0))
656+
575657

576658
class TestStackSize(unittest.TestCase):
577659
# These tests check that the computed stack size for a code object

‎Misc/NEWS‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ Core and Builtins
1717
Python 3.5.1 to hide the exact implementation of atomic C types, to avoid
1818
compiler issues.
1919

20+
- Issue #25843: When compiling code, don't merge constants if they are equal
21+
but have a different types. For example, ``f1, f2 = lambda: 1, lambda: 1.0``
22+
is now correctly compiled to two different functions: ``f1()`` returns ``1``
23+
(``int``) and ``f2()`` returns ``1.0`` (``int``), even if ``1`` and ``1.0``
24+
are equal.
25+
2026
- Issue #25731: Fix set and deleting __new__ on a class.
2127

2228
- Issue #22995: [UPDATE] Comment out the one of the pickleability tests in

‎Objects/codeobject.c‎

Lines changed: 138 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,11 +409,135 @@ code_repr(PyCodeObject *co)
409409
}
410410
}
411411

412+
PyObject*
413+
_PyCode_ConstantKey(PyObject *op)
414+
{
415+
PyObject *key;
416+
417+
/* Py_None and Py_Ellipsis are singleton */
418+
if (op == Py_None || op == Py_Ellipsis
419+
|| PyLong_CheckExact(op)
420+
|| PyBool_Check(op)
421+
|| PyBytes_CheckExact(op)
422+
|| PyUnicode_CheckExact(op)
423+
/* code_richcompare() uses _PyCode_ConstantKey() internally */
424+
|| PyCode_Check(op)) {
425+
key = PyTuple_Pack(2, Py_TYPE(op), op);
426+
}
427+
else if (PyFloat_CheckExact(op)) {
428+
double d = PyFloat_AS_DOUBLE(op);
429+
/* all we need is to make the tuple different in either the 0.0
430+
* or -0.0 case from all others, just to avoid the "coercion".
431+
*/
432+
if (d == 0.0 && copysign(1.0, d) < 0.0)
433+
key = PyTuple_Pack(3, Py_TYPE(op), op, Py_None);
434+
else
435+
key = PyTuple_Pack(2, Py_TYPE(op), op);
436+
}
437+
else if (PyComplex_CheckExact(op)) {
438+
Py_complex z;
439+
int real_negzero, imag_negzero;
440+
/* For the complex case we must make complex(x, 0.)
441+
different from complex(x, -0.) and complex(0., y)
442+
different from complex(-0., y), for any x and y.
443+
All four complex zeros must be distinguished.*/
444+
z = PyComplex_AsCComplex(op);
445+
real_negzero = z.real == 0.0 && copysign(1.0, z.real) < 0.0;
446+
imag_negzero = z.imag == 0.0 && copysign(1.0, z.imag) < 0.0;
447+
/* use True, False and None singleton as tags for the real and imag
448+
* sign, to make tuples different */
449+
if (real_negzero && imag_negzero) {
450+
key = PyTuple_Pack(3, Py_TYPE(op), op, Py_True);
451+
}
452+
else if (imag_negzero) {
453+
key = PyTuple_Pack(3, Py_TYPE(op), op, Py_False);
454+
}
455+
else if (real_negzero) {
456+
key = PyTuple_Pack(3, Py_TYPE(op), op, Py_None);
457+
}
458+
else {
459+
key = PyTuple_Pack(2, Py_TYPE(op), op);
460+
}
461+
}
462+
else if (PyTuple_CheckExact(op)) {
463+
Py_ssize_t i, len;
464+
PyObject *tuple;
465+
466+
len = PyTuple_GET_SIZE(op);
467+
tuple = PyTuple_New(len);
468+
if (tuple == NULL)
469+
return NULL;
470+
471+
for (i=0; i < len; i++) {
472+
PyObject *item, *item_key;
473+
474+
item = PyTuple_GET_ITEM(op, i);
475+
item_key = _PyCode_ConstantKey(item);
476+
if (item_key == NULL) {
477+
Py_DECREF(tuple);
478+
return NULL;
479+
}
480+
481+
PyTuple_SET_ITEM(tuple, i, item_key);
482+
}
483+
484+
key = PyTuple_Pack(3, Py_TYPE(op), op, tuple);
485+
Py_DECREF(tuple);
486+
}
487+
else if (PyFrozenSet_CheckExact(op)) {
488+
Py_ssize_t pos = 0;
489+
PyObject *item;
490+
Py_hash_t hash;
491+
Py_ssize_t i, len;
492+
PyObject *tuple, *set;
493+
494+
len = PySet_GET_SIZE(op);
495+
tuple = PyTuple_New(len);
496+
if (tuple == NULL)
497+
return NULL;
498+
499+
i = 0;
500+
while (_PySet_NextEntry(op, &pos, &item, &hash)) {
501+
PyObject *item_key;
502+
503+
item_key = _PyCode_ConstantKey(item);
504+
if (item_key == NULL) {
505+
Py_DECREF(tuple);
506+
return NULL;
507+
}
508+
509+
assert(i < len);
510+
PyTuple_SET_ITEM(tuple, i, item_key);
511+
i++;
512+
}
513+
set = PyFrozenSet_New(tuple);
514+
Py_DECREF(tuple);
515+
if (set == NULL)
516+
return NULL;
517+
518+
key = PyTuple_Pack(3, Py_TYPE(op), op, set);
519+
Py_DECREF(set);
520+
return key;
521+
}
522+
else {
523+
/* for other types, use the object identifier as an unique identifier
524+
* to ensure that they are seen as unequal. */
525+
PyObject *obj_id = PyLong_FromVoidPtr(op);
526+
if (obj_id == NULL)
527+
return NULL;
528+
529+
key = PyTuple_Pack(3, Py_TYPE(op), op, obj_id);
530+
Py_DECREF(obj_id);
531+
}
532+
return key;
533+
}
534+
412535
static PyObject *
413536
code_richcompare(PyObject *self, PyObject *other, int op)
414537
{
415538
PyCodeObject *co, *cp;
416539
int eq;
540+
PyObject *consts1, *consts2;
417541
PyObject *res;
418542

419543
if ((op != Py_EQ && op != Py_NE) ||
@@ -439,8 +563,21 @@ code_richcompare(PyObject *self, PyObject *other, int op)
439563
if (!eq) goto unequal;
440564
eq = PyObject_RichCompareBool(co->co_code, cp->co_code, Py_EQ);
441565
if (eq <= 0) goto unequal;
442-
eq = PyObject_RichCompareBool(co->co_consts, cp->co_consts, Py_EQ);
566+
567+
/* compare constants */
568+
consts1 = _PyCode_ConstantKey(co->co_consts);
569+
if (!consts1)
570+
return NULL;
571+
consts2 = _PyCode_ConstantKey(cp->co_consts);
572+
if (!consts2) {
573+
Py_DECREF(consts1);
574+
return NULL;
575+
}
576+
eq = PyObject_RichCompareBool(consts1, consts2, Py_EQ);
577+
Py_DECREF(consts1);
578+
Py_DECREF(consts2);
443579
if (eq <= 0) goto unequal;
580+
444581
eq = PyObject_RichCompareBool(co->co_names, cp->co_names, Py_EQ);
445582
if (eq <= 0) goto unequal;
446583
eq = PyObject_RichCompareBool(co->co_varnames, cp->co_varnames, Py_EQ);

‎Python/compile.c‎

Lines changed: 10 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ list2dict(PyObject *list)
393393
return NULL;
394394
}
395395
k = PyList_GET_ITEM(list, i);
396-
k = PyTuple_Pack(2, k, k->ob_type);
396+
k = _PyCode_ConstantKey(k);
397397
if (k == NULL || PyDict_SetItem(dict, k, v) < 0) {
398398
Py_XDECREF(k);
399399
Py_DECREF(v);
@@ -456,7 +456,7 @@ dictbytype(PyObject *src, int scope_type, int flag, Py_ssize_t offset)
456456
return NULL;
457457
}
458458
i++;
459-
tuple = PyTuple_Pack(2, k, k->ob_type);
459+
tuple = _PyCode_ConstantKey(k);
460460
if (!tuple || PyDict_SetItem(dest, tuple, item) < 0) {
461461
Py_DECREF(sorted_keys);
462462
Py_DECREF(item);
@@ -559,7 +559,7 @@ compiler_enter_scope(struct compiler *c, identifier name,
559559
compiler_unit_free(u);
560560
return 0;
561561
}
562-
tuple = PyTuple_Pack(2, name, Py_TYPE(name));
562+
tuple = _PyCode_ConstantKey(name);
563563
if (!tuple) {
564564
compiler_unit_free(u);
565565
return 0;
@@ -1100,47 +1100,8 @@ compiler_add_o(struct compiler *c, PyObject *dict, PyObject *o)
11001100
{
11011101
PyObject *t, *v;
11021102
Py_ssize_t arg;
1103-
double d;
1104-
1105-
/* necessary to make sure types aren't coerced (e.g., float and complex) */
1106-
/* _and_ to distinguish 0.0 from -0.0 e.g. on IEEE platforms */
1107-
if (PyFloat_Check(o)) {
1108-
d = PyFloat_AS_DOUBLE(o);
1109-
/* all we need is to make the tuple different in either the 0.0
1110-
* or -0.0 case from all others, just to avoid the "coercion".
1111-
*/
1112-
if (d == 0.0 && copysign(1.0, d) < 0.0)
1113-
t = PyTuple_Pack(3, o, o->ob_type, Py_None);
1114-
else
1115-
t = PyTuple_Pack(2, o, o->ob_type);
1116-
}
1117-
else if (PyComplex_Check(o)) {
1118-
Py_complex z;
1119-
int real_negzero, imag_negzero;
1120-
/* For the complex case we must make complex(x, 0.)
1121-
different from complex(x, -0.) and complex(0., y)
1122-
different from complex(-0., y), for any x and y.
1123-
All four complex zeros must be distinguished.*/
1124-
z = PyComplex_AsCComplex(o);
1125-
real_negzero = z.real == 0.0 && copysign(1.0, z.real) < 0.0;
1126-
imag_negzero = z.imag == 0.0 && copysign(1.0, z.imag) < 0.0;
1127-
if (real_negzero && imag_negzero) {
1128-
t = PyTuple_Pack(5, o, o->ob_type,
1129-
Py_None, Py_None, Py_None);
1130-
}
1131-
else if (imag_negzero) {
1132-
t = PyTuple_Pack(4, o, o->ob_type, Py_None, Py_None);
1133-
}
1134-
else if (real_negzero) {
1135-
t = PyTuple_Pack(3, o, o->ob_type, Py_None);
1136-
}
1137-
else {
1138-
t = PyTuple_Pack(2, o, o->ob_type);
1139-
}
1140-
}
1141-
else {
1142-
t = PyTuple_Pack(2, o, o->ob_type);
1143-
}
1103+
1104+
t = _PyCode_ConstantKey(o);
11441105
if (t == NULL)
11451106
return -1;
11461107

@@ -1454,7 +1415,7 @@ static int
14541415
compiler_lookup_arg(PyObject *dict, PyObject *name)
14551416
{
14561417
PyObject *k, *v;
1457-
k = PyTuple_Pack(2, name, name->ob_type);
1418+
k = _PyCode_ConstantKey(name);
14581419
if (k == NULL)
14591420
return -1;
14601421
v = PyDict_GetItem(dict, k);
@@ -4562,9 +4523,10 @@ dict_keys_inorder(PyObject *dict, Py_ssize_t offset)
45624523
return NULL;
45634524
while (PyDict_Next(dict, &pos, &k, &v)) {
45644525
i = PyLong_AS_LONG(v);
4565-
/* The keys of the dictionary are tuples. (see compiler_add_o)
4566-
The object we want is always first, though. */
4567-
k = PyTuple_GET_ITEM(k, 0);
4526+
/* The keys of the dictionary are tuples. (see compiler_add_o
4527+
* and _PyCode_ConstantKey). The object we want is always second,
4528+
* though. */
4529+
k = PyTuple_GET_ITEM(k, 1);
45684530
Py_INCREF(k);
45694531
assert((i - offset) < size);
45704532
assert((i - offset) >= 0);

0 commit comments

Comments
 (0)