Skip to content

Commit 3d6ecd0

Browse files
ammaraskarA5rocks
andcommitted
Use A5rock's approach of marking semanal unreachability.
Co-authored-by: A5rocks <[email protected]>
1 parent 51756c0 commit 3d6ecd0

File tree

5 files changed

+38
-32
lines changed

5 files changed

+38
-32
lines changed

mypy/binder.py

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import enum
34
from collections import defaultdict
45
from collections.abc import Iterator
56
from contextlib import contextmanager
@@ -36,6 +37,11 @@ class CurrentType(NamedTuple):
3637
from_assignment: bool
3738

3839

40+
class UnreachableType(enum.Enum):
41+
BINDER_UNREACHABLE = enum.auto()
42+
SEMANAL_UNREACHABLE = enum.auto()
43+
44+
3945
class Frame:
4046
"""A Frame represents a specific point in the execution of a program.
4147
It carries information about the current types of expressions at
@@ -51,7 +57,7 @@ class Frame:
5157
def __init__(self, id: int, conditional_frame: bool = False) -> None:
5258
self.id = id
5359
self.types: dict[Key, CurrentType] = {}
54-
self.unreachable = False
60+
self.unreachable: UnreachableType | None = None
5561
self.conditional_frame = conditional_frame
5662
self.suppress_unreachable_warnings = False
5763

@@ -161,8 +167,11 @@ def put(self, expr: Expression, typ: Type, *, from_assignment: bool = True) -> N
161167
self._add_dependencies(key)
162168
self._put(key, typ, from_assignment)
163169

164-
def unreachable(self) -> None:
165-
self.frames[-1].unreachable = True
170+
def unreachable(self, from_semanal: bool = False) -> None:
171+
unreachable_type = UnreachableType.BINDER_UNREACHABLE
172+
if from_semanal:
173+
unreachable_type = UnreachableType.SEMANAL_UNREACHABLE
174+
self.frames[-1].unreachable = unreachable_type
166175

167176
def suppress_unreachable_warnings(self) -> None:
168177
self.frames[-1].suppress_unreachable_warnings = True
@@ -175,12 +184,22 @@ def get(self, expr: Expression) -> Type | None:
175184
return None
176185
return found.type
177186

178-
def is_unreachable(self) -> bool:
187+
def is_unreachable(self) -> UnreachableType | None:
179188
# TODO: Copy the value of unreachable into new frames to avoid
180189
# this traversal on every statement?
181-
return any(f.unreachable for f in self.frames)
190+
unreachable_type = None
191+
for f in self.frames:
192+
if f.unreachable and not unreachable_type:
193+
unreachable_type = f.unreachable
194+
elif f.unreachable == UnreachableType.SEMANAL_UNREACHABLE:
195+
unreachable_type = f.unreachable
196+
return unreachable_type
182197

183198
def is_unreachable_warning_suppressed(self) -> bool:
199+
# Do not report unreachable warnings from frames that were marked
200+
# unreachable by the semanal_pass1.
201+
if self.is_unreachable() == UnreachableType.SEMANAL_UNREACHABLE:
202+
return True
184203
return any(f.suppress_unreachable_warnings for f in self.frames)
185204

186205
def cleanse(self, expr: Expression) -> None:
@@ -202,6 +221,12 @@ def update_from_options(self, frames: list[Frame]) -> bool:
202221
If a key is declared as AnyType, only update it if all the
203222
options are the same.
204223
"""
224+
if all(f.unreachable for f in frames):
225+
semanal_unreachable = any(
226+
f.unreachable == UnreachableType.SEMANAL_UNREACHABLE for f in frames
227+
)
228+
self.unreachable(from_semanal=semanal_unreachable)
229+
205230
all_reachable = all(not f.unreachable for f in frames)
206231
frames = [f for f in frames if not f.unreachable]
207232
changed = False
@@ -262,8 +287,6 @@ def update_from_options(self, frames: list[Frame]) -> bool:
262287
self._put(key, type, from_assignment=True)
263288
changed = True
264289

265-
self.frames[-1].unreachable = not frames
266-
267290
return changed
268291

269292
def pop_frame(self, can_skip: bool, fall_through: int) -> Frame:
@@ -411,7 +434,7 @@ def allow_jump(self, index: int) -> None:
411434
for f in self.frames[index + 1 :]:
412435
frame.types.update(f.types)
413436
if f.unreachable:
414-
frame.unreachable = True
437+
frame.unreachable = f.unreachable
415438
self.options_on_return[index].append(frame)
416439

417440
def handle_break(self) -> None:

mypy/checker.py

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3021,7 +3021,7 @@ def visit_block(self, b: Block) -> None:
30213021
# This block was marked as being unreachable during semantic analysis.
30223022
# It turns out any blocks marked in this way are *intentionally* marked
30233023
# as unreachable -- so we don't display an error.
3024-
self.binder.unreachable()
3024+
self.binder.unreachable(from_semanal=True)
30253025
return
30263026
for s in b.body:
30273027
if self.binder.is_unreachable():
@@ -4871,14 +4871,6 @@ def visit_if_stmt(self, s: IfStmt) -> None:
48714871
if s.else_body:
48724872
self.accept(s.else_body)
48734873

4874-
# If this if-statement had a block that is considered always true for
4875-
# semantic reachability in semanal_pass1 and it has no else block;
4876-
# then we surpress unreachable warnings for code after the
4877-
# if-statement. These can be caused by early returns or raises and
4878-
# are only applicable on certain platforms or versions.
4879-
if s.has_pass1_always_true_block and s.else_body and (not s.else_body.body):
4880-
self.binder.suppress_unreachable_warnings()
4881-
48824874
def visit_while_stmt(self, s: WhileStmt) -> None:
48834875
"""Type check a while statement."""
48844876
if_stmt = IfStmt([s.expr], [s.body], None)

mypy/nodes.py

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1523,27 +1523,20 @@ def accept(self, visitor: StatementVisitor[T]) -> T:
15231523

15241524

15251525
class IfStmt(Statement):
1526-
__slots__ = ("expr", "body", "else_body", "has_pass1_always_true_block")
1526+
__slots__ = ("expr", "body", "else_body")
15271527

1528-
__match_args__ = ("expr", "body", "else_body", "has_pass1_always_true_block")
1528+
__match_args__ = ("expr", "body", "else_body")
15291529

15301530
expr: list[Expression]
15311531
body: list[Block]
15321532
else_body: Block | None
15331533
pass1_always_true_block: bool
15341534

1535-
def __init__(
1536-
self,
1537-
expr: list[Expression],
1538-
body: list[Block],
1539-
else_body: Block | None,
1540-
has_pass1_always_true_block: bool = False,
1541-
) -> None:
1535+
def __init__(self, expr: list[Expression], body: list[Block], else_body: Block | None) -> None:
15421536
super().__init__()
15431537
self.expr = expr
15441538
self.body = body
15451539
self.else_body = else_body
1546-
self.has_pass1_always_true_block = has_pass1_always_true_block
15471540

15481541
def accept(self, visitor: StatementVisitor[T]) -> T:
15491542
return visitor.visit_if_stmt(self)

mypy/reachability.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ def infer_reachability_of_if_statement(s: IfStmt, options: Options) -> None:
5757
# The condition is considered always false, so we skip the if/elif body.
5858
mark_block_unreachable(s.body[i])
5959
elif result in (ALWAYS_TRUE, MYPY_TRUE):
60-
s.has_pass1_always_true_block = True
6160
# This condition is considered always true, so all of the remaining
6261
# elif/else bodies should not be checked.
6362
if result == MYPY_TRUE:

test-data/unit/check-unreachable-code.test

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -395,17 +395,16 @@ def foo() -> int:
395395
[builtins fixtures/ops.pyi]
396396
[out]
397397

398-
-- TODO: We currently suppress unreachability warnings past semantic
399-
-- reachability checks. Need to decide if this should be fixed and enabled.
400-
[case testSysPlatformEarlyReturnActualUnreachableCodeNotForPlatform-skip]
398+
[case testSysPlatformEarlyReturnActualUnreachableCodeNotForPlatform]
401399
# flags: --warn-unreachable
402400
import sys
403401

404402
def foo() -> int:
405403
if sys.platform != 'fictional':
406404
return 1
407405
return 0
408-
return 0 + 1 # E: Statement is unreachable
406+
return 0 + 1 # Will not throw `Statement is unreachable` because we are
407+
# not on the fictional platform.
409408
[builtins fixtures/ops.pyi]
410409
[out]
411410

0 commit comments

Comments
 (0)