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

Fix crash with yield in comprehension #12048

Merged
merged 4 commits into from
Jan 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 20 additions & 16 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -3830,13 +3830,15 @@ def visit_star_expr(self, expr: StarExpr) -> None:
expr.expr.accept(self)

def visit_yield_from_expr(self, e: YieldFromExpr) -> None:
if not self.is_func_scope(): # not sure
if not self.is_func_scope():
self.fail('"yield from" outside function', e, serious=True, blocker=True)
elif self.is_comprehension_stack[-1]:
self.fail('"yield from" inside comprehension or generator expression',
e, serious=True, blocker=True)
elif self.function_stack[-1].is_coroutine:
self.fail('"yield from" in async function', e, serious=True, blocker=True)
else:
if self.function_stack[-1].is_coroutine:
self.fail('"yield from" in async function', e, serious=True, blocker=True)
else:
self.function_stack[-1].is_generator = True
self.function_stack[-1].is_generator = True
if e.expr:
e.expr.accept(self)

Expand Down Expand Up @@ -4214,20 +4216,22 @@ def visit__promote_expr(self, expr: PromoteExpr) -> None:
if analyzed is not None:
expr.type = analyzed

def visit_yield_expr(self, expr: YieldExpr) -> None:
def visit_yield_expr(self, e: YieldExpr) -> None:
if not self.is_func_scope():
self.fail('"yield" outside function', expr, serious=True, blocker=True)
else:
if self.function_stack[-1].is_coroutine:
if self.options.python_version < (3, 6):
self.fail('"yield" in async function', expr, serious=True, blocker=True)
else:
self.function_stack[-1].is_generator = True
self.function_stack[-1].is_async_generator = True
self.fail('"yield" outside function', e, serious=True, blocker=True)
elif self.is_comprehension_stack[-1]:
self.fail('"yield" inside comprehension or generator expression',
e, serious=True, blocker=True)
elif self.function_stack[-1].is_coroutine:
if self.options.python_version < (3, 6):
self.fail('"yield" in async function', e, serious=True, blocker=True)
else:
self.function_stack[-1].is_generator = True
if expr.expr:
expr.expr.accept(self)
self.function_stack[-1].is_async_generator = True
else:
self.function_stack[-1].is_generator = True
if e.expr:
e.expr.accept(self)

def visit_await_expr(self, expr: AwaitExpr) -> None:
if not self.is_func_scope():
Expand Down
2 changes: 1 addition & 1 deletion mypy/semanal_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ def semantic_analyze_target(target: str,
Return tuple with these items:
- list of deferred targets
- was some definition incomplete (need to run another pass)
- were any new names were defined (or placeholders replaced)
- were any new names defined (or placeholders replaced)
"""
state.manager.processed_targets.append(target)
tree = state.tree
Expand Down
41 changes: 39 additions & 2 deletions test-data/unit/check-semanal-error.test
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,47 @@ continue # E: "continue" outside loop

[case testYieldOutsideFunction]
Copy link
Member

Choose a reason for hiding this comment

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

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 if clause

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added tests -- is that what you had in mind?

yield # E: "yield" outside function

[case testYieldFromOutsideFunction]
x = 1
yield from x # E: "yield from" outside function
[(yield 1) for _ in x] # E: "yield" inside comprehension or generator expression
{(yield 1) for _ in x} # E: "yield" inside comprehension or generator expression
{i: (yield 1) for i in x} # E: "yield" inside comprehension or generator expression
((yield 1) for _ in x) # E: "yield" inside comprehension or generator expression
y = 1
[(yield from x) for _ in y] # E: "yield from" inside comprehension or generator expression
{(yield from x) for _ in y} # E: "yield from" inside comprehension or generator expression
{i: (yield from x) for i in y} # E: "yield from" inside comprehension or generator expression
((yield from x) for _ in y) # E: "yield from" inside comprehension or generator expression
def f(y):
[x for x in (yield y)]
{x for x in (yield y)}
{x: x for x in (yield y)}
(x for x in (yield y))
[x for x in (yield from y)]
{x for x in (yield from y)}
{x: x for x in (yield from y)}
(x for x in (yield from y))
def g(y):
[(yield 1) for _ in y] # E: "yield" inside comprehension or generator expression
{(yield 1) for _ in y} # E: "yield" inside comprehension or generator expression
{i: (yield 1) for i in y} # E: "yield" inside comprehension or generator expression
((yield 1) for _ in y) # E: "yield" inside comprehension or generator expression
lst = 1
[(yield from lst) for _ in y] # E: "yield from" inside comprehension or generator expression
{(yield from lst) for _ in y} # E: "yield from" inside comprehension or generator expression
{i: (yield from lst) for i in y} # E: "yield from" inside comprehension or generator expression
((yield from lst) for _ in y) # E: "yield from" inside comprehension or generator expression
def h(y):
lst = 1
[x for x in lst if (yield y)] # E: "yield" inside comprehension or generator expression
{x for x in lst if (yield y)} # E: "yield" inside comprehension or generator expression
{x: x for x in lst if (yield y)} # E: "yield" inside comprehension or generator expression
(x for x in lst if (yield y)) # E: "yield" inside comprehension or generator expression
lst = 1
[x for x in lst if (yield from y)] # E: "yield from" inside comprehension or generator expression
{x for x in lst if (yield from y)} # E: "yield from" inside comprehension or generator expression
{x: x for x in lst if (yield from y)} # E: "yield from" inside comprehension or generator expression
(x for x in lst if (yield from y)) # E: "yield from" inside comprehension or generator expression

[case testImportFuncDup]

Expand Down