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

Use union types instead of join in binder #18538

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
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
101 changes: 45 additions & 56 deletions mypy/binder.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
from typing_extensions import TypeAlias as _TypeAlias

from mypy.erasetype import remove_instance_last_known_values
from mypy.join import join_simple
from mypy.literals import Key, literal, literal_hash, subkeys
from mypy.nodes import Expression, IndexExpr, MemberExpr, NameExpr, RefExpr, TypeInfo, Var
from mypy.subtypes import is_same_type, is_subtype
from mypy.typeops import make_simplified_union
from mypy.types import (
AnyType,
Instance,
Expand All @@ -21,6 +21,7 @@
Type,
TypeOfAny,
TypeType,
TypeVarType,
UnionType,
UnpackType,
find_unpack_in_list,
Expand Down Expand Up @@ -237,9 +238,21 @@ def update_from_options(self, frames: list[Frame]) -> bool:
):
type = AnyType(TypeOfAny.from_another_any, source_any=declaration_type)
else:
for other in resulting_values[1:]:
assert other is not None
type = join_simple(self.declarations[key], type, other.type)
possible_types = []
for t in resulting_values:
assert t is not None
possible_types.append(t.type)
if len(possible_types) == 1:
# This is to avoid calling get_proper_type() unless needed, as this may
# interfere with our (hacky) TypeGuard support.
type = possible_types[0]
else:
type = make_simplified_union(possible_types)
# Legacy guard for corner case when the original type is TypeVarType.
if isinstance(declaration_type, TypeVarType) and not is_subtype(
type, declaration_type
):
type = declaration_type
# Try simplifying resulting type for unions involving variadic tuples.
# Technically, everything is still valid without this step, but if we do
# not do this, this may create long unions after exiting an if check like:
Expand All @@ -258,7 +271,7 @@ def update_from_options(self, frames: list[Frame]) -> bool:
)
if simplified == self.declarations[key]:
type = simplified
if current_value is None or not is_same_type(type, current_value[0]):
if current_value is None or not is_same_type(type, current_value.type):
self._put(key, type, from_assignment=True)
changed = True

Expand Down Expand Up @@ -300,9 +313,7 @@ def accumulate_type_assignments(self) -> Iterator[Assigns]:
yield self.type_assignments
self.type_assignments = old_assignments

