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

Typecheck unreachable code #18707

Open
wants to merge 40 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
be1071e
Start type checking unreachable code
A5rocks Jan 24, 2025
d996918
Pass self-check
A5rocks Feb 19, 2025
07de5bf
Hacky fix to narrow unreachable literals
A5rocks Feb 19, 2025
bac268b
20% of tests
A5rocks Feb 19, 2025
2da4b7c
25% of tests
A5rocks Feb 19, 2025
ba9dab2
30% of tests
A5rocks Feb 19, 2025
7c39efa
40% of tests
A5rocks Feb 19, 2025
18642d0
50% of tests
A5rocks Feb 19, 2025
879b274
60% of tests
A5rocks Feb 19, 2025
c62093e
Pass all tests
A5rocks Feb 19, 2025
e242b5e
Narrow `isinstance` checks to `Never`
A5rocks Feb 20, 2025
69efb41
Don't treat `reveal_type(<Never>)` as a no-op
A5rocks Feb 20, 2025
f846af7
Ensure functions with typevar values don't get false positives
A5rocks Feb 21, 2025
390ade8
Ensure that `<Never>.x` is `Never`
A5rocks Feb 21, 2025
f3ba149
Ensure that `<Never>()` is `Never`
A5rocks Feb 21, 2025
92a4428
Only report unreachability once
A5rocks Feb 24, 2025
c76d422
Narrow `callable` checks to `Never`
A5rocks Feb 24, 2025
be1d524
Address CI failures
A5rocks Feb 24, 2025
13ad1cb
Narrow `match` captures to `Never`
A5rocks Feb 24, 2025
53553e7
Don't check unreachable code with partial types
A5rocks Feb 24, 2025
6cc9930
Avoid strange behavior regarding "narrowing" expressions for unreacha…
A5rocks Feb 24, 2025
4ddd34e
Unpack `Never` iterables
A5rocks Feb 24, 2025
3cda7db
Narrow disjoint narrows to `Never`
A5rocks Mar 3, 2025
6e97289
Fix `redundant-expr` for comprehensions
A5rocks Mar 3, 2025
28894ce
Narrow `issubclass` to `Never`
A5rocks Mar 3, 2025
be6573e
Narrow things based on key types even if the type is `Never`
A5rocks Mar 3, 2025
4718cfd
Update some tests
A5rocks Mar 4, 2025
475f8b1
`typing.assert_never(5)` is fine in unreachable code
A5rocks Mar 4, 2025
72349c0
Non-ambiguous `Never` is fine as inference result
A5rocks Mar 4, 2025
ca11151
Implement or-ing typemaps for `Never`-based unreachability
A5rocks Mar 6, 2025
22e73d5
Tuple length checks should narrow to `Never`
A5rocks Mar 6, 2025
2bd0802
Narrow walrus to `Never` if unreachable
A5rocks Mar 6, 2025
8fb6fab
Narrow unreachable captures due to guards in `match` to `Never`
A5rocks Mar 6, 2025
0f41d33
Narrow impossible values in mapping pattern to `Never`
A5rocks Mar 6, 2025
b641ed4
Smooth out edges
A5rocks Mar 6, 2025
dcc4f29
Propagate suppressing unreachability warnings
A5rocks Mar 7, 2025
98abde4
Fix typemap handling for ternaries
A5rocks Mar 9, 2025
390388a
Ensure `--strict-equality` works correctly with partial types
A5rocks Mar 9, 2025
dc8ed71
Support `Never` callables in plugins
A5rocks Mar 13, 2025
102ac11
Add a test about narrowing behavior
A5rocks Mar 13, 2025
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
12 changes: 12 additions & 0 deletions mypy/binder.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
TypeOfAny,
TypeType,
TypeVarType,
UninhabitedType,
UnionType,
UnpackType,
find_unpack_in_list,
Expand Down Expand Up @@ -65,6 +66,7 @@ def __init__(self, id: int, conditional_frame: bool = False) -> None:
self.unreachable = False
self.conditional_frame = conditional_frame
self.suppress_unreachable_warnings = False
self.unreachable_warning_emitted = False

def __repr__(self) -> str:
return f"Frame({self.id}, {self.types}, {self.unreachable}, {self.conditional_frame})"
Expand Down Expand Up @@ -166,6 +168,10 @@ def put(self, expr: Expression, typ: Type, *, from_assignment: bool = True) -> N

