[Patches] [ python-Patches-1531113 ] "a += yield foo" compiles as "a += yield (yield foo)"

[SourceForge.net]

Patches item #1531113, was opened at 2006-07-29 23:56
Message generated for change (Tracker Item Submitted) made by Item Submitter
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: Open
Resolution: None
Priority: 5
Submitted By: Christopher Tur Lesniewski-Laas (ctl)
Assigned to: Nobody/Anonymous (nobody)
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


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1531113&group_id=5470