Patches item #1531113, was opened at 2006-07-29 20:56
Message generated for change (Comment added) made by nnorwitz
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1531113&group_id=5470
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Parser/Compiler
Group: Python 2.5
>Status: Closed
>Resolution: Fixed
Priority: 9
Submitted By: Christopher Tur Lesniewski-Laas (ctl)
>Assigned to: Neal Norwitz (nnorwitz)
Summary: "a += yield foo" compiles as "a += yield (yield foo)"
Initial Comment:
[Note: my apologies for not attaching the patches using
the file upload; the file upload button is not showing
up in my Mozilla-based browsers. I'd be happy to email
the patches in MIME attachments if you provide an email
address.]
While working on a new async framework package based on
Python 2.5's new coroutine generators, I discovered a
"this could never have worked" bug in the AST compiler
or augmented assigns using the new yield operator.
The bug is that "a += yield foo" is compiled as "a +=
yield (yield foo)". A simple program that demonstrates
this bug is:
def coroutine():
a = 0
while a < 200:
a += yield
print a
c = coroutine()
c.next()
c.send(10)
c.send(20)
c.send(30)
c.send(40)
c.send(50)
This prints out:
20
60
instead of the expected:
10
30
60
100
150
The disassembler shows that two Yield instructions are
getting inserted where
there should be just one:
2 0 LOAD_CONST 1 (0)
3 STORE_FAST 0 (a)
3 6 SETUP_LOOP 35 (to 44)
>> 9 LOAD_FAST 0 (a)
12 LOAD_CONST 2 (200)
15 COMPARE_OP 0 (<)
18 JUMP_IF_FALSE 21 (to 42)
21 POP_TOP
4 22 LOAD_FAST 0 (a)
25 LOAD_CONST 0 (None)
28 YIELD_VALUE
29 YIELD_VALUE
30 INPLACE_ADD
31 STORE_FAST 0 (a)
5 34 LOAD_FAST 0 (a)
37 PRINT_ITEM
38 PRINT_NEWLINE
39 JUMP_ABSOLUTE 9
>> 42 POP_TOP
43 POP_BLOCK
>> 44 LOAD_CONST 0 (None)
47 RETURN_VALUE
The offending code is in Python/ast.c, function
ast_for_expr_stmt():
expr2 = Yield(ast_for_expr(c, ch), LINENO(ch),
ch->n_col_offset, c->c_arena);
since ast_for_expr(c, ch) processes the Yield node
again. The following patch resolves the problem for me:
Index: Python/ast.c
===================================================================
--- Python/ast.c (revision 50930)
+++ Python/ast.c (working copy)
@@ -1964,7 +1964,7 @@
if (TYPE(ch) == testlist)
expr2 = ast_for_testlist(c, ch);
else
- expr2 = Yield(ast_for_expr(c, ch),
LINENO(ch), ch->n_col_offset, c->c_arena);
+ expr2 = ast_for_expr(c, ch);
if (!expr2)
return NULL;
--------------------------------------------------------------------
A separate, minor issue I discovered while looking at
this code: the ast_for_expr_stmt function seems to
contain code to handle the case:
(yield foo) += bar
even though it correctly fails on the case:
(yield foo) = bar
This is just confusing; the two cases should match.
The following patch is one way of solving this problem:
Index: Python/ast.c
===================================================================
--- Python/ast.c (revision 50930)
+++ Python/ast.c (working copy)
@@ -1928,11 +1928,11 @@
operator_ty newoperator;
node *ch = CHILD(n, 0);
- if (TYPE(ch) == testlist)
- expr1 = ast_for_testlist(c, ch);
- else
- expr1 = Yield(ast_for_expr(c, CHILD(ch,
0)), LINENO(ch), n->n_col_offset,
- c->c_arena);
+ if (TYPE(ch) == yield_expr) {
+ ast_error(ch, "assignment to yield
expression not possible");
+ return NULL;
+ }
+ expr1 = ast_for_testlist(c, ch);
if (!expr1)
return NULL;
Cheers,
Chris Lesniewski
ctl mit edu
----------------------------------------------------------------------
>Comment By: Neal Norwitz (nnorwitz)
Date: 2006-07-30 00:01
Message:
Logged In: YES
user_id=33168
Thanks!
The second part about fixing assignment to yield expressions
wasn't correct. Also there was a SystemError for (yield
bar) = foo. That definitely needed fixing. Let me know if
you find any more issues.
Committed revision 50968.
BTW, I believe SF fixed the upload button issue by removing
the button. I guess we need to update the comment. :-) It
should just work now.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1531113&group_id=5470
_______________________________________________
Patches mailing list
Patches@python.org
http://mail.python.org/mailman/listinfo/patches
