Issue1764

classification
Title: Broken behavior with (a + b).foo = bar
Type: Severity: normal
Components: Core Versions: 2.5.2, 2.5.1, 2.5b1, 2.5b0, 25rc4, 2.5.0, 2.5.2b1, 2.5.2rc
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: fwierzbicki Nosy List: amak, fwierzbicki, luizribeiro, pjenvey
Priority: high Keywords:

Created on 2011-06-25.13:41:09 by luizribeiro, last changed 2012-03-19.20:42:14 by fwierzbicki.

Files
File name Uploaded Description Edit Remove
check.txt luizribeiro, 2011-08-01.22:42:50 Output from the byte code verifier
Messages
msg6560 (view) Author: Luiz Ribeiro (luizribeiro) Date: 2011-06-25.13:46:20
Here is the expected output from CPython 2.5:

Python 2.5.2 (r252:60911, Jan 24 2010, 17:44:40) 
[GCC 4.3.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> a = [1,2]
>>> b = [3]
>>> (a + b).foo = 42
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'list' object has no attribute 'foo'

Here is the broken behavior from Jython 2.5.2:

Jython 2.5.2 (Release_2_5_2:7206, Mar 2 2011, 23:12:06) 
[OpenJDK Server VM (Sun Microsystems Inc.)] on java1.6.0_20
Type "help", "copyright", "credits" or "license" for more information.
>>> a = [1,2]
>>> b = [3]
>>> (a + b).foo = 42
java.lang.VerifyError: (class: org/python/pycode/_pyx3, method: f$0 signature: (Lorg/python/core/PyFrame;Lorg/python/core/ThreadState;)Lorg/python/core/PyObject;) Unable to pop operand off an empty stack
	at java.lang.Class.getDeclaredConstructors0(Native Method)
	at java.lang.Class.privateGetDeclaredConstructors(Class.java:2406)
	at java.lang.Class.getConstructor0(Class.java:2716)
	at java.lang.Class.getConstructor(Class.java:1674)
	at org.python.core.BytecodeLoader.makeCode(BytecodeLoader.java:68)
	at org.python.compiler.LegacyCompiler$LazyLegacyBundle.loadCode(LegacyCompiler.java:43)
	at org.python.core.CompilerFacade.compile(CompilerFacade.java:34)
	at org.python.core.Py.compile_flags(Py.java:1703)
	at org.python.core.Py.compile_command_flags(Py.java:1749)
	at org.python.util.InteractiveInterpreter.runsource(InteractiveInterpreter.java:52)
	at org.python.util.InteractiveInterpreter.runsource(InteractiveInterpreter.java:46)
	at org.python.util.InteractiveConsole.push(InteractiveConsole.java:110)
	at org.python.util.InteractiveConsole.interact(InteractiveConsole.java:90)
	at org.python.util.jython.run(jython.java:317)
	at org.python.util.jython.main(jython.java:129)

java.lang.VerifyError: java.lang.VerifyError: (class: org/python/pycode/_pyx3, method: f$0 signature: (Lorg/python/core/PyFrame;Lorg/python/core/ThreadState;)Lorg/python/core/PyObject;) Unable to pop operand off an empty stack


The same broken behavior applies to versions 2.5.1 and 2.5.0. Version 2.2.1 matches with the expected output, though:

Jython 2.2.1 on java1.6.0_20
Type "copyright", "credits" or "license" for more information.
>>> a = [1,2]
>>> b = [3]
>>> (a + b).foo = 42
Traceback (innermost last):
  File "<console>", line 1, in ?
AttributeError: 'list' object has no attribute 'foo'
msg6593 (view) Author: Luiz Ribeiro (luizribeiro) Date: 2011-08-01.22:19:59
After tracking this issue for a while, I believe this is a problem on the generated AST. Since I'm not very familiar with Jython's inner-workings, can anyone confirm this to me?

Take this example script:
a = 1
b = 2
(a + b)
(a + b).c = 3

The generated AST is:
('Module',
 ('body',
  ('Assign (1,0)',
   ('targets', ('Name (1,0)', ('id', 'a'), ('ctx', ('Store',)))),
   ('value', ('Num (1,4)', ('n', 1)))),
  ('Assign (2,0)',
   ('targets', ('Name (2,0)', ('id', 'b'), ('ctx', ('Store',)))),
   ('value', ('Num (2,4)', ('n', 2)))),
  ('Expr (3,0)',
   ('value',
    ('BinOp (3,1)',
     ('left', ('Name (3,1)', ('id', 'a'), ('ctx', ('Load',)))),
     ('op', ('Add',)),
     ('right', ('Name (3,5)', ('id', 'b'), ('ctx', ('Load',))))))),
  ('Assign (4,0)',
   ('targets',
    ('Attribute (4,0)',
     ('value',
      ('BinOp (4,1)',
       ('left', ('Name (4,1)', ('id', 'a'), ('ctx', ('Store',)))),
       ('op', ('Add',)),
       ('right', ('Name (4,5)', ('id', 'b'), ('ctx', ('Store',)))))),
     ('attr', 'c'),
     ('ctx', ('Store',)))),
   ('value', ('Num (4,12)', ('n', 3))))))

I believe the context from the last statement is wrong. Shouldn't it be using the Load context on the operands of BinOp (4,1)?

In case you're wondering, expressions like (~a).attr = b are also crashing Jython.
msg6594 (view) Author: Luiz Ribeiro (luizribeiro) Date: 2011-08-01.22:42:49
I have attached the output from the byte code verifier CheckClassAdapter from the ASM project. You can see that there is a call to PyFrame's setLocal method on the wrong place.
msg6595 (view) Author: Philip Jenvey (pjenvey) Date: 2011-08-01.23:34:03
You can compare the astview.py output as run on CPython 2.5 and see if there's a difference
msg6596 (view) Author: Luiz Ribeiro (luizribeiro) Date: 2011-08-02.03:09:34
Thanks Philip, I didn't thought of that. :)

Looks like my guessing was correct, here's the output with CPython for the same script:
('Module',
 ('body',
  ('Assign (1,0)',
   ('targets', ('Name (1,0)', ('id', 'a'), ('ctx', ('Store',)))),
   ('value', ('Num (1,4)', ('n', 1)))),
  ('Assign (2,0)',
   ('targets', ('Name (2,0)', ('id', 'b'), ('ctx', ('Store',)))),
   ('value', ('Num (2,4)', ('n', 2)))),
  ('Expr (3,0)',
   ('value',
    ('BinOp (3,1)',
     ('left', ('Name (3,1)', ('id', 'a'), ('ctx', ('Load',)))),
     ('op', ('Add',)),
     ('right', ('Name (3,5)', ('id', 'b'), ('ctx', ('Load',))))))),
  ('Assign (4,0)',
   ('targets',
    ('Attribute (4,1)',
     ('value',
      ('BinOp (4,1)',
       ('left', ('Name (4,1)', ('id', 'a'), ('ctx', ('Load',)))),
       ('op', ('Add',)),
       ('right', ('Name (4,5)', ('id', 'b'), ('ctx', ('Load',)))))),
     ('attr', 'c'),
     ('ctx', ('Store',)))),
   ('value', ('Num (4,12)', ('n', 3))))))
msg6622 (view) Author: Frank Wierzbicki (fwierzbicki) Date: 2011-08-31.17:10:04
I have verified this and tracked this down to a particularly ugly bit of the grammar - if you are interested grep for the understatement "//XXX: This could be better."

The general case of nodes with children on the lhs of assignment is broken - I should be able to put a fix together this evening.
msg6623 (view) Author: Frank Wierzbicki (fwierzbicki) Date: 2011-08-31.17:42:16
I should clarify - the case of lhs nodes with children that are part of a call, attribute or subscript is broken -- things like
a, b = c, d
are of course not broken (that would have shown up quickly in tests!)
msg6625 (view) Author: Frank Wierzbicki (fwierzbicki) Date: 2011-09-02.16:56:44
Sorry I didn't really get to this the other day - the fix is trivial, I just need to get my mercurial workflow back in order and check things in. In case it helps now the fix is:

-                  if ($etype instanceof Context) {
-                      ((Context)$etype).setContext(expr_contextType.Load);
-                  }
+                  actions.recurseSetContext($etype, expr_contextType.Load);
msg6626 (view) Author: Luiz Ribeiro (luizribeiro) Date: 2011-09-02.22:27:16
Your patch worked like a charm here. Thanks for taking your time into this, Frank.
msg6627 (view) Author: Frank Wierzbicki (fwierzbicki) Date: 2011-09-02.22:49:22
Luiz: No problem, thanks for filing a bug! I've been meaning to get back into Jython development, and this was a nice warm up :)
msg6639 (view) Author: Frank Wierzbicki (fwierzbicki) Date: 2011-09-04.23:51:07
This fix is now merged into trunk and the 2.5 maintenance branch - I'm going to leave it open for now to remind myself to add a test to test_ast.py
msg6856 (view) Author: Alan Kennedy (amak) Date: 2012-03-19.20:15:51
Is this fixed? Should the issue be closed?
msg6867 (view) Author: Frank Wierzbicki (fwierzbicki) Date: 2012-03-19.20:42:14
Thanks Alan, closing.
History
Date User Action Args
2012-03-19 20:42:14fwierzbickisetstatus: open -> closed
messages: + msg6867
2012-03-19 20:15:51amaksetnosy: + amak
messages: + msg6856
2011-09-04 23:51:07fwierzbickisetresolution: accepted -> fixed
messages: + msg6639
2011-09-02 22:49:23fwierzbickisetmessages: + msg6627
2011-09-02 22:27:17luizribeirosetmessages: + msg6626
2011-09-02 16:56:44fwierzbickisetmessages: + msg6625
2011-08-31 17:42:16fwierzbickisetmessages: + msg6623
2011-08-31 17:10:05fwierzbickisetpriority: high
resolution: accepted
messages: + msg6622
2011-08-03 15:36:11fwierzbickisetassignee: fwierzbicki
nosy: + fwierzbicki
2011-08-02 03:09:34luizribeirosetmessages: + msg6596
2011-08-01 23:34:03pjenveysetnosy: + pjenvey
messages: + msg6595
2011-08-01 22:42:50luizribeirosetfiles: + check.txt
messages: + msg6594
2011-08-01 22:20:00luizribeirosetmessages: + msg6593
2011-06-25 13:46:21luizribeirosetmessages: + msg6560
title: Broken behavior with (a + b).attr = 10 -> Broken behavior with (a + b).foo = bar
2011-06-25 13:41:09luizribeirocreate