This is used for isinstance() etc. Assignments should go through assign_type().
"""
proper_typ = get_proper_type(typ)
if isinstance(proper_typ, UninhabitedType) and not proper_typ.ambiguous:
self.frames[-1].unreachable = True

if not isinstance(expr, (IndexExpr, MemberExpr, NameExpr)):
return
if not literal(expr):
Expand All @@ -183,6 +189,9 @@ def unreachable(self) -> None:
def suppress_unreachable_warnings(self) -> None:
self.frames[-1].suppress_unreachable_warnings = True

def emitted_unreachable_warning(self) -> None:
self.frames[-1].unreachable_warning_emitted = True

def get(self, expr: Expression) -> Type | None:
key = literal_hash(expr)
assert key is not None, "Internal error: binder tried to get non-literal"
Expand All @@ -199,6 +208,9 @@ def is_unreachable(self) -> bool:
def is_unreachable_warning_suppressed(self) -> bool:
return any(f.suppress_unreachable_warnings for f in self.frames)

def is_unreachable_warning_emitted(self) -> bool:
return any(f.unreachable_warning_emitted for f in self.frames)

def cleanse(self, expr: Expression) -> None:
"""Remove all references to a Node from the binder."""
key = literal_hash(expr)
Expand Down
223 changes: 117 additions & 106 deletions mypy/checker.py

Large diffs are not rendered by default.

72 changes: 56 additions & 16 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -1638,6 +1638,9 @@ def check_call(
object_type,
original_type=callee,
)
elif isinstance(callee, UninhabitedType):
self.infer_arg_types_in_empty_context(args)
return (UninhabitedType(), UninhabitedType())
else:
return self.msg.not_callable(callee, context), AnyType(TypeOfAny.from_error)

Expand Down Expand Up @@ -1795,9 +1798,20 @@ def check_callable_call(
callable_name,
)

self.check_argument_types(
arg_types, arg_kinds, args, callee, formal_to_actual, context, object_type=object_type
)
if not (
isinstance(callable_node, RefExpr)
and callable_node.fullname in ("typing.assert_never", "typing_extensions.assert_never")
and self.chk.binder.is_unreachable()
):
self.check_argument_types(
arg_types,
arg_kinds,
args,
callee,
formal_to_actual,
context,
object_type=object_type,
)

if (
callee.is_type_obj()
Expand Down Expand Up @@ -4273,34 +4287,41 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type:
elif e.op == "or":
left_map, right_map = self.chk.find_isinstance_check(e.left)

left_impossible = left_map is None or any(
isinstance(get_proper_type(v), UninhabitedType) for v in left_map.values()
)
right_impossible = right_map is None or any(
isinstance(get_proper_type(v), UninhabitedType) for v in right_map.values()
)

# If left_map is None then we know mypy considers the left expression
# to be redundant.
if (
codes.REDUNDANT_EXPR in self.chk.options.enabled_error_codes
and left_map is None
and left_impossible
# don't report an error if it's intentional
and not e.right_always
):
self.msg.redundant_left_operand(e.op, e.left)

if (
self.chk.should_report_unreachable_issues()
and right_map is None
and right_impossible
# don't report an error if it's intentional
and not e.right_unreachable
):
self.msg.unreachable_right_operand(e.op, e.right)

right_type = self.analyze_cond_branch(right_map, e.right, expanded_left_type)

if left_map is None and right_map is None:
if left_impossible and right_impossible:
return UninhabitedType()

if right_map is None:
if right_impossible:
# The boolean expression is statically known to be the left value
assert left_map is not None
return left_type
if left_map is None:
if left_impossible:
# The boolean expression is statically known to be the right value
assert right_map is not None
return right_type
Expand Down Expand Up @@ -5798,9 +5819,13 @@ def check_for_comp(self, e: GeneratorExpr | DictionaryComprehension) -> None:
self.chk.push_type_map(true_map)

if codes.REDUNDANT_EXPR in self.chk.options.enabled_error_codes:
if true_map is None:
if true_map is None or any(
isinstance(get_proper_type(t), UninhabitedType) for t in true_map.values()
):
self.msg.redundant_condition_in_comprehension(False, condition)
elif false_map is None:
elif false_map is None or any(
isinstance(get_proper_type(t), UninhabitedType) for t in false_map.values()
):
self.msg.redundant_condition_in_comprehension(True, condition)

def visit_conditional_expr(self, e: ConditionalExpr, allow_none_return: bool = False) -> Type:
Expand All @@ -5811,9 +5836,13 @@ def visit_conditional_expr(self, e: ConditionalExpr, allow_none_return: bool = F
# but only for the current expression
if_map, else_map = self.chk.find_isinstance_check(e.cond)
if codes.REDUNDANT_EXPR in self.chk.options.enabled_error_codes:
if if_map is None:
if if_map is None or any(
isinstance(get_proper_type(t), UninhabitedType) for t in if_map.values()
):
self.msg.redundant_condition_in_if(False, e.cond)
elif else_map is None:
elif else_map is None or any(
isinstance(get_proper_type(t), UninhabitedType) for t in else_map.values()
):
self.msg.redundant_condition_in_if(True, e.cond)

if_type = self.analyze_cond_branch(
Expand Down Expand Up @@ -5890,17 +5919,28 @@ def analyze_cond_branch(
node: Expression,
context: Type | None,
allow_none_return: bool = False,
suppress_unreachable_errors: bool = True,
suppress_unreachable_errors: bool | None = None,
) -> Type:
# TODO: default based on flag (default to `True` if flag is not passed)
unreachable_errors_suppressed = (
suppress_unreachable_errors
if suppress_unreachable_errors is not None
else self.chk.binder.is_unreachable_warning_suppressed()
)
with self.chk.binder.frame_context(can_skip=True, fall_through=0):
if map is None:
self.chk.push_type_map(map)

if map is None or any(
isinstance(get_proper_type(t), UninhabitedType) for t in map.values()
):
# We still need to type check node, in case we want to
# process it for isinstance checks later. Since the branch was
# determined to be unreachable, any errors should be suppressed.
with self.msg.filter_errors(filter_errors=suppress_unreachable_errors):

with self.msg.filter_errors(filter_errors=unreachable_errors_suppressed):
self.accept(node, type_context=context, allow_none_return=allow_none_return)
return UninhabitedType()
self.chk.push_type_map(map)

return self.accept(node, type_context=context, allow_none_return=allow_none_return)

#
Expand Down
3 changes: 3 additions & 0 deletions mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
TypeVarLikeType,
TypeVarTupleType,
TypeVarType,
UninhabitedType,
UnionType,
get_proper_type,
)
Expand Down Expand Up @@ -253,6 +254,8 @@ def _analyze_member_access(
elif isinstance(typ, DeletedType):
mx.msg.deleted_as_rvalue(typ, mx.context)
return AnyType(TypeOfAny.from_error)
elif isinstance(typ, UninhabitedType):
return UninhabitedType()
return report_missing_attribute(mx.original_type, typ, name, mx)


Expand Down
23 changes: 11 additions & 12 deletions mypy/checkpattern.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,11 @@ def visit_as_pattern(self, o: AsPattern) -> PatternType:
else:
typ, rest_type, type_map = current_type, UninhabitedType(), {}

if not is_uninhabited(typ) and o.name is not None:
if o.name is not None:
typ, _ = self.chk.conditional_types_with_intersection(
current_type, [get_type_range(typ)], o, default=current_type
)
if not is_uninhabited(typ):
type_map[o.name] = typ
type_map[o.name] = typ

return PatternType(typ, rest_type, type_map)

Expand Down Expand Up @@ -470,14 +469,14 @@ def visit_mapping_pattern(self, o: MappingPattern) -> PatternType:
captures: dict[Expression, Type] = {}
for key, value in zip(o.keys, o.values):
inner_type = self.get_mapping_item_type(o, current_type, key)
if inner_type is None:
if is_uninhabited(inner_type):
can_match = False
inner_type = self.chk.named_type("builtins.object")

pattern_type = self.accept(value, inner_type)
if is_uninhabited(pattern_type.type):
can_match = False
else:
self.update_type_map(captures, pattern_type.captures)

self.update_type_map(captures, pattern_type.captures)

if o.rest is not None:
mapping = self.chk.named_type("typing.Mapping")
Expand All @@ -502,13 +501,13 @@ def visit_mapping_pattern(self, o: MappingPattern) -> PatternType:

def get_mapping_item_type(
self, pattern: MappingPattern, mapping_type: Type, key: Expression
) -> Type | None:
) -> Type:
mapping_type = get_proper_type(mapping_type)
if isinstance(mapping_type, TypedDictType):
with self.msg.filter_errors() as local_errors:
result: Type | None = self.chk.expr_checker.visit_typeddict_index_expr(
mapping_type, key
)[0]
result: Type = self.chk.expr_checker.visit_typeddict_index_expr(mapping_type, key)[
0
]
has_local_errors = local_errors.has_new_errors()
# If we can't determine the type statically fall back to treating it as a normal
# mapping
Expand All @@ -517,7 +516,7 @@ def get_mapping_item_type(
result = self.get_simple_mapping_item_type(pattern, mapping_type, key)

if local_errors.has_new_errors():
result = None
result = UninhabitedType()
else:
with self.msg.filter_errors():
result = self.get_simple_mapping_item_type(pattern, mapping_type, key)
Expand Down
9 changes: 5 additions & 4 deletions mypyc/ir/pprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,15 +222,16 @@ def visit_call_c(self, op: CallC) -> str:
def visit_primitive_op(self, op: PrimitiveOp) -> str:
args = []
arg_index = 0
type_arg_index = 0
# type_arg_index = 0
for arg_type in zip(op.desc.arg_types):
if arg_type:
args.append(self.format("%r", op.args[arg_index]))
arg_index += 1
else:
assert op.type_args
args.append(self.format("%r", op.type_args[type_arg_index]))
type_arg_index += 1
assert False
# assert op.type_args
# args.append(self.format("%r", op.type_args[type_arg_index]))
# type_arg_index += 1

args_str = ", ".join(args)
if op.is_void:
Expand Down
14 changes: 4 additions & 10 deletions mypyc/test-data/irbuild-match.test
Original file line number Diff line number Diff line change
Expand Up @@ -1765,10 +1765,8 @@ def f(x):
r10 :: object
r11 :: object[1]
r12 :: object_ptr
r13, r14 :: object
r15 :: i32
r16 :: bit
r17, r18 :: bool
r13 :: object
r14 :: bool
L0:
r0 = __main__.Foo :: type
r1 = PyObject_IsInstance(x, r0)
Expand All @@ -1792,13 +1790,9 @@ L2:
goto L8
L3:
L4:
r14 = box(bool, 0)
r15 = PyObject_IsTrue(r14)
r16 = r15 >= 0 :: signed
r17 = truncate r15: i32 to builtins.bool
if r17 goto L6 else goto L5 :: bool
if 0 goto L6 else goto L5 :: bool
L5:
r18 = raise AssertionError('Unreachable')
r14 = raise AssertionError('Unreachable')
unreachable
L6:
goto L8
Expand Down
6 changes: 0 additions & 6 deletions mypyc/test-data/run-singledispatch.test
Original file line number Diff line number Diff line change
Expand Up @@ -347,11 +347,6 @@ def verify_typeinfo(stub: TypeInfo, a: MaybeMissing[Type[Any]], b: List[str]) ->
yield Error('in TypeInfo')
yield Error('hello')

@verify.register(TypeVarExpr)
def verify_typevarexpr(stub: TypeVarExpr, a: MaybeMissing[Any], b: List[str]) -> Iterator[Error]:
if False:
yield None

def verify_list(stub, a, b) -> List[str]:
"""Helper function that converts iterator of errors to list of messages"""
return list(err.msg for err in verify(stub, a, b))
Expand All @@ -361,7 +356,6 @@ def test_verify() -> None:
assert verify_list(MypyFile(), MISSING, ['a', 'b']) == ["shouldn't be missing"]
assert verify_list(MypyFile(), 5, ['a', 'b']) == ['in TypeInfo', 'hello']
assert verify_list(TypeInfo(), str, ['a', 'b']) == ['in TypeInfo', 'hello']
assert verify_list(TypeVarExpr(), 'a', ['x', 'y']) == []


[case testArgsInRegisteredImplNamedDifferentlyFromMainFunction]
Expand Down
Loading