[Patches] [ python-Patches-1167628 ] [AST] Generator expressions

[SourceForge.net]

Patches item #1167628, was opened at 2005-03-21 06:45
Message generated for change (Comment added) made by bcannon
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1167628&group_id=5470

Category: Core (C code)
Group: AST
>Status: Closed
>Resolution: Accepted
Priority: 5
Submitted By: Nick Coghlan (ncoghlan)
Assigned to: Brett Cannon (bcannon)
Summary: [AST] Generator expressions

Initial Comment:
Adds generator expression support to the AST branch. 
Support is sufficient to allow test_grammar to pass. 
 
Also eliminates the display of interim results within functions 
compiled at the interactive prompt, and the allocation of large 
amounts of memory when zero is passed to asdl_seq_new. 

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

>Comment By: Brett Cannon (bcannon)
Date: 2005-04-13 12:59

Message:
Logged In: YES 
user_id=357491

OK, I went ahead and applied the patch realizing that if I
waited until test_grammar didn't segfault I would have one
massive commit and that would be bad.

So applied as rev. 1.1.2.12 for Python-ast.(c|h), 1.1.2.6
for asdl.c, 1.1.2.59 for ast.c, 2.247.2.3 for compile.c,
1.1.2.106 for newcompile.c, 2.10.8.32 for symtable.c,
1.1.2.11 for Python.asdl, 1.1.2.7 for asdl_c.py, and
1.1.2.14 for compile.txt .

I also went ahead and cleaned up all references to
GeneratorComp to be GeneratorExp instead.

Once again, thanks, Nick, for the patch!

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

Comment By: Brett Cannon (bcannon)
Date: 2005-04-12 17:22

Message:
Logged In: YES 
user_id=357491

OK, I have applied the patch and tweaked it some, but I have
no committed yet because I can't run test_grammar to
completion yet; need to get to the other patches before I
can get that far.

Couple comments on the patch, though, Nick.  First, please
use unified diffs if you can.  I personally find them easier
to read and they are the agreed-upon standard for patches. 
Another is to please use PEP 7 as a coding guideline when
possible.  And if the surrounding code isn't huge and breaks
the coding standard please fix it.  I am going to go through
and fix all the code eventually during final code review,
but whatever can get done now would be great.

And now I see what you mean about the grammar for genexps. 
All of that duplicate work between listcomps and genexps
just because of some REQ() statements seems wasteful.

Thanks for the hard work on the AST branch so far!

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

Comment By: Nick Coghlan (ncoghlan)
Date: 2005-03-22 05:18

Message:
Logged In: YES 
user_id=1038590

I'm going to be out of the country until late next week, so
if you want to incorporate this (or parts of it) into the
PyCon AST sprint, please do. I'll be interested to see the
results of the sprint when I get back.

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

Comment By: Nick Coghlan (ncoghlan)
Date: 2005-03-22 00:37

Message:
Logged In: YES 
user_id=1038590

Correcting a previous comment: the check that ensures an 
unparenthesised generator expression is a sole argument is in 
ast_for_call. 

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

Comment By: Nick Coghlan (ncoghlan)
Date: 2005-03-21 20:04

Message:
Logged In: YES 
user_id=1038590

Updated patch ast_genexp_2.diff which correctly allows generator 
expressions to be part of a testlist_gexp node or an argument node. 
 
Removed old patch which failed for debug builds. 

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

Comment By: Nick Coghlan (ncoghlan)
Date: 2005-03-21 19:42

Message:
Logged In: YES 
user_id=1038590

I'm actually wondering if the grammar is entirely correct
here. Really, what is allowed for an argument is:

argument: test [gen_for | ('=' test)]

But that still permits generator expressions that are not
the sole argument. So I'd be tempted to move the 'gen_for'
up to the arglist level:

arglist: (test gen_for) | ((argument ',')* (argument [','] |
'*' test [',' '**' test] |  '**' test))
argument: test ['=' test]

As it is, I simply have a check in ast_gen_exp which imposes
the above rule (i.e. a generator expression as an argument
must be the sole argument, or parenthesised so that it
becomes a 'test' node in its own right)

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

Comment By: Nick Coghlan (ncoghlan)
Date: 2005-03-21 19:22

Message:
Logged In: YES 
user_id=1038590

I did wonder about that, but the assert wasn't triggering
for me. The offending line is REQ(n, testlist_gexpr) in
ast_for_genexp.

I'll setup a debug build to check all of the assertions
properly.

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

Comment By: John Ehresman (jpe)
Date: 2005-03-21 10:02

Message:
Logged In: YES 
user_id=22785

This triggers an assert with a genexp in an argument because
the node is not a testlist_gexp, e.g. foo(i for i in
range(5)).  It's unclear what to do with foo(a = i for i in
range(5); see bug # 1167751

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

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