yield inside of list comprehension

[isidentical]

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]

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]

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]

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]

@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 :)