Skip to content

Conversation

@carljm
Copy link
Member

@carljm carljm commented Sep 27, 2024

Fix the case where a comprehension iteration variable is inlined into a function that otherwise does not have a variable by that name, but the name should be a free variable from an enclosing scope passed through to other enclosed scopes.

@JelleZijlstra
Copy link
Member

Found this failure case:

>>> def f(x):
...     def g():
...         [(x, lambda: x) for x in []]
...         def h():
...             print(x)
...         h()
...     g()
...     
>>> f(1)
Traceback (most recent call last):
  File "<python-input-1>", line 1, in <module>
    f(1)
    ~^^^
  File "<python-input-0>", line 7, in f
    g()
    ~^^
  File "<python-input-0>", line 6, in g
    h()
    ~^^
  File "<python-input-0>", line 5, in h
    print(x)
          ^
NameError: cannot access free variable 'x' where it is not associated with a value in enclosing scope

@JelleZijlstra
Copy link
Member

Simpler repro:

def f(x):
    def g():
        [lambda: x for x in []]
        def h():
            assert x == 1
        h()
    g()
    
f(1)

code = """
x = 1
def f():
[lambda: x for x in [1]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[lambda: x for x in [1]]
[lambda: x for x in [2]]

As discussed:

  • Use a different value (the test ran correctly but used the wrong 1)
  • Should change it to execute the lambda so we ensure the value of x inside the lambda is right

@carljm carljm marked this pull request as draft September 27, 2024 23:14
@limwz01
Copy link

limwz01 commented Nov 20, 2024

@carljm I think the tests are not testing the exact situation in my original example. They already pass in Python 3.13. The intervening list comprehension needs to be in the innermost scope like my original example, which still fails.

@limwz01
Copy link

limwz01 commented Nov 20, 2024

oh, sorry ignore me, I was confused. apparently the tests passed because I ran it at module scope. they are also tested in function scope, so that is ok, I think

@limwz01
Copy link

limwz01 commented Nov 20, 2024

but maybe you should also add a test to also test a local in f() to be sure. (I keep getting confused by the change of the function names between our examples)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants