[issue9011] ast_for_factor unary minus optimization changes AST
Changes by Mark Dickinson dicki...@gmail.com: -- versions: -Python 2.6, Python 3.1 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson added the comment: New patch. -- assignee: - mark.dickinson Added file: http://bugs.python.org/file28118/issue9011_v2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Roundup Robot added the comment: New changeset ea36de7926f8 by Mark Dickinson in branch '2.7': Issue #9011: AST creation no longer modifies CST for negated numeric literals. http://hg.python.org/cpython/rev/ea36de7926f8 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Changes by Mark Dickinson dicki...@gmail.com: -- resolution: - fixed status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Antonio Cuni added the comment: there is still an inconsistency in handling negative imaginary literals: -1j.real -0.0 complex('-1j').real 0.0 -- nosy: +antocuni ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson added the comment: No, that's expected behaviour. 1j is complex(0.0, 1.0) -1j is complex(-0.0, -1.0) so -1j.real is -0.0. There's not really any other sensible way to handle this. The complex-from-string constructor, on the other hand, is more careful about interpreting signs. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Antonio Cuni added the comment: I would say that the complex-from-string constructor should be fixed to handle this special case correctly. I find very confusing that we get a different result whether we use a string literal or not. For example, in pypy we use the same code for parsing literals and converting strings, so you get -0.0 in both cases. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson added the comment: With the string, the minus sign applies only to the imaginary part; with the expression '-1j', it applies to the whole complex number (both real and imaginary parts). I don't see any sensible way to 'fix' the string to complex conversion; indeed, I think any change would make it worse than before. It's a known issue with complex arithmetic that x + 1j*y doesn't give you complex(x, y); the conversions from string and the complex(x, y) form are there to make it possible to carefully create a complex number with known real and imaginary parts. For example, in pypy we use the same code for parsing literals and converting strings, so you get -0.0 in both cases. But -1j isn't a literal. It's unary minus applied to a the complex number given by the literal '1j'. Python's code *does* give the same results both for converting strings and parsing literals. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Amaury Forgeot d'Arc added the comment: Indeed, -1j is not a literal: dis.dis(lambda :-1j.real) 1 0 LOAD_CONST 0 (1j) 3 LOAD_ATTR0 (real) 6 UNARY_NEGATIVE 7 RETURN_VALUE -- nosy: +amaury.forgeotdarc ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson added the comment: And -1j.real is -(1j.real), of course, not (-1j).real. My bad. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson dicki...@gmail.com added the comment: Yes, sorry; I'm not likely to find time to do anything with this. Unassigning, and downgrading priority. Is it worth leaving this open in case anyone wants to do something about it? -- assignee: mark.dickinson - priority: high - normal ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
R. David Murray rdmur...@bitdance.com added the comment: Given the long projected lifetime of 2.7, I suppose it is. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
R. David Murray rdmur...@bitdance.com added the comment: Mark, are you still planning to do something for 3.1/2.7, or should this be closed? -- nosy: +r.david.murray stage: commit review - needs patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Neil Schemenauer nas-pyt...@arctrix.com added the comment: I'm late to the party but looks like Mark has a good handle on the issues. I would recommend not changing behavior in a bugfix release. I'm pretty sure code in the wild would be broken. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson dicki...@gmail.com added the comment: It turns out that removing this code from py3k wasn't a no-op. It changed the semantics for complex literals. In Python 3.1.2: -7j -7j (-7j).real 0.0 But in current release31-maint branch, and in 3.2a0: Python 3.2a0 (py3k:82347M, Jun 28 2010, 22:25:11) [GCC 4.2.1 (Apple Inc. build 5659)] on darwin Type help, copyright, credits or license for more information. -7j (-0-7j) (-7j).real -0.0 So IMO r82044 (the release31-maint branch) should be reverted, to avoid nasty surprises when people upgrade from 3.1.2 to 3.1.3. But I'd like to keep this change in py3k: I consider that the 3.1 behaviour is buggy, and that the 3.2 behaviour is more correct (strange though it may seem at first sight), so I'd prefer to leave the current py3k behaviour as is, and add a Misc/NEWS entry describing the change if necessary. Assigning this to me because I don't want it to get forgotten. But if anyone else is interested in producing a patch for the cleanup of the CST-AST code, please do! -- assignee: - mark.dickinson versions: -Python 2.6, Python 2.7 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson dicki...@gmail.com added the comment: r82044 reverted in r82389. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson dicki...@gmail.com added the comment: Added tests for the current Python 3.2 treatment of imaginary literals in r82390 (release31-maint). These tests should be backported to trunk, but I'll wait until after the release of 2.7 for that. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson dicki...@gmail.com added the comment: Added tests for Python 3.2's corrected treatment of negated imaginary literals in r82391, along with a Misc/NEWS entry indicating the changed behaviour. Restored version numbers for this issue, which I seem to have accidentally deleted earlier. (Including 3.1, since it could also benefit from the same cleanup.) -- versions: +Python 2.6, Python 2.7, Python 3.1 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Changes by Meador Inge mead...@gmail.com: -- nosy: +minge ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson dicki...@gmail.com added the comment: Summary of brief discussion on #python-dev: - This is a hacky patch; it would be better to rewrite that portion of ast.c in a way that doesn't modify the original CST at all. With 2.7 so close, I daren't try anything more invasive than this patch, though. (I don't know whether there are any other portions of this file that deliberately change the CST.) - Given that no-one noticed this problem in 5 years, the fix can probably wait until 2.7.1, when it can be done properly. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Changes by Mark Dickinson dicki...@gmail.com: -- nosy: +mark.dickinson ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson dicki...@gmail.com added the comment: Confirmed in trunk and py3k, both of which raise ValueError on the second compile: Python 2.7rc1+ (trunk:82042, Jun 17 2010, 09:52:12) [GCC 4.0.1 (Apple Inc. build 5490)] on darwin Type help, copyright, credits or license for more information. import parser st = parser.expr(-3) st.compile() code object module at 0x450448, file syntax-tree, line 1 st.compile() Traceback (most recent call last): File stdin, line 1, in module ValueError: could not convert string to float: --3 This looks nasty; unless there's some rule that says that you're not supposed to hang on to AST objects after compiling them. (Is there?) Benjamin, would it be worth just getting rid of this optimization for 2.7, and trying to reinstate a correct version for 2.7.1? [Removing 2.5 from the versions list since there's nothing we can do about it there (2.5 is only getting security fixes at this point).] See also issue 1441486. -- versions: +Python 2.7, Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Changes by Mark Dickinson dicki...@gmail.com: -- versions: -Python 2.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson dicki...@gmail.com added the comment: N.B. That if block isn't pure optimization: removing it gives a minor change in behaviour: Currently (Python 2.7, on a 64-bit machine): -9223372036854775808 -9223372036854775808 And with the optimization removed: -9223372036854775808 -9223372036854775808L I actually consider the second behaviour more correct than the first, since it follows clearly from the language rules (numeric literals have no sign, so the above *should* be interpreted as the unary minus operator applied to a literal, and that literal really is a PyLong). But obviously the contributors to issue 1441486 either disagree, or didn't want to introduce a regression from 2.4. I still consider that removing that if block is the right thing to do for 2.7. The change in behaviour really shouldn't affect any reasonable code---anywhere that an int is acceptable, a long should be too. Neil, any comments? -- nosy: +nascheme priority: normal - high ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson dicki...@gmail.com added the comment: Results from Jython: newton:jython2.5.1 dickinsm$ ./jython Jython 2.5.1 (Release_2_5_1:6813, Sep 26 2009, 13:47:54) [Java HotSpot(TM) 64-Bit Server VM (Apple Inc.)] on java1.6.0_20 Type help, copyright, credits or license for more information. -2147483648 -2147483648L On the other hand, IronPython appears to detect this special case and produces an int instead of a long. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson dicki...@gmail.com added the comment: Fixed in r82043 (py3k) and r82044 (release31-maint), simply by removing the relevant 'if' block. For 2.x, Antoine (on IRC) pointed out that there might well be third party code that depends on -0x8000 being an int rather than a long (32-bit machine), so changing that behaviour could cause breakage. Can anyone propose a fix that doesn't change behaviour? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Changes by Mark Dickinson dicki...@gmail.com: -- versions: -Python 3.1, Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Alex Samuel a...@alexsamuel.net added the comment: How about saving the original value of STR(pnum) and restoring it after calling ast_for_atom()? This is not thread-safe, but I don't understand Python's threading model well enough to know whether the GIL is held in this function. On 6/17/2010 8:40 AM, Mark Dickinson wrote: Mark Dickinsondicki...@gmail.com added the comment: Fixed in r82043 (py3k) and r82044 (release31-maint), simply by removing the relevant 'if' block. For 2.x, Antoine (on IRC) pointed out that there might well be third party code that depends on -0x8000 being an int rather than a long (32-bit machine), so changing that behaviour could cause breakage. Can anyone propose a fix that doesn't change behaviour? -- ___ Python trackerrep...@bugs.python.org http://bugs.python.org/issue9011 ___ -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Mark Dickinson dicki...@gmail.com added the comment: That sounds like a reasonable quick fix. Here's a patch. -- keywords: +patch stage: - commit review Added file: http://bugs.python.org/file17696/issue9011.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
New submission from Alex Samuel a...@alexsamuel.net: The unary negative optimization in ast_for_factor() seems to modify the ST in a way changes its meaning. In Python 3.1.2, the ST is no longer compilable: $ cat exprbug.py import parser st = parser.expr(-3) print(st.totuple()) compiled = st.compile() print(eval(compiled)) print(st.totuple()) print(eval(st.compile())) $ ~/sw/Python-3.1.2/bin/python3 exprbug.py (258, (326, (301, (305, (306, (307, (308, (310, (311, (312, (313, (314, (315, (316, (317, (15, '-'), (317, (318, (319, (2, '3')), (4, ''), (0, '')) -3 (258, (326, (301, (305, (306, (307, (308, (310, (311, (312, (313, (314, (315, (316, (317, (15, '-'), (317, (318, (319, (2, '-3')), (4, ''), (0, '')) Traceback (most recent call last): File exprbug.py, line 8, in module print(eval(st.compile())) ValueError: could not convert string to float: --3 In earlier versions of Python (I have confirmed 2.5 and 2.6), it is compiled to incorrect code and produces wrong results when evaluated: $ ~/sw/Python-2.6.2/bin/python exprbug.py (258, (327, (304, (305, (306, (307, (308, (310, (311, (312, (313, (314, (315, (316, (15, '-'), (316, (317, (318, (2, '3'), (4, ''), (0, '')) -3 (258, (327, (304, (305, (306, (307, (308, (310, (311, (312, (313, (314, (315, (316, (15, '-'), (316, (317, (318, (2, '-3'), (4, ''), (0, '')) -1.0 If I remove the big if statement from the front of ast_to_factor(), the code behaves correctly. I think this is because STR(pnum) is changed in place and never restored to its previous value. -- components: Interpreter Core messages: 107928 nosy: alexhsamuel priority: normal severity: normal status: open title: ast_for_factor unary minus optimization changes AST type: behavior versions: Python 2.5, Python 2.6, Python 3.1 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9011] ast_for_factor unary minus optimization changes AST
Changes by R. David Murray rdmur...@bitdance.com: -- nosy: +benjamin.peterson ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9011 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com