Skip to content

Commit

Permalink
Improve unification of conditional expressions when the left branch h…
Browse files Browse the repository at this point in the history
…as an empty list/set/dict.

Fix #1094.
  • Loading branch information
Guido van Rossum committed Apr 18, 2016
1 parent f4de7a5 commit 1d2a30d
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 18 deletions.
68 changes: 50 additions & 18 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from mypy import messages
from mypy.infer import infer_type_arguments, infer_function_type_arguments
from mypy import join
from mypy.subtypes import is_subtype
from mypy.subtypes import is_subtype, is_equivalent
from mypy import applytype
from mypy import erasetype
from mypy.checkmember import analyze_member_access, type_object_type
Expand Down Expand Up @@ -1403,6 +1403,7 @@ def check_for_comp(self, e: Union[GeneratorExpr, DictionaryComprehension]) -> No
def visit_conditional_expr(self, e: ConditionalExpr) -> Type:
cond_type = self.accept(e.cond)
self.check_not_void(cond_type, e)
ctx = self.chk.type_context[-1]

# Gain type information from isinstance if it is there
# but only for the current expression
Expand All @@ -1411,26 +1412,44 @@ def visit_conditional_expr(self, e: ConditionalExpr) -> Type:
self.chk.type_map,
self.chk.typing_mode_weak())

self.chk.binder.push_frame()

if if_map:
for var, type in if_map.items():
self.chk.binder.push(var, type)

if_type = self.accept(e.if_expr)

self.chk.binder.pop_frame()
self.chk.binder.push_frame()

if else_map:
for var, type in else_map.items():
self.chk.binder.push(var, type)
with self.chk.binder:
if if_map:
for var, type in if_map.items():
self.chk.binder.push(var, type)
if_type = self.accept(e.if_expr, context=ctx)

if has_unfinished_types(if_type):
# Analyze the right branch disregarding the left branch.
with self.chk.binder:
if else_map:
for var, type in else_map.items():
self.chk.binder.push(var, type)
else_type = self.accept(e.else_expr, context=ctx)

# If it would make a difference, re-analyze the left
# branch using the right branch's type as context.
if ctx is None or not is_equivalent(else_type, ctx):
# TODO: If it's possible that the previous analysis of
# the left branch produced errors that are avoided
# using this context, suppress those errors.
with self.chk.binder:
if if_map:
for var, type in if_map.items():
self.chk.binder.push(var, type)
if_type = self.accept(e.if_expr, context=else_type)

else_type = self.accept(e.else_expr, context=if_type)
else:
# Analyze the right branch in the context of the left
# branch's type.
with self.chk.binder:
if else_map:
for var, type in else_map.items():
self.chk.binder.push(var, type)
else_type = self.accept(e.else_expr, context=if_type)

self.chk.binder.pop_frame()
res = join.join_types(if_type, else_type)

return join.join_types(if_type, else_type)
return res

def visit_backquote_expr(self, e: BackquoteExpr) -> Type:
self.accept(e.expr)
Expand Down Expand Up @@ -1502,6 +1521,19 @@ def not_ready_callback(self, name: str, context: Context) -> None:
self.chk.handle_cannot_determine_type(name, context)


# TODO: What's a good name for this function?
def has_unfinished_types(t: Type) -> bool:
"""Check whether t has type variables replaced with 'None'.
This can happen when `[]` is evaluated without sufficient context.
"""
if isinstance(t, NoneTyp):
return True
if isinstance(t, Instance):
return any(has_unfinished_types(arg) for arg in t.args)
return False


def map_actuals_to_formals(caller_kinds: List[int],
caller_names: List[str],
callee_kinds: List[int],
Expand Down
57 changes: 57 additions & 0 deletions mypy/test/data/check-inference.test
Original file line number Diff line number Diff line change
Expand Up @@ -1639,3 +1639,60 @@ a1 = D1() if f() else D2()
a1.foo1()
a2 = D2() if f() else D1()
a2.foo2()

[case testUnificationEmptyListLeft]
def f(): pass
a = [] if f() else [0]
a.append(0)
[builtins fixtures/list.py]

[case testUnificationEmptyListRight]
def f(): pass
a = [0] if f() else []
a.append(0)
[builtins fixtures/list.py]

[case testUnificationEmptyListLeftInContext]
from typing import List
def f(): pass
a = [] if f() else [0] # type: List[int]
a.append(0)
[builtins fixtures/list.py]

[case testUnificationEmptyListRightInContext]
# TODO Find an example that really needs the context
from typing import List
def f(): pass
a = [0] if f() else [] # type: List[int]
a.append(0)
[builtins fixtures/list.py]

[case testUnificationEmptySetLeft]
def f(): pass
a = set() if f() else {0}
a.add(0)
[builtins fixtures/set.py]

[case testUnificationEmptyDictLeft]
def f(): pass
a = {} if f() else {0: 0}
a.update({0: 0})
[builtins fixtures/dict.py]

[case testUnificationEmptyDictRight]
def f(): pass
a = {0: 0} if f() else {}
a.update({0: 0})
[builtins fixtures/dict.py]

[case testUnificationDictWithEmptyListLeft]
def f(): pass
a = {0: []} if f() else {0: [0]}
a.update({0: [0]})
[builtins fixtures/dict.py]

[case testUnificationDictWithEmptyListRight]
def f(): pass
a = {0: [0]} if f() else {0: []}
a.update({0: [0]})
[builtins fixtures/dict.py]

0 comments on commit 1d2a30d

Please sign in to comment.