Evaluation of iterables in generator expressions uses wrong scope

[robertwb]

In Python, this works:

it = (1,2,3)
l = list(it for it in it)

In current Cython, this raises an error at runtime as the iterable inside of the generator expression refers to the loop target variable, not the name in the outer scope. (So the iterable happens to be None.)

The name of the iterable (or rather the entire iterable expression) must be evaluated in the outer scope.

Actually, it's even different - CPython simply pre-evaluates the outermost iterable when creating the generator. Example (Py3.2):

>>> x = [range(10)]
>>> g = (i for x in x for i in x)
>>> list(g)
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]

>>> x = [range(10)]
>>> g = (i for i in range(5) for x in x)
>>> list(g)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 1, in <genexpr>
UnboundLocalError: local variable 'x' referenced before assignment

So, basically, the outermost iterable should be evaluated into a closure variable of the generator, instead of letting the generator evaluate it itself. This:

>>> x = [range(10)]
>>> g = (i for x in x for i in x)

should be transformed into

x = [range(10)]

def _gen(_outer_it):
    for x in _outer_it:
        for i in x:
            yield i

g = _gen(x)

Note that this also applies to list comprehensions with language_level=3.

Migrated from http://trac.cython.org/ticket/600

[robertwb]

scoder changed description from

In Python, this works:

it = (1,2,3)
l = list(it for it in it)

In current Cython, this raises an error at runtime as the iterable inside of the generator expression refers to the loop target variable, not the name in the outer scope. (So the iterable happens to be None.)

The name of the iterable must be looked up before creating the entry of the target variable in the inner scope.

to

In Python, this works:

it = (1,2,3)
l = list(it for it in it)

In current Cython, this raises an error at runtime as the iterable inside of the generator expression refers to the loop target variable, not the name in the outer scope. (So the iterable happens to be None.)

The name of the iterable (or rather the entire iterable expression) must be evaluated in the outer scope.
commented

[robertwb]

scoder changed cc to vitja
description from

In Python, this works:

it = (1,2,3)
l = list(it for it in it)

In current Cython, this raises an error at runtime as the iterable inside of the generator expression refers to the loop target variable, not the name in the outer scope. (So the iterable happens to be None.)

The name of the iterable (or rather the entire iterable expression) must be evaluated in the outer scope.

to

In Python, this works:

it = (1,2,3)
l = list(it for it in it)

In current Cython, this raises an error at runtime as the iterable inside of the generator expression refers to the loop target variable, not the name in the outer scope. (So the iterable happens to be None.)

The name of the iterable (or rather the entire iterable expression) must be evaluated in the outer scope.
commented

Actually, it's even different - CPython simply pre-evaluates the outermost iterable when creating the generator. Example (Py3.2):

>>> x = [g = (i for x in x for i in x)
>>> list(g)
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9](range(10)]
>>>)

>>> x = [g = (i for i in range(5) for x in x)
>>> list(g)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 1, in <genexpr>
UnboundLocalError: local variable 'x' referenced before assignment

So, basically, the outermost iterable should be evaluated into a closure variable of the generator, instead of letting the generator evaluate it itself. This:

x = [range(10)](range(10)]
>>>)
g = (i for x in x for i in x)

should be transformed into

x = [range(10)]

def _gen(_outer_it):
    for x in _outer_it:
        for i in x:
            yield i

g = _gen(x)

[robertwb]

scoder commented

Another test that currently works differently in Python than in Cython:

>>> def seff():
...     print("TEST")
...     return range(10)
... 
>>> g = (i for i in seff())
TEST
>>> list(g)
[1, 2, 3, 4, 5, 6, 7, 8, 9](0,)
>>> g = (i for i in (v for v in seff()))
TEST

[robertwb]

scoder changed priority from major to critical
commented

[robertwb]

scoder changed milestone from wishlist to 0.20
owner to scoder
priority from critical to major
status from new to assigned
commented

Fixed here:

87a9bc2

[robertwb]

scoder changed priority from major to critical
resolution to fixed
status from assigned to closed
commented

[robertwb]

scoder changed milestone from 0.20 to wishlist
resolution from fixed to empty
status from closed to reopened
commented

[embray]

@scoder thanks for the cleanup--I see now how the bug described in this ticket is still a problem too. I'll see if I can do something about it...

[embray]

Apparently the original attempt to fix this was also reverted in a3d4461, which is part of my confusion as well.

[scoder]

I'd be happy to have more people look into this, but be "warned", it's not entirely trivial. Cython looks at the iterable in several places in order to optimise loops. Moving the iterable out of the generator too early will prevent these optimisations. Moving it out too late will lead to incorrect declarations inside and/or outside of the generator. This is one of the bugs that have been languishing for a bit of a reason.

[embray]

Thanks for the admonition. That will help me understand better what the problem is.

[scoder]

Nominating this as a release blocker for Cython 3.0 to keep it in sight, although it's not really a breaking change.