Fix crash with yield in comprehension #12048
Conversation
mypy/semanal.py
Outdated
| if not self.is_func_scope(): # not sure | ||
| self.fail('"yield from" outside function', e, serious=True, blocker=True) | ||
| if not self.is_func_scope() or self.is_comprehension_stack[-1]: | ||
| self.fail('"yield from" outside function or lambda', e, serious=True, blocker=True) |
There was a problem hiding this comment.
I don't think we need the "or lambda" part. Lambdas are functions, and it's very rare to have a yield in a lambda, so it seems more likely to confuse users than to help them.
There was a problem hiding this comment.
Also, it would be good to have a separate error message for yields in comprehensions.
| @@ -76,11 +76,20 @@ break # E: "break" outside loop | |||
| continue # E: "continue" outside loop | |||
|
|
|||
| [case testYieldOutsideFunction] | |||
There was a problem hiding this comment.
I think all the yield/yield from cases can go in a single test case. Having more test cases hurts the performance of the test suite.
Also, could you test some more cases:
- yield in the outermost comprehension (which is valid, because it's executed in the enclosing function's scope)
- yield in an inner comprehension or in the
ifclause
There was a problem hiding this comment.
Added tests -- is that what you had in mind?
test-data/unit/check-ignore.test
Outdated
| @@ -217,7 +217,7 @@ def f() -> None: pass | |||
| [out] | |||
|
|
|||
| [case testCannotIgnoreBlockingError] | |||
| yield # type: ignore # E: "yield" outside function | |||
| yield # type: ignore # E: 'yield' outside function | |||
There was a problem hiding this comment.
We're actually trying to standardize on double quotes in error messages. I believe it gets highlighted better.
There was a problem hiding this comment.
OK, changed back to double quotes
Fixes #10189
Produce an error when encountering a yield expression (both
yieldandyield fromclauses) in comprehensions and generator expressions. The latter is a syntax error in python 3.8+; see #10189 and also python issue 10544.Test Plan
Added
testYieldOutsideFunction(incheck-semanal-error.test).