Issue 11205: Evaluation order of dictionary display is different from reference manual. (original) (raw)

process

Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: benjamin.peterson, docs@python, eric.araujo, eric.snow, ezio.melotti, freakboy, freakboy3742, georg.brandl, gvanrossum, ncoghlan, nedbat, python-dev, r.david.murray, rhettinger, sandro.tosi, sdougherty, swamiyeswanth, takayuki, terry.reedy
Priority: normal Keywords: patch

Created on 2011-02-13 09:39 by takayuki, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
compile.diff skip.montanaro,2011-02-13 13:32 review
issue11205.patch sdougherty,2015-04-27 20:53 review
issue11205-v2.patch sdougherty,2015-04-28 14:02 review
issue11205-v3.patch sdougherty,2015-04-28 19:08 review
Messages (33)
msg128482 - (view) Author: (takayuki) Date: 2011-02-13 09:39
Running the following code shows "2 1 4 3", but in reference manual http://docs.python.org/reference/expressions.html#expression-lists the evaluation order described as {expr1: expr2, expr3: expr4} def f(i): print i return i {f(1):f(2), f(3):f(4)} I found some of past discussions about this problem, for example, http://mail.python.org/pipermail/python-dev/2002-November/030461.html http://mail.python.org/pipermail/python-dev/2002-November/030458.html http://bugs.python.org/issue448679 http://svn.python.org/view?view=rev&revision=30148 , but current implementation seems not to reflect these accomplishment. In present, which behaviour is legal?
msg128487 - (view) Author: Skip Montanaro (skip.montanaro) * (Python triager) Date: 2011-02-13 13:04
Here's a patch.
msg128489 - (view) Author: Skip Montanaro (skip.montanaro) * (Python triager) Date: 2011-02-13 13:08
Working on a test case too.
msg128490 - (view) Author: Skip Montanaro (skip.montanaro) * (Python triager) Date: 2011-02-13 13:14
It's not so easy as first appeared.
msg128492 - (view) Author: Skip Montanaro (skip.montanaro) * (Python triager) Date: 2011-02-13 13:32
Okay, this looks better. I was confusing myself with leftover .pyc files I think. Test included.
msg128493 - (view) Author: Skip Montanaro (skip.montanaro) * (Python triager) Date: 2011-02-13 13:37
Georg, I think this might need to sneak into 3.2.
msg128494 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-02-13 14:13
I don't think so -- it's a very minor deviation from the spec and not a critical bug.
msg128498 - (view) Author: Skip Montanaro (skip.montanaro) * (Python triager) Date: 2011-02-13 14:26
3.2 and earlier versions are all frozen, but for 3.3 it might make sense to bump the version number of the bytecode and change STORE_MAP to take the key and value in the opposite order, thus allowing to remove the ROT_TWO I had to add to make this work.
msg128500 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-13 14:35
Interesting. I would actually have expected the observed behavior. I think of the : in a dictionary literal as an assignment.
msg128502 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-02-13 14:56
BTW, it would be nice to know if this behavior was consistent with the docs at any time (the merge of the AST branch in 2.5 might be an obvious candidate where it was broken). Also interesting would be what other implementations of Python do.
msg128507 - (view) Author: Skip Montanaro (skip.montanaro) * (Python triager) Date: 2011-02-13 15:17
As Georg suggested, it is correct in 2.4, wrong in 2.5.
msg128520 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2011-02-13 21:51
If only a test had been checked-in eight years ago ... It looks like a number of things have to be changed in order to bring behavior back to what the docs promise. Dict literals and dict comprehensions need to be fixed in both the compile.py and compile.c. To avoid introducing a ROT_TWO, the store STORE_MAP and MAP_ADD opcodes need minor modifications (just switch the u and w variable assignments).
msg128521 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2011-02-13 21:52
takayuki, a special thanks to you for submitting such a well-researched bug report :-)
msg128522 - (view) Author: Skip Montanaro (skip.montanaro) * (Python triager) Date: 2011-02-13 21:54
> To avoid introducing a ROT_TWO, the store STORE_MAP and MAP_ADD > opcodes need minor modifications (just switch the u and w variable > assignments). Either of which would not be possible in anything other that 3.3 or later, right?
msg128530 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2011-02-13 23:41
How much to change and how far to backport may make for a good python-dev discussion. ISTM that changing generated code goes hand-in-hand with changing opcode semantics (as long as the magic number gets bumped). OTOH, none of this may be worth backporting at all since no one seems to have noticed or cared for eight years. I don't have any feelings about it one way or the other, but it would great to see Py3.2.1 get fixed properly.
msg138223 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-06-12 22:35
If I understand, since the present patch uses present opcode semantics, adding a rotate, it could go into 2.7 and 3.2. Correct? 3.3 could get an improved patch that instead changed opcodes to expect key and value in the other order.
msg175120 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-11-07 19:18
Today in pydev thread "chained assignment weirdity" Guido said http://permalink.gmane.org/gmane.comp.python.devel/135746 "I agree that we should be *very* conservative in changing the meaning of existing opcodes (adding new one is a different story)." ... "Hm. I really don't think that is a good development for Python to compromise in the area of expression evaluation order where side effects are involved." ... "I haven't looked at the proposed fixes, but I think correctness is more important than saving an extra bytecode (OTOH keeping the set of opcodes the same trumps both). I can't imagine that this extra opcode will be significant in many cases." To which Nick C. replied "Since you've indicated the implementation is in the wrong here and you also want to preserve opcode semantics, I think Skip's patch is correct, but also needs to be applied to dict comprehensions (now we have them). The extra bytecode is only ROT_TWO, which is one of the cheapest we have kicking around." To which Guido said "Ok, somebody go for it! (Also please refer to my pronouncement in the bug"
msg178133 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-12-25 14:58
I came across the same problem in #16777. IMHO the current behavior is better, and the documentation should be fixed instead, for the following reasons: 1) it's consistent with assignments, where the RHS is evaluated before the LHS (see also ). This should also be the behavior of the dict(k=v) syntax, and what I personally expect; 2) changing it back is not backward-compatible with any code written during the last 10 years; 3) keeping the current behavior and fixing the docs is simpler than fixing the code to match the docs; In addition, I would avoid writing code with side-effects in a dict literal, even if the order was documented and guaranteed. The fact that we don't see many reports about this seems to indicate that people don't write such code, or if they do they rely on the current order.
msg179585 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-01-10 19:21
I am sticking with my opinion from before: the code should be fixed. It doesn't look like assignment to me. I am fine with making this a "feature" only fixed in 3.4. (You can even fix the docs in 3.3 as long as you fix them back for 3.4.) Minor note for Ezio: dict(k=v, ...) doesn't have this problem because k is just a keyword, not an expression.
msg185438 - (view) Author: Ned Batchelder (nedbat) * (Python triager) Date: 2013-03-28 12:09
Since this is documented in the Python Language Reference, it doesn't make much sense to have it describe one way for 3.3 and another for 3.4, does it? By definition, doesn't that make this an implementation dependency? We should update the docs to say that pairs in dicts will be evaluated left-to-right, but that the order of key and value is implementation dependent, since it actually is.
msg185446 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-03-28 13:23
I strongly agree with Guido that we should fix the code. To me, defined left-to-right evaluation of sub-expressions within an expression is a feature. C's 'implementation defined' is a nuisance. I do not think we should temporarily change the doc. In other cases where we have only applied a bugfix to a future version, we have not temporarily doc'ed the bug as 'correct'. The change *should* be in What's New.
msg240222 - (view) Author: Steve Dougherty (sdougherty) * Date: 2015-04-07 18:21
Any word on either changing the documentation to match the behaviour or fixing this as a bug?
msg240225 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2015-04-07 19:33
This needs a code patch. But it can only be fixed for 3.5. On Apr 7, 2015 11:21 AM, "Steve Dougherty" <report@bugs.python.org> wrote: > > Steve Dougherty added the comment: > > Any word on either changing the documentation to match the behaviour or > fixing this as a bug? > > ---------- > nosy: +sdougherty > > _______________________________________ > Python tracker <report@bugs.python.org> > <http://bugs.python.org/issue11205> > _______________________________________ >
msg242157 - (view) Author: Steve Dougherty (sdougherty) * Date: 2015-04-27 20:53
I've added a patch to change the order of evaluation and of STORE_MAP's arguments. It includes a test incorporating the review on the previous version. I noticed I had to remove existing .pyc files to avoid TypeErrors about values being unhashable. I take it the version numbers in the filenames are sufficient to prevent that problem in a release?
msg242161 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2015-04-27 23:37
The pyc issue suggests the magic number embedded in pyc files to indicate bytecode compatibility needs to be incremented. If I recall correctly, that lives in Lib/importlib/_bootstrap.py these days.
msg242183 - (view) Author: Steve Dougherty (sdougherty) * Date: 2015-04-28 14:02
I've added a change to the bytecode magic number as well. I don't see a magic tag anymore, either in the code or for recent versions. There are autogenerated changes to Python/importlibs.h. Is my understanding correct that these are not to be committed?
msg242200 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-04-28 18:45
Changes to importlib.h should always be committed. It is the frozen importlib._bootstrap module, which is the implementation of the import system used by the interpreter. So failure to commit changes to importlib.h means your changes to importlib._bootstrap will not have any effect.
msg242204 - (view) Author: Steve Dougherty (sdougherty) * Date: 2015-04-28 19:08
I've added the importlib.h changes and changed the name of the test to be more descriptive.
msg244336 - (view) Author: Steve Dougherty (sdougherty) * Date: 2015-05-28 18:37
Anyone care to review -v3.patch ?
msg244339 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2015-05-28 18:58
I downloaded and tried to apply to 3.5 (then default) as it was last Saturday, before the .b1 cutoff. Only ceval.py and test_compile.py patches worked. Everything else failed. You need to update your repository (3.5 is now a branch and default is 3.6) and then the patch.
msg244340 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-05-28 19:31
New changeset 6f05f83c7010 by Benjamin Peterson in branch '3.5': in dict displays, evaluate the key before the value (closes #11205) https://hg.python.org/cpython/rev/6f05f83c7010 New changeset ba9e4df5368c by Benjamin Peterson in branch 'default': merge 3.5 (#11205) https://hg.python.org/cpython/rev/ba9e4df5368c
msg263219 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2016-04-12 01:55
Temporarily reopening this as a docs bug - I think it's worth mentioning in the "Porting to Python 3.5" section of the What's New docs and as a "version changed" note in the dis module docs, as even though it's obscure, anyone that was inadvertently relying on the prior deviation from the spec is going to be confused by the behavioural change in 3.5. (The specific case where this came up was Russell Keith-Magee encountering the semantic change in BUILD_MAP's expectations for argument order on the stack for his VOD bytecode transpiler)
msg303791 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2017-10-06 02:29
I'm unlikely to ever get to this (especially as 3.5 is now in security-fix only mode), so closing it again.
History
Date User Action Args
2022-04-11 14:57:12 admin set github: 55414
2017-10-06 02:29:41 ncoghlan set status: open -> closednosy: + freakboy, freakboy3742messages: + resolution: fixedstage: needs patch -> resolved
2016-04-12 01:55:43 ncoghlan set status: closed -> openassignee: docs@pythonstage: resolved -> needs patchversions: + Python 3.5, - Python 2.7, Python 3.3, Python 3.4nosy: + docs@pythonmessages: + priority: high -> normalcomponents: + Documentation, - Interpreter Coreresolution: fixed -> (no value)
2015-05-28 19:31:27 python-dev set status: open -> closednosy: + python-devmessages: + resolution: fixedstage: patch review -> resolved
2015-05-28 18:58:40 terry.reedy set messages: +
2015-05-28 18:37:50 sdougherty set messages: +
2015-04-28 19:08:11 sdougherty set files: + issue11205-v3.patchmessages: +
2015-04-28 18:45:23 eric.snow set nosy: + eric.snowmessages: +
2015-04-28 14:02:38 sdougherty set files: + issue11205-v2.patchmessages: +
2015-04-27 23:37:09 ncoghlan set messages: +
2015-04-27 20:53:51 sdougherty set files: + issue11205.patchmessages: +
2015-04-09 02:25:56 rhettinger set priority: low -> high
2015-04-07 19:33:47 gvanrossum set messages: +
2015-04-07 18:21:29 sdougherty set nosy: + sdoughertymessages: +
2013-06-30 05:57:54 terry.reedy set versions: - Python 3.2
2013-03-28 13:23:58 terry.reedy set messages: +
2013-03-28 12:09:01 nedbat set nosy: + nedbatmessages: +
2013-01-10 19:21:43 gvanrossum set messages: +
2012-12-25 14:58:49 ezio.melotti set nosy: + gvanrossum, ezio.melottitype: behaviormessages: +
2012-12-25 14:41:49 ezio.melotti link issue16777 superseder
2012-11-07 19🔞21 terry.reedy set messages: +
2012-11-07 15:16:04 ncoghlan set priority: high -> lownosy: + ncoghlanversions: + Python 3.4
2012-06-24 20:12:44 pitrou set nosy: + benjamin.peterson
2012-06-23 07:15:54 terry.reedy set components: + Interpreter Core, - None
2012-01-09 21:24:11 sandro.tosi set nosy: + sandro.tosi
2012-01-09 20:02:42 amaury.forgeotdarc link issue13729 superseder
2011-06-12 22:35:21 terry.reedy set versions: - Python 3.1nosy: + terry.reedymessages: + stage: patch review
2011-03-19 21:22:28 rhettinger set priority: normal -> highnosy:georg.brandl, rhettinger, eric.araujo, r.david.murray, swamiyeswanth, takayuki
2011-03-19 19:09:24 skip.montanaro set nosy: - skip.montanaro
2011-02-18 19:19:14 eric.araujo set nosy: + eric.araujo
2011-02-13 23:41:53 rhettinger set nosy:skip.montanaro, georg.brandl, rhettinger, r.david.murray, swamiyeswanth, takayukimessages: +
2011-02-13 21:54:39 skip.montanaro set nosy:skip.montanaro, georg.brandl, rhettinger, r.david.murray, swamiyeswanth, takayukimessages: +
2011-02-13 21:52:17 rhettinger set nosy:skip.montanaro, georg.brandl, rhettinger, r.david.murray, swamiyeswanth, takayukimessages: +
2011-02-13 21:51:07 rhettinger set nosy:skip.montanaro, georg.brandl, rhettinger, r.david.murray, swamiyeswanth, takayukimessages: +
2011-02-13 15:20:54 pitrou set nosy: + rhettinger
2011-02-13 15:17:10 skip.montanaro set nosy:skip.montanaro, georg.brandl, r.david.murray, swamiyeswanth, takayukimessages: +
2011-02-13 14:56:29 georg.brandl set nosy:skip.montanaro, georg.brandl, r.david.murray, swamiyeswanth, takayukimessages: +
2011-02-13 14:35:33 r.david.murray set nosy: + r.david.murraymessages: +
2011-02-13 14:26:54 skip.montanaro set nosy:skip.montanaro, georg.brandl, swamiyeswanth, takayukimessages: +
2011-02-13 14:13:52 georg.brandl set nosy:skip.montanaro, georg.brandl, swamiyeswanth, takayukimessages: +
2011-02-13 13:37:42 skip.montanaro set nosy: + georg.brandlmessages: +
2011-02-13 13:32:38 skip.montanaro set files: + compile.diffnosy:skip.montanaro, swamiyeswanth, takayukimessages: +
2011-02-13 13:14:57 skip.montanaro set nosy:skip.montanaro, swamiyeswanth, takayukimessages: +
2011-02-13 13:14:33 skip.montanaro set files: - compile.diffnosy:skip.montanaro, swamiyeswanth, takayuki
2011-02-13 13:08:52 skip.montanaro set nosy:skip.montanaro, swamiyeswanth, takayukimessages: +
2011-02-13 13:04:09 skip.montanaro set files: + compile.diffversions: + Python 3.1, Python 3.2, Python 3.3nosy: + skip.montanaromessages: + keywords: + patch
2011-02-13 12:28:50 swamiyeswanth set nosy: + swamiyeswanth
2011-02-13 09:39:12 takayuki create