def assign_type(
self, expr: Expression, type: Type, declared_type: Type | None, restrict_any: bool = False
) -> None:
def assign_type(self, expr: Expression, type: Type, declared_type: Type | None) -> None:
# We should erase last known value in binder, because if we are using it,
# it means that the target is not final, and therefore can't hold a literal.
type = remove_instance_last_known_values(type)
Expand Down Expand Up @@ -333,41 +344,32 @@ def assign_type(

p_declared = get_proper_type(declared_type)
p_type = get_proper_type(type)
enclosing_type = get_proper_type(self.most_recent_enclosing_type(expr, type))
if isinstance(enclosing_type, AnyType) and not restrict_any:
# If x is Any and y is int, after x = y we do not infer that x is int.
# This could be changed.
# Instead, since we narrowed type from Any in a recent frame (probably an
# isinstance check), but now it is reassigned, we broaden back
# to Any (which is the most recent enclosing type)
self.put(expr, enclosing_type)
# As a special case, when assigning Any to a variable with a
# declared Optional type that has been narrowed to None,
# replace all the Nones in the declared Union type with Any.
# This overrides the normal behavior of ignoring Any assignments to variables
# in order to prevent false positives.
# (See discussion in #3526)
elif (
isinstance(p_type, AnyType)
and isinstance(p_declared, UnionType)
and any(isinstance(get_proper_type(item), NoneType) for item in p_declared.items)
and isinstance(
get_proper_type(self.most_recent_enclosing_type(expr, NoneType())), NoneType
)
):
# Replace any Nones in the union type with Any
new_items = [
type if isinstance(get_proper_type(item), NoneType) else item
for item in p_declared.items
]
self.put(expr, UnionType(new_items))
elif isinstance(p_type, AnyType) and not (
isinstance(p_declared, UnionType)
and any(isinstance(get_proper_type(item), AnyType) for item in p_declared.items)
):
# Assigning an Any value doesn't affect the type to avoid false negatives, unless
# there is an Any item in a declared union type.
self.put(expr, declared_type)
if isinstance(p_type, AnyType):
# Any type requires some special casing, for both historical reasons,
# and to optimise user experience without sacrificing correctness too much.
if isinstance(expr, RefExpr) and isinstance(expr.node, Var) and expr.node.is_inferred:
# First case: a local/global variable without explicit annotation,
# in this case we just assign Any (essentially following the SSA logic).
self.put(expr, type)
elif isinstance(p_declared, UnionType) and any(
isinstance(get_proper_type(item), NoneType) for item in p_declared.items
):
# Second case: explicit optional type, in this case we optimize for a common
# pattern when an untyped value used as a fallback replacing None.
new_items = [
type if isinstance(get_proper_type(item), NoneType) else item
for item in p_declared.items
]
self.put(expr, UnionType(new_items))
elif isinstance(p_declared, UnionType) and any(
isinstance(get_proper_type(item), AnyType) for item in p_declared.items
):
# Third case: a union already containing Any (most likely from an un-imported
# name), in this case we allow assigning Any as well.
self.put(expr, type)
else:
# In all other cases we don't narrow to Any to minimize false negatives.
self.put(expr, declared_type)
else:
self.put(expr, type)

Expand All @@ -389,19 +391,6 @@ def invalidate_dependencies(self, expr: BindableExpression) -> None:
for dep in self.dependencies.get(key, set()):
self._cleanse_key(dep)

def most_recent_enclosing_type(self, expr: BindableExpression, type: Type) -> Type | None:
type = get_proper_type(type)
if isinstance(type, AnyType):
return get_declaration(expr)
key = literal_hash(expr)
assert key is not None
enclosers = [get_declaration(expr)] + [
f.types[key].type
for f in self.frames
if key in f.types and is_subtype(type, f.types[key][0])
]
return enclosers[-1]

def allow_jump(self, index: int) -> None:
# self.frames and self.options_on_return have different lengths
# so make sure the index is positive
Expand Down
7 changes: 3 additions & 4 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -3282,7 +3282,7 @@ def check_assignment(
if rvalue_type and infer_lvalue_type and not isinstance(lvalue_type, PartialType):
# Don't use type binder for definitions of special forms, like named tuples.
if not (isinstance(lvalue, NameExpr) and lvalue.is_special_form):
self.binder.assign_type(lvalue, rvalue_type, lvalue_type, False)
self.binder.assign_type(lvalue, rvalue_type, lvalue_type)
if (
isinstance(lvalue, NameExpr)
and isinstance(lvalue.node, Var)
Expand Down Expand Up @@ -4021,7 +4021,7 @@ def check_multi_assignment_from_union(
if isinstance(expr, StarExpr):
expr = expr.expr

# TODO: See todo in binder.py, ConditionalTypeBinder.assign_type
# TODO: See comment in binder.py, ConditionalTypeBinder.assign_type
# It's unclear why the 'declared_type' param is sometimes 'None'
clean_items: list[tuple[Type, Type]] = []
for type, declared_type in items:
Expand All @@ -4033,7 +4033,6 @@ def check_multi_assignment_from_union(
expr,
make_simplified_union(list(types)),
make_simplified_union(list(declared_types)),
False,
)
for union, lv in zip(union_types, self.flatten_lvalues(lvalues)):
# Properly store the inferred types.
Expand Down Expand Up @@ -5231,7 +5230,7 @@ def visit_del_stmt(self, s: DelStmt) -> None:
for elt in flatten(s.expr):
if isinstance(elt, NameExpr):
self.binder.assign_type(
elt, DeletedType(source=elt.name), get_declaration(elt), False
elt, DeletedType(source=elt.name), get_declaration(elt)
)

def visit_decorator(self, e: Decorator) -> None:
Expand Down
4 changes: 3 additions & 1 deletion mypy/fastparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -1106,7 +1106,9 @@ def make_argument(
if argument_elide_name(arg.arg):
pos_only = True

argument = Argument(Var(arg.arg, arg_type), arg_type, self.visit(default), kind, pos_only)
var = Var(arg.arg, arg_type)
var.is_inferred = False
argument = Argument(var, arg_type, self.visit(default), kind, pos_only)
argument.set_line(
arg.lineno,
arg.col_offset,
Expand Down
49 changes: 0 additions & 49 deletions mypy/join.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,55 +183,6 @@ def join_instances_via_supertype(self, t: Instance, s: Instance) -> ProperType:
return best


def join_simple(declaration: Type | None, s: Type, t: Type) -> ProperType:
"""Return a simple least upper bound given the declared type.

This function should be only used by binder, and should not recurse.
For all other uses, use `join_types()`.
"""
declaration = get_proper_type(declaration)
s = get_proper_type(s)
t = get_proper_type(t)

if (s.can_be_true, s.can_be_false) != (t.can_be_true, t.can_be_false):
# if types are restricted in different ways, use the more general versions
s = mypy.typeops.true_or_false(s)
t = mypy.typeops.true_or_false(t)

if isinstance(s, AnyType):
return s

if isinstance(s, ErasedType):
return t

if is_proper_subtype(s, t, ignore_promotions=True):
return t

if is_proper_subtype(t, s, ignore_promotions=True):
return s

if isinstance(declaration, UnionType):
return mypy.typeops.make_simplified_union([s, t])

if isinstance(s, NoneType) and not isinstance(t, NoneType):
s, t = t, s

if isinstance(s, UninhabitedType) and not isinstance(t, UninhabitedType):
s, t = t, s

# Meets/joins require callable type normalization.
s, t = normalize_callables(s, t)

if isinstance(s, UnionType) and not isinstance(t, UnionType):
s, t = t, s

value = t.accept(TypeJoinVisitor(s))
if declaration is None or is_subtype(value, declaration):
return value

return declaration


def trivial_join(s: Type, t: Type) -> Type:
"""Return one of types (expanded) if it is a supertype of other, otherwise top type."""
if is_subtype(s, t):
Expand Down
10 changes: 5 additions & 5 deletions mypy/test/testtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from mypy.erasetype import erase_type, remove_instance_last_known_values
from mypy.indirection import TypeIndirectionVisitor
from mypy.join import join_simple, join_types
from mypy.join import join_types
from mypy.meet import meet_types, narrow_declared_type
from mypy.nodes import (
ARG_NAMED,
Expand Down Expand Up @@ -817,12 +817,12 @@ def test_any_type(self) -> None:
self.assert_join(t, self.fx.anyt, self.fx.anyt)

def test_mixed_truth_restricted_type_simple(self) -> None:
# join_simple against differently restricted truthiness types drops restrictions.
# make_simplified_union against differently restricted truthiness types drops restrictions.
true_a = true_only(self.fx.a)
false_o = false_only(self.fx.o)
j = join_simple(self.fx.o, true_a, false_o)
assert j.can_be_true
assert j.can_be_false
u = make_simplified_union([true_a, false_o])
assert u.can_be_true
assert u.can_be_false

def test_mixed_truth_restricted_type(self) -> None:
# join_types against differently restricted truthiness types drops restrictions.
Expand Down
9 changes: 0 additions & 9 deletions mypyc/test-data/irbuild-any.test
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ def f(a: Any, n: int, c: C) -> None:
c.n = a
a = n
n = a
a.a = n
[out]
def f(a, n, c):
a :: object
Expand All @@ -49,10 +48,6 @@ def f(a, n, c):
r3 :: bool
r4 :: object
r5 :: int
r6 :: str
r7 :: object
r8 :: i32
r9 :: bit
L0:
r0 = box(int, n)
c.a = r0; r1 = is_error
Expand All @@ -62,10 +57,6 @@ L0:
a = r4
r5 = unbox(int, a)
n = r5
r6 = 'a'
r7 = box(int, n)
r8 = PyObject_SetAttr(a, r6, r7)
r9 = r8 >= 0 :: signed
return 1

[case testCoerceAnyInOps]
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -3709,7 +3709,7 @@ def new(uc: Type[U]) -> U:
if 1:
u = uc(0)
u.foo()
u = uc('') # Error
uc('') # Error
u.foo(0) # Error
return uc()
u = new(User)
Expand Down
6 changes: 4 additions & 2 deletions test-data/unit/check-dynamic-typing.test
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,10 @@ d = None # All ok
d = t
d = g
d = A
t = d
f = d

d1: Any
t = d1
f = d1
[builtins fixtures/tuple.pyi]


Expand Down
9 changes: 4 additions & 5 deletions test-data/unit/check-isinstance.test
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ if int():
x = B()
x.z
x = foo()
x.z # E: "A" has no attribute "z"
x.y
reveal_type(x) # N: Revealed type is "Any"
reveal_type(x) # N: Revealed type is "__main__.A"

[case testSingleMultiAssignment]
x = 'a'
Expand Down Expand Up @@ -1919,13 +1919,12 @@ if isinstance(x, str, 1): # E: Too many arguments for "isinstance"
from typing import Any

def narrow_any_to_str_then_reassign_to_int() -> None:
v = 1 # type: Any
v: Any = 1

if isinstance(v, str):
reveal_type(v) # N: Revealed type is "builtins.str"
v = 2
reveal_type(v) # N: Revealed type is "Any"

reveal_type(v) # N: Revealed type is "builtins.int"
[builtins fixtures/isinstance.pyi]

[case testNarrowTypeAfterInList]
Expand Down
18 changes: 18 additions & 0 deletions test-data/unit/check-narrowing.test
Original file line number Diff line number Diff line change
Expand Up @@ -2416,3 +2416,21 @@ while x is not None and b():
x = f()

[builtins fixtures/primitives.pyi]

[case testNarrowingTypeVarMultiple]
from typing import TypeVar

class A: ...
class B: ...

T = TypeVar("T")
def foo(x: T) -> T:
if isinstance(x, A):
pass
elif isinstance(x, B):
pass
else:
raise
reveal_type(x) # N: Revealed type is "T`-1"
return x
[builtins fixtures/isinstance.pyi]
Loading