code_richcompare() don't use constant type when comparing code constants

[TijsVanOevelen]

BPO	25843
Nosy	@arigo, @rhettinger, @mdickinson, @ncoghlan, @vstinner, @larryhastings, @ezio-melotti, @bitdancer, @Bluehorn, @serhiy-storchaka, @Vgr255
Files	code_richcompare.patch code_richcompare-2.patch code_eq.py code_richcompare-3.patch code_richcompare-4.patch code_richcompare-5.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2016-01-22.15:54:05.277>
created_at = <Date 2015-12-11.19:23:13.886>
labels = ['type-bug']
title = "code_richcompare() don't use constant type when comparing code constants"
updated_at = <Date 2016-06-14.22:44:29.013>
user = 'https://bugs.python.org/TijsVanOevelen'

bugs.python.org fields:

activity = <Date 2016-06-14.22:44:29.013>
actor = 'larry'
assignee = 'none'
closed = True
closed_date = <Date 2016-01-22.15:54:05.277>
closer = 'vstinner'
components = []
creation = <Date 2015-12-11.19:23:13.886>
creator = 'Tijs Van Oevelen'
dependencies = []
files = ['41307', '41308', '41309', '41671', '41676', '41682']
hgrepos = []
issue_num = 25843
keywords = ['patch']
message_count = 53.0
messages = ['256229', '256230', '256233', '256234', '256235', '256245', '256250', '256251', '256252', '256274', '256285', '256290', '256317', '256318', '256320', '256321', '256323', '256327', '256330', '256332', '256381', '256382', '256383', '256386', '256390', '256398', '256399', '256400', '256401', '256402', '256403', '256404', '256405', '256406', '256409', '258532', '258683', '258707', '258724', '258725', '258753', '258756', '258795', '258796', '258814', '258815', '258817', '258818', '259016', '268555', '268556', '268557', '268588']
nosy_count = 17.0
nosy_names = ['arigo', 'rhettinger', 'mark.dickinson', 'ncoghlan', 'vstinner', 'larry', 'donmez', 'ezio.melotti', 'r.david.murray', 'torsten', 'fijall', 'python-dev', 'serhiy.storchaka', 'David MacIver', 'abarry', 'Kevin Shweh', 'Tijs Van Oevelen']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue25843'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

Linked PRs

• gh-70030: Remove _PyCode_ConstantKey() function #139735

[TijsVanOevelen]

The bug is described on Stack Overflow in length:

http://stackoverflow.com/questions/34100732/unexpected-output-using-pythons-ternary-operator-in-combination-with-lambda

[bitdancer]

For reference, the minimal reproducer is:

>>> f1, f2 = lambda: 1, lambda: 1.0
>>> f2()
1

The cause (according to the answer in the link) is that the two lambda's incorrectly compare as equal and end up sharing the same code object...one of the inequality checks is line number, so this only happens when the lambdas are on the same line.

Aside: it would be helpful if people would post bug summaries instead of just linking to an outside article. The link is of course very valuable information, but it saves us time if the poster summarizes the bug so the right people know to look at it.

[TijsVanOevelen]

Apologies for not posting a summary of the bug. I really had no idea how to describe the problem, as it is over my head. I could only refer to my question on Stack Overflow that triggered the discovery of the bug.

[TijsVanOevelen]

It's definitely also in 3.4 by the way.

[bitdancer]

Thanks for posting the bug. I added the aside because this is the third or fourth of these reference-only things I've seen in the past couple weeks and I finally figured out what bothered me about them. We should put something about this in the devguide on bug reporting, but of course most people reporting bugs will for good reason not read that. So, I just have to live with it :)

And yes, the reproducer reproduces the problem in all python versions currently under maintenance.

I've now changed the title to reflect the underlying bug.

[rhettinger]

The equality of code objects is determined by the code_richcompare() logic in Objects/codeobject.c.

Two code objects are equal if all of their attributes compare equal. That includes co_name, co_argcount, co_kwonlyargcount, co_nlocals, co_flags, co_firstlineno, co_code, co_consts, co_names, co_varnames, co_freevars, and co_cellvars.

At the heart of David Murray's minimal example, the reason the two distinct code objects compare equal is that their co_consts compare as equal.

If you wanted to fix this, code objects would need to recursively check for both normal equality and type equality.

>>> f1 = lambda: 1
>>> f2 = lambda: 1.0
>>> f1.__code__.co_consts == f2.__code__.co_consts
True
>>> map(type, f1.__code__.co_consts) == map(type, f2.__code__.co_consts)
False

[KevinShweh]

A type-based check runs into problems with 0.0 vs -0.0. For example, on Python 2.7.11:

>>> x, y = lambda: 0.0, lambda: -0.0
>>> y()
0.0

I wasn't able to reproduce the -0.0 problem with Python 3.4 on Ideone; y.__code__.co_consts seems to have an unused 0.0 in it on 3.4. I don't have access to Python 3.5, so I don't know what the situation is like on that version.

[rhettinger]

Here's another variant (courtesy of Nick Coghlan):

python3.5 -c "seq1 = [1.0 for x in range(5)]; seq2 = [True for x in range(5)]; print(seq1); print(seq2)"

[rhettinger]

One possible solution for all these variants is to let code objects track both the co.firstlineno and co.firstrowno.

[rhettinger]

FWIW, the bug is present in PyPy as well.

[ncoghlan]

From what I can see:

• checking constant types in addition to their values should be a two line change (plus tests)
• checking the column number in addition to the line number would be a more comprehensive fix, but also a constructor API change (since PyCode_New doesn't currently accept a column parameter)

The "values of the same constant type that are distinct but equivalent may still compare equal" case is obscure enough that I think the lower impact change is likely a better option, especially as 3.x currently "handles" the "lambda: -0.0" case by having both the unfolded 0.0 and the folded -0.0 in the constant list.

------------
Additional detail for those interested:

The lowest impact fix from a code change would be to add a type equivalence check for constants as Raymond first suggested, as that only involves adding an extra check to code_richcompare: https://hg.python.org/cpython/file/tip/Objects/codeobject.c#l416

However, the idea of tracking "co_firstcolno" in addition to "co_firstlineno" is a more robust fix, as it means any lexically distinct code objects will necessarily be considered distinct by the interpreter. The downside is that it means touching more code and adding a new public API, since PyCode_New doesn't currently accept a "firstcolno" parameter - code objects are implicitly assumed to be one-per-line.

Out of curiosity, I also went looking for the code in the code generator that collapses the "equivalent" code objects together. The issue is that constants are stored in a dict (mapping them to their co_consts index) during compilation and assembly, so equivalent objects will be merged together (and refer to the index of the first one defined).

[rhettinger]

Interestingly, co_filename is not used as part of the equivalence criteria, so code object equivalence can be fooled across multiple input files. Fortunately in this case, the false equivalence isn't relied on by the code generator.

  $ cat a.py
  def f():
      return 1

  $ cat b.py
  def f():
      return 1.0

  $ ./python.exe -q
  >>> import a, b
  >>> a.f.__code__ == b.f.__code__  # False equivalence
  True
  >>> a.f()
  1
  >>> b.f()                         # Generated code is correct
  1.0

Besides aliasing int/float/decimal/fraction/complex/bool, codeobj.__eq__() can also alias str/unicode on Python 2.7. Likewise, 0.0 and -0.0 can be conflated. NaNs don't seem to be a problem.

I think we should focus on fixing the spec for code object equivalents. Perhaps the test can be simplified to use (co_firstlineno, co_firstrowno, co_filename).