WIP: Replace the binder's breaking_out by an 'unreachable' property of Frame

Author: rwbarton

mypy/binder.py

@@ -10,7 +10,8 @@
 
 
 class Frame(Dict[Any, Type]):
-    pass
+    def __init__(self) -> None:
+        self.unreachable = False
 
 
 class Key(AnyType):
@@ -55,16 +56,8 @@ def __init__(self) -> None:
         # Whenever a new key (e.g. x.a.b) is added, we update this
         self.dependencies = {}  # type: Dict[Key, Set[Key]]
 
-        # breaking_out is set to True on return/break/continue/raise
-        # It is cleared on pop_frame() and placed in last_pop_breaking_out
-        # Lines of code after breaking_out = True are unreachable and not
-        # typechecked.
-        self.breaking_out = False
-
         # Whether the last pop changed the newly top frame on exit
         self.last_pop_changed = False
-        # Whether the last pop was necessarily breaking out, and couldn't fall through
-        self.last_pop_breaking_out = False
 
         self.try_frames = set()  # type: Set[int]
         self.loop_frames = []  # type: List[int]
@@ -105,9 +98,15 @@ def push(self, expr: Node, typ: Type) -> None:
             self._add_dependencies(key)
         self._push(key, typ)
 
+    def unreachable(self) -> bool:
+        self.frames[-1].unreachable = True
+
     def get(self, expr: Node) -> Type:
         return self._get(expr.literal_hash)
 
+    def is_unreachable(self) -> bool:
+        return self.frames[-1].unreachable
+
     def cleanse(self, expr: Node) -> None:
         """Remove all references to a Node from the binder."""
         self._cleanse_key(expr.literal_hash)
@@ -126,6 +125,8 @@ def update_from_options(self, frames: List[Frame]) -> bool:
         options are the same.
         """
 
+        frames = [f for f in frames if not f.unreachable]
+        # XXX What if our frame is unreachable?
         changed = False
         keys = set(key for f in frames for key in f)
 
@@ -147,21 +148,29 @@ def update_from_options(self, frames: List[Frame]) -> bool:
                 self._push(key, type)
                 changed = True
 
+        old_unreachable = self.frames[-1].unreachable
+        new_unreachable = old_unreachable and not frames
+        self.frames[-1].unreachable = new_unreachable
+        changed = changed or (old_unreachable != new_unreachable)
+
         return changed
 
     def pop_frame(self, fall_through: int = 0) -> Frame:
         """Pop a frame and return it.
 
         See frame_context() for documentation of fall_through.
         """
-        if fall_through and not self.breaking_out:
+
+        if fall_through > 0:
             self.allow_jump(-fall_through)
 
         result = self.frames.pop()
         options = self.options_on_return.pop()
 
+        if fall_through == 0:
+            self.unreachable()
+
         self.last_pop_changed = self.update_from_options(options)
-        self.last_pop_breaking_out = self.breaking_out
 
         return result
 
@@ -240,6 +249,8 @@ def allow_jump(self, index: int) -> None:
         frame = Frame()
         for f in self.frames[index + 1:]:
             frame.update(f)
+            if f.unreachable:
+                frame.unreachable = True
         self.options_on_return[index].append(frame)
 
     def push_loop_frame(self) -> None:
@@ -255,10 +266,12 @@ def frame_context(self, fall_through: int = 0) -> Iterator[Frame]:
         If fall_through > 0, then it will allow the frame to escape to
         its ancestor `fall_through` levels higher.
 
-        A simple 'with binder.frame_context(): pass' will change the
-        last_pop_* flags but nothing else.
+        If fall_through == 0, then control cannot flow through the frame;
+        also in this case entering the frame is not optional. Control
+        must flow through a nested frame_context(N) block with N > 1.
+
+        A simple 'with binder.frame_context(1): pass' will change the
+        last_pop_changed flag but nothing else.
         """
-        was_breaking_out = self.breaking_out
         yield self.push_frame()
         self.pop_frame(fall_through)
-        self.breaking_out = was_breaking_out

mypy/checker.py

@@ -911,9 +911,9 @@ def visit_block(self, b: Block) -> Type:
         if b.is_unreachable:
             return None
         for s in b.body:
-            self.accept(s)
-            if self.binder.breaking_out:
+            if self.binder.is_unreachable():
                 break
