Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

yield inside of list comprehension #132

Open
isidentical opened this issue May 30, 2020 · 4 comments
Open

yield inside of list comprehension #132

isidentical opened this issue May 30, 2020 · 4 comments

Comments

@isidentical
Copy link
Collaborator

for <=py3.7

 $ python3.7
Python 3.7.5 (default, Apr 19 2020, 20:18:17) 
[GCC 9.2.1 20191008] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> (lambda: [(yield x + 1) for x in seq])
<function <lambda> at 0x7f185ed11440>
>>> get_errors("(lambda: [(yield x + 1) for x in seq])")
[<Issue: 901>]
>>> get_errors("(lambda: [(yield x + 1) for x in seq])")[0].message
"SyntaxError: 'yield' outside function"
>>> 

py3.8>=

 $ python3  
Python 3.10.0a0 (heads/master:7b78e7f9fd, May 30 2020, 13:18:34) 
[GCC 9.2.1 20191008] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> (lambda: [(yield x + 1) for x in seq])
  File "<stdin>", line 1
SyntaxError: 'yield' inside list comprehension
>>> get_errors("(lambda: [(yield x + 1) for x in seq])")[0].message
"SyntaxError: 'yield' outside function"
@davidhalter
Copy link
Owner

BTW In general I don't care if the errors don't match exactly for older versions. In this case both messages make sense for 3.7 and 3.8. We can simply use the 3.8 version for all Python versions and somehow adjust the tests IMO.

@isidentical
Copy link
Collaborator Author

The main problem with this issue is that usage of yield is fine with lambda (on every version, not sure about 2 though)

>>> get_errors("lambda: (yield)")
["SyntaxError: 'yield' outside function"]
>>> lambda: (yield)
<function <lambda> at 0x7ff212322050>

@gousaiyang
Copy link
Collaborator

Looks like things are a bit tricky here. The current code is checking if self._normalizer.context.node.type != 'funcdef', however lambdef is not recognized as a type of context currently. I'm trying two approaches.

Approach 1: with search_ancestor

I tried to do a search_ancestor to locate the nearest ancestor which is either a funcdef, classdef, lambdef or comprehension. A yield expression is only allowed if the nearest ancestor is a funcdef or lambdef. But this approach turns out to be inaccurate:

  • If a yield expression appears in the default value or annotation part of a function declaration, it does not belong to the scope inside the function, but belongs to containing scope. Examples:
    • def foo(x=(yield 1)): pass
    • lambda x=(yield 1): 1
    • def foo(x: (yield 1)): pass
    • def foo() -> (yield 1): pass
      These code snippets are invalid but will be incorrectly determined as valid in this approach.
  • Similarly, a yield expression may appear in the arglist part of a classdef.

For example,

def foo():
    class A:
        yield 1

is invalid, but

def foo():
    class A((yield 1)):
        pass

is syntactically valid.

  • The first iterable expression (i.e. the expression after the first in) in a comprehension is executed in the containing scope, while other parts are inside the comprehension inner scope.
    • def foo(): [1 for x in [(yield 1)]] is valid
    • def foo(): [(yield 1) for x in [1]] is invalid
    • def foo(): [1 for x in [1] if (yield 1)] is invalid
    • def foo(): [1 for x in [1] for y in [(yield 1)]] is invalid

If we were to use this search_ancestor approach, we have to check whether a child node is inside the suite part of a funcdef or classdef, and special-case the first iterable expression of comprehension.

Approach 2: using context

The notion of context in parso/python/errors.py seems to correctly handle funcdef and classdef scope problem in Approach 1. We probably need to extend the current _Context and ErrorFinder classes in parso/python/errors.py to recognize lambdef and comprehensions as types of context in addition to funcdef and classdef. This would help to:

  • Identify that the yield expression in lambda: (yield 1) is in a lambdef context and thus valid.
  • Identify that the assignment expression in class A: [lambda: (x := 1) for y in [1]] is in a lambdef context rather than a classdef context, and thus valid. (While class A: [(x := 1) for y in [1]] is a forbidden case described in PEP 572.)
  • Correctly identify the first iterable expression of comprehension as not in the comprehension scope. (While the other parts of the comprehension are considered to be executed in an implicit function scope.)

To make this change we need to look at the parso/python/errors.py file carefully for any use of context. A tricky example is that, if comprehensions are recognized as a type of context, in order to correctly identify async def foo(): [(await x) for y in [1]] as valid (not to mistakenly report 'await' outside async function), we need to look through parent contexts to find async_funcdef, instead of only checking whether the innermost context is an async_funcdef.

@davidhalter
Copy link
Owner

@gousaiyang I guess I'm ok with whatever you come up with. My gut tells me that lambdef is a type of context. But if you feel like the other approach is better, feel free to do that.

In general I think with these kinds of issues good test coverage is probably almost more important than good code :)

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

No branches or pull requests

3 participants