[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

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.