+            self.accept(s)
 
     def visit_assignment_stmt(self, s: AssignmentStmt) -> Type:
         """Type check an assignment statement.
@@ -1354,7 +1354,10 @@ def visit_expression_stmt(self, s: ExpressionStmt) -> Type:
 
     def visit_return_stmt(self, s: ReturnStmt) -> Type:
         """Type check a return statement."""
-        self.binder.breaking_out = True
+        self.check_return_stmt(s)
+        self.binder.unreachable()
+
+    def check_return_stmt(self, s: ReturnStmt) -> None:
         if self.is_within_function():
             if self.function_stack[-1].is_generator:
                 return_type = self.get_generator_return_type(self.return_types[-1])
@@ -1422,7 +1425,6 @@ def count_nested_types(self, typ: Instance, check_type: str) -> int:
 
     def visit_if_stmt(self, s: IfStmt) -> Type:
         """Type check an if statement."""
-        breaking_out = True
         # This frame records the knowledge from previous if/elif clauses not being taken.
         with self.binder.frame_context():
             for e, b in zip(s.expr, s.body):
@@ -1444,7 +1446,6 @@ def visit_if_stmt(self, s: IfStmt) -> Type:
                                 self.binder.push(var, type)
 
                         self.accept(b)
-                    breaking_out = breaking_out and self.binder.last_pop_breaking_out
 
                     if else_map:
                         for var, type in else_map.items():
@@ -1460,9 +1461,6 @@ def visit_if_stmt(self, s: IfStmt) -> Type:
                 with self.binder.frame_context(2):
                     if s.else_body:
                         self.accept(s.else_body)
-                breaking_out = breaking_out and self.binder.last_pop_breaking_out
-        if breaking_out:
-            self.binder.breaking_out = True
         return None
 
     def visit_while_stmt(self, s: WhileStmt) -> Type:
@@ -1498,11 +1496,11 @@ def visit_assert_stmt(self, s: AssertStmt) -> Type:
 
     def visit_raise_stmt(self, s: RaiseStmt) -> Type:
         """Type check a raise statement."""
-        self.binder.breaking_out = True
         if s.expr:
             self.type_check_raise(s.expr, s)
         if s.from_expr:
             self.type_check_raise(s.from_expr, s)
+        self.binder.unreachable()
 
     def type_check_raise(self, e: Node, s: RaiseStmt) -> None:
         typ = self.accept(e)
@@ -1535,28 +1533,26 @@ def visit_try_stmt(self, s: TryStmt) -> Type:
         with self.binder.frame_context():
             if s.finally_body:
                 self.binder.try_frames.add(len(self.binder.frames) - 1)
-                breaking_out = self.visit_try_without_finally(s)
+                self.visit_try_without_finally(s)
                 self.binder.try_frames.remove(len(self.binder.frames) - 1)
                 # First we check finally_body is type safe for all intermediate frames
                 self.accept(s.finally_body)
-                breaking_out = breaking_out or self.binder.breaking_out
             else:
-                breaking_out = self.visit_try_without_finally(s)
+                self.visit_try_without_finally(s)
 
-        if not breaking_out and s.finally_body:
+        if s.finally_body:
             # Then we try again for the more restricted set of options that can fall through
             self.accept(s.finally_body)
-        self.binder.breaking_out = breaking_out
+
         return None
 
-    def visit_try_without_finally(self, s: TryStmt) -> bool:
+    def visit_try_without_finally(self, s: TryStmt) -> None:
         """Type check a try statement, ignoring the finally block.
 
         Return whether we are guaranteed to be breaking out.
         Otherwise, it will place the results possible frames of
         that don't break out into self.binder.frames[-2].
         """
-        breaking_out = True
         # This frame records the possible states that exceptions can leave variables in
         # during the try: block
         with self.binder.frame_context():
@@ -1566,7 +1562,6 @@ def visit_try_without_finally(self, s: TryStmt) -> bool:
                 self.binder.try_frames.remove(len(self.binder.frames) - 2)
                 if s.else_body:
                     self.accept(s.else_body)
-            breaking_out = breaking_out and self.binder.last_pop_breaking_out
             for i in range(len(s.handlers)):
                 with self.binder.frame_context(3):
                     if s.types[i]:
@@ -1592,8 +1587,6 @@ def visit_try_without_finally(self, s: TryStmt) -> bool:
                         var = cast(Var, s.vars[i].node)
                         var.type = DeletedType(source=source)
                         self.binder.cleanse(s.vars[i])
-                breaking_out = breaking_out and self.binder.last_pop_breaking_out
-        return breaking_out
 
     def visit_except_handler_test(self, n: Node) -> Type:
         """Type check an exception handler test clause."""
@@ -1811,13 +1804,13 @@ def visit_member_expr(self, e: MemberExpr) -> Type:
         return self.expr_checker.visit_member_expr(e)
 
     def visit_break_stmt(self, s: BreakStmt) -> Type:
-        self.binder.breaking_out = True
         self.binder.allow_jump(self.binder.loop_frames[-1] - 1)
+        self.binder.unreachable()
         return None
 
     def visit_continue_stmt(self, s: ContinueStmt) -> Type:
-        self.binder.breaking_out = True
         self.binder.allow_jump(self.binder.loop_frames[-1])
+        self.binder.unreachable()
         return None
 
     def visit_int_expr(self, e: IntExpr) -> Type:

mypy/checkexpr.py

@@ -1091,6 +1091,8 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type:
             right_map = None
 
         with self.chk.binder.frame_context():
+            with self.chk.binder.frame_context(2):
+                pass
             if right_map:
                 for var, type in right_map.items():
                     self.chk.binder.push(var, type)
@@ -1489,6 +1491,8 @@ def check_for_comp(self, e: Union[GeneratorExpr, DictionaryComprehension]) -> No
         ... for x in y if z
         """
         with self.chk.binder.frame_context():
+            with self.chk.binder.frame_context(2):
+                pass
             for index, sequence, conditions in zip(e.indices, e.sequences,
                                                    e.condlists):
                 sequence_type = self.chk.analyze_iterable_item_type(sequence)
@@ -1534,6 +1538,8 @@ def visit_conditional_expr(self, e: ConditionalExpr) -> Type:
     def analyze_cond_branch(self, map: Optional[Dict[Node, Type]],
                             node: Node, context: Optional[Type]) -> Type:
         with self.chk.binder.frame_context():
+            with self.chk.binder.frame_context(2):
+                pass
             if map:
                 for var, type in map.items():
                     self.chk.binder.push(var, type)