From 8cd215e05c3bd052e53e9f5179af1fe9602d898c Mon Sep 17 00:00:00 2001 From: Eric Price Date: Tue, 21 Jun 2016 21:16:15 -0700 Subject: [PATCH 01/28] Change ConditionalTypeBinder usage to mainly use with statements. Rather than use `binder.push_frame()` and `binder.pop_frame(*args)`, you now do `with binder.frame_context(*args)`. There was a previous context manager used as `with binder`, but it was only used in a few places because it didn't support options. In the process, I discovered several bugs in the old control flow analysis; new test cases have been added to catch them. --- mypy/checker.py | 534 ++++++++++++++------------- mypy/checkexpr.py | 29 +- test-data/unit/check-isinstance.test | 72 +++- 3 files changed, 364 insertions(+), 271 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index a7af74911c1e..165f60bf13a9 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -67,7 +67,32 @@ def min_with_None_large(x: T, y: T) -> T: class Frame(Dict[Any, Type]): - pass + """Frame for the ConditionalTypeBinder. + + Frames actually in the binder must have + binder.frames[i].parent == binder.frames[i-1]. + + options_on_return represents a list of possible frames that we + will incorporate next time we are the top frame (e.g. frames from + the end of each if/elif/else block). + """ + + changed = False + + def __init__(self, parent: 'Frame' = None) -> None: + super().__init__() + self.parent = parent + self.options_on_return = [] # type: List[Frame] + + def add_exit_option(self, frame: 'Frame') -> None: + """When this frame __exit__'s, its parent may have state `frame`. + + So self.add_exit_option(self) means that you can fall through + to the outer frame, and self.add_exit_option(self.parent) + means that the parent is allowed to continue unchanged by what + happened in this frame. + """ + self.parent.options_on_return.append(frame) class Key(AnyType): @@ -81,13 +106,13 @@ def __init__(self) -> None: self.frames = [] # type: List[Frame] # The first frame is special: it's the declared types of variables. self.frames.append(Frame()) + # We want a second frame that we can update. + self.frames.append(Frame()) # Set of other keys to invalidate if a key is changed. self.dependencies = {} # type: Dict[Key, Set[Key]] # Set of keys with dependencies added already. self._added_dependencies = set() # type: Set[Key] - self.frames_on_escape = {} # type: Dict[int, List[Frame]] - self.try_frames = set() # type: Set[int] self.loop_frames = [] # type: List[int] @@ -105,10 +130,26 @@ def _add_dependencies(self, key: Key, value: Key = None) -> None: for elt in cast(Any, key): self._add_dependencies(elt, value) - def push_frame(self) -> Frame: - d = Frame() - self.frames.append(d) - return d + def push_frame(self, canskip: bool = False, fallthrough: bool = False) -> Frame: + """Push a new frame into the binder. + + If canskip, then allow types to skip all the inner frame + blocks. That is, changes that happened in the inner frames + are not necessarily reflected in the outer frame (for example, + an if block that may be skipped). + + If fallthrough, then allow types to escape from the inner + frame to the resulting frame. That is, the state of types at + the end of the last frame are allowed to fall through into the + enclosing frame. + """ + f = Frame(self.frames[-1]) + self.frames.append(f) + if canskip: + f.add_exit_option(f.parent) + if fallthrough: + f.add_exit_option(f) + return f def _push(self, key: Key, type: Type, index: int=-1) -> None: self._add_dependencies(key) @@ -187,34 +228,20 @@ def update_expand(self, frame: Frame, index: int = -1) -> bool: result = True return result - def pop_frame(self, canskip=True, fallthrough=False) -> Tuple[bool, Frame]: - """Pop a frame. - - If canskip, then allow types to skip all the inner frame - blocks. That is, changes that happened in the inner frames - are not necessarily reflected in the outer frame (for example, - an if block that may be skipped). - - If fallthrough, then allow types to escape from the inner - frame to the resulting frame. That is, the state of types at - the end of the last frame are allowed to fall through into the - enclosing frame. + def pop_frame(self) -> Frame: + """Pop a frame and return it. - Return whether the newly innermost frame was modified since it - was last on top, and what it would be if the block had run to - completion. + frame.changed represents whether the newly innermost frame was + modified since it was last on top. """ result = self.frames.pop() - options = self.frames_on_escape.pop(len(self.frames) - 1, []) # type: List[Frame] - if canskip: - options.append(self.frames[-1]) - if fallthrough: - options.append(result) + options = self.frames[-1].options_on_return + self.frames[-1].options_on_return = [] - changed = self.update_from_options(options) + result.changed = self.update_from_options(options) - return (changed, result) + return result def get_declaration(self, expr: Any) -> Type: if hasattr(expr, 'node') and isinstance(expr.node, Var): @@ -292,19 +319,35 @@ def allow_jump(self, index: int) -> None: for k in f: new_frame[k] = f[k] - self.frames_on_escape.setdefault(index, []).append(new_frame) + self.frames[index].options_on_return.append(new_frame) - def push_loop_frame(self): + def push_loop_frame(self) -> None: self.loop_frames.append(len(self.frames) - 1) - def pop_loop_frame(self): + def pop_loop_frame(self) -> None: self.loop_frames.pop() - def __enter__(self) -> None: - self.push_frame() + def frame_context(self, canskip: bool = False, + fallthrough: bool = False) -> 'FrameContextManager': + return FrameContextManager(self, canskip, fallthrough) + + +class FrameContextManager: + """This is a context manager returned by binder.frame_context(). + + Pushes a frame on __enter__, pops it on __exit__. + """ + def __init__(self, binder: ConditionalTypeBinder, canskip: bool, + fallthrough: bool) -> None: + self.binder = binder + self.canskip = canskip + self.fallthrough = fallthrough + + def __enter__(self) -> Frame: + return self.binder.push_frame(self.canskip, self.fallthrough) def __exit__(self, *args: Any) -> None: - self.pop_frame() + self.binder.pop_frame() def meet_frames(*frames: Frame) -> Frame: @@ -398,7 +441,6 @@ def __init__(self, errors: Errors, modules: Dict[str, MypyFile], self.msg = MessageBuilder(errors, modules) self.type_map = {} self.binder = ConditionalTypeBinder() - self.binder.push_frame() self.expr_checker = mypy.checkexpr.ExpressionChecker(self, self.msg) self.return_types = [] self.type_context = [] @@ -488,15 +530,13 @@ def accept(self, node: Node, type_context: Type = None) -> Type: else: return typ - def accept_in_frame(self, node: Node, type_context: Type = None, - repeat_till_fixed: bool = False) -> Type: - """Type check a node in the given type context in a new frame of inferred types.""" + def accept_till_fixed(self, node: Node) -> Type: + """Repeatedly type check a node until the frame doesn't change.""" while True: - self.binder.push_frame() - answer = self.accept(node, type_context) - changed, _ = self.binder.pop_frame(True, True) - self.breaking_out = False - if not repeat_till_fixed or not changed: + with self.binder.frame_context(True, False) as frame: + answer = self.accept(node) + self.option_if_not_breaking(frame, frame) + if not frame.changed: return answer # @@ -668,97 +708,96 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: str) -> None: for item, typ in self.expand_typevars(defn, typ): old_binder = self.binder self.binder = ConditionalTypeBinder() - self.binder.push_frame() - defn.expanded.append(item) - - # We may be checking a function definition or an anonymous - # function. In the first case, set up another reference with the - # precise type. - if isinstance(item, FuncDef): - fdef = item - else: - fdef = None - - if fdef: - # Check if __init__ has an invalid, non-None return type. - if (fdef.info and fdef.name() == '__init__' and - not isinstance(typ.ret_type, Void) and - not self.dynamic_funcs[-1]): - self.fail(messages.INIT_MUST_HAVE_NONE_RETURN_TYPE, - item.type) - - show_untyped = not self.is_typeshed_stub or self.warn_incomplete_stub - if self.disallow_untyped_defs and show_untyped: - # Check for functions with unspecified/not fully specified types. - def is_implicit_any(t: Type) -> bool: - return isinstance(t, AnyType) and t.implicit - - if fdef.type is None: - self.fail(messages.FUNCTION_TYPE_EXPECTED, fdef) - elif isinstance(fdef.type, CallableType): - if is_implicit_any(fdef.type.ret_type): - self.fail(messages.RETURN_TYPE_EXPECTED, fdef) - if any(is_implicit_any(t) for t in fdef.type.arg_types): - self.fail(messages.ARGUMENT_TYPE_EXPECTED, fdef) - - if name in nodes.reverse_op_method_set: - self.check_reverse_op_method(item, typ, name) - elif name == '__getattr__': - self.check_getattr_method(typ, defn) - - # Refuse contravariant return type variable - if isinstance(typ.ret_type, TypeVarType): - if typ.ret_type.variance == CONTRAVARIANT: - self.fail(messages.RETURN_TYPE_CANNOT_BE_CONTRAVARIANT, - typ.ret_type) - - # Check that Generator functions have the appropriate return type. - if defn.is_generator: - if not self.is_generator_return_type(typ.ret_type): - self.fail(messages.INVALID_RETURN_TYPE_FOR_GENERATOR, typ) - - # Python 2 generators aren't allowed to return values. - if (self.pyversion[0] == 2 and - isinstance(typ.ret_type, Instance) and - typ.ret_type.type.fullname() == 'typing.Generator'): - if not (isinstance(typ.ret_type.args[2], Void) - or isinstance(typ.ret_type.args[2], AnyType)): - self.fail(messages.INVALID_GENERATOR_RETURN_ITEM_TYPE, typ) - - # Push return type. - self.return_types.append(typ.ret_type) - - # Store argument types. - for i in range(len(typ.arg_types)): - arg_type = typ.arg_types[i] - - # Refuse covariant parameter type variables - if isinstance(arg_type, TypeVarType): - if arg_type.variance == COVARIANT: - self.fail(messages.FUNCTION_PARAMETER_CANNOT_BE_COVARIANT, - arg_type) - - if typ.arg_kinds[i] == nodes.ARG_STAR: - # builtins.tuple[T] is typing.Tuple[T, ...] - arg_type = self.named_generic_type('builtins.tuple', - [arg_type]) - elif typ.arg_kinds[i] == nodes.ARG_STAR2: - arg_type = self.named_generic_type('builtins.dict', - [self.str_type(), - arg_type]) - item.arguments[i].variable.type = arg_type - - # Type check initialization expressions. - for arg in item.arguments: - init = arg.initialization_statement - if init: - self.accept(init) - - # Clear out the default assignments from the binder - self.binder.pop_frame() - self.binder.push_frame() + with self.binder.frame_context(): + defn.expanded.append(item) + + # We may be checking a function definition or an anonymous + # function. In the first case, set up another reference with the + # precise type. + if isinstance(item, FuncDef): + fdef = item + else: + fdef = None + + if fdef: + # Check if __init__ has an invalid, non-None return type. + if (fdef.info and fdef.name() == '__init__' and + not isinstance(typ.ret_type, Void) and + not self.dynamic_funcs[-1]): + self.fail(messages.INIT_MUST_HAVE_NONE_RETURN_TYPE, + item.type) + + show_untyped = not self.is_typeshed_stub or self.warn_incomplete_stub + if self.disallow_untyped_defs and show_untyped: + # Check for functions with unspecified/not fully specified types. + def is_implicit_any(t: Type) -> bool: + return isinstance(t, AnyType) and t.implicit + + if fdef.type is None: + self.fail(messages.FUNCTION_TYPE_EXPECTED, fdef) + elif isinstance(fdef.type, CallableType): + if is_implicit_any(fdef.type.ret_type): + self.fail(messages.RETURN_TYPE_EXPECTED, fdef) + if any(is_implicit_any(t) for t in fdef.type.arg_types): + self.fail(messages.ARGUMENT_TYPE_EXPECTED, fdef) + + if name in nodes.reverse_op_method_set: + self.check_reverse_op_method(item, typ, name) + elif name == '__getattr__': + self.check_getattr_method(typ, defn) + + # Refuse contravariant return type variable + if isinstance(typ.ret_type, TypeVarType): + if typ.ret_type.variance == CONTRAVARIANT: + self.fail(messages.RETURN_TYPE_CANNOT_BE_CONTRAVARIANT, + typ.ret_type) + + # Check that Generator functions have the appropriate return type. + if defn.is_generator: + if not self.is_generator_return_type(typ.ret_type): + self.fail(messages.INVALID_RETURN_TYPE_FOR_GENERATOR, typ) + + # Python 2 generators aren't allowed to return values. + if (self.pyversion[0] == 2 and + isinstance(typ.ret_type, Instance) and + typ.ret_type.type.fullname() == 'typing.Generator'): + if not (isinstance(typ.ret_type.args[2], Void) + or isinstance(typ.ret_type.args[2], AnyType)): + self.fail(messages.INVALID_GENERATOR_RETURN_ITEM_TYPE, typ) + + # Push return type. + self.return_types.append(typ.ret_type) + + # Store argument types. + for i in range(len(typ.arg_types)): + arg_type = typ.arg_types[i] + + # Refuse covariant parameter type variables + if isinstance(arg_type, TypeVarType): + if arg_type.variance == COVARIANT: + self.fail(messages.FUNCTION_PARAMETER_CANNOT_BE_COVARIANT, + arg_type) + + if typ.arg_kinds[i] == nodes.ARG_STAR: + # builtins.tuple[T] is typing.Tuple[T, ...] + arg_type = self.named_generic_type('builtins.tuple', + [arg_type]) + elif typ.arg_kinds[i] == nodes.ARG_STAR2: + arg_type = self.named_generic_type('builtins.dict', + [self.str_type(), + arg_type]) + item.arguments[i].variable.type = arg_type + + # Type check initialization expressions. + for arg in item.arguments: + init = arg.initialization_statement + if init: + self.accept(init) + # Type check body in a new scope. - self.accept_in_frame(item.body) + with self.binder.frame_context(): + self.accept(item.body) + self.breaking_out = False self.return_types.pop() @@ -1067,8 +1106,8 @@ def visit_class_def(self, defn: ClassDef) -> Type: self.enter_partial_types() old_binder = self.binder self.binder = ConditionalTypeBinder() - self.binder.push_frame() - self.accept(defn.defs) + with self.binder.frame_context(): + self.accept(defn.defs) self.binder = old_binder self.check_multiple_inheritance(typ) self.leave_partial_types() @@ -1675,76 +1714,62 @@ def count_nested_types(self, typ: Instance, check_type: str) -> int: def visit_if_stmt(self, s: IfStmt) -> Type: """Type check an if statement.""" broken = True - ending_frames = [] # type: List[Frame] - clauses_frame = self.binder.push_frame() - for e, b in zip(s.expr, s.body): - t = self.accept(e) - self.check_not_void(t, e) - if_map, else_map = find_isinstance_check( - e, self.type_map, - self.typing_mode_weak() - ) - if if_map is None: - # The condition is always false - # XXX should issue a warning? - pass + with self.binder.frame_context() as clauses_frame: + for e, b in zip(s.expr, s.body): + t = self.accept(e) + self.check_not_void(t, e) + if_map, else_map = find_isinstance_check( + e, self.type_map, + self.typing_mode_weak() + ) + if if_map is None: + # The condition is always false + # XXX should issue a warning? + pass + else: + # Only type check body if the if condition can be true. + with self.binder.frame_context() as frame: + if if_map: + for var, type in if_map.items(): + self.binder.push(var, type) + + self.accept(b) + broken = self.option_if_not_breaking(clauses_frame, frame) and broken + + if else_map: + for var, type in else_map.items(): + self.binder.push(var, type) + if else_map is None: + # The condition is always true => remaining elif/else blocks + # can never be reached. + + # Might also want to issue a warning + # print("Warning: isinstance always true") + if broken: + self.breaking_out = True + return None + break else: - # Only type check body if the if condition can be true. - self.binder.push_frame() - if if_map: - for var, type in if_map.items(): - self.binder.push(var, type) - - self.accept(b) - _, frame = self.binder.pop_frame() - if not self.breaking_out: - broken = False - ending_frames.append(meet_frames(clauses_frame, frame)) - - self.breaking_out = False - - if else_map: - for var, type in else_map.items(): - self.binder.push(var, type) - if else_map is None: - # The condition is always true => remaining elif/else blocks - # can never be reached. - - # Might also want to issue a warning - # print("Warning: isinstance always true") - if broken: - self.binder.pop_frame() - self.breaking_out = True - return None - break - else: - if s.else_body: - self.accept(s.else_body) + if s.else_body: + self.accept(s.else_body) - if self.breaking_out and broken: - self.binder.pop_frame() - return None - - if not self.breaking_out: - ending_frames.append(clauses_frame) + if self.breaking_out and broken: + return None - self.breaking_out = False - else: - ending_frames.append(clauses_frame) + self.option_if_not_breaking(clauses_frame, clauses_frame) - self.binder.pop_frame() - self.binder.update_from_options(ending_frames) + else: + clauses_frame.add_exit_option(clauses_frame) def visit_while_stmt(self, s: WhileStmt) -> Type: """Type check a while statement.""" - self.binder.push_frame() - self.binder.push_loop_frame() - self.accept_in_frame(IfStmt([s.expr], [s.body], None), - repeat_till_fixed=True) - self.binder.pop_loop_frame() - if s.else_body: - self.accept(s.else_body) - self.binder.pop_frame(False, True) + with self.binder.frame_context(fallthrough=True): + with self.binder.frame_context(fallthrough=True): + self.binder.push_loop_frame() + self.accept_till_fixed(IfStmt([s.expr], [s.body], None)) + self.binder.pop_loop_frame() + if s.else_body: + self.accept(s.else_body) def visit_operator_assignment_stmt(self, s: OperatorAssignmentStmt) -> Type: @@ -1805,56 +1830,57 @@ def type_check_raise(self, e: Node, s: RaiseStmt) -> None: self.named_type('builtins.BaseException'), s, messages.INVALID_EXCEPTION) + def option_if_not_breaking(self, outer_frame: Frame, inner_frame: Frame) -> bool: + broken = self.breaking_out + self.breaking_out = False + if not broken: + outer_frame.add_exit_option(inner_frame) + return broken + def visit_try_stmt(self, s: TryStmt) -> Type: """Type check a try statement.""" - completed_frames = [] # type: List[Frame] - self.binder.push_frame() - self.binder.try_frames.add(len(self.binder.frames) - 2) - self.accept(s.body) - self.binder.try_frames.remove(len(self.binder.frames) - 2) - self.breaking_out = False - changed, frame_on_completion = self.binder.pop_frame() - completed_frames.append(frame_on_completion) - - for i in range(len(s.handlers)): - self.binder.push_frame() - if s.types[i]: - t = self.visit_except_handler_test(s.types[i]) - if s.vars[i]: - # To support local variables, we make this a definition line, - # causing assignment to set the variable's type. - s.vars[i].is_def = True - self.check_assignment(s.vars[i], self.temp_node(t, s.vars[i])) - self.accept(s.handlers[i]) - if s.vars[i]: - # Exception variables are deleted in python 3 but not python 2. - # But, since it's bad form in python 2 and the type checking - # wouldn't work very well, we delete it anyway. - - # Unfortunately, this doesn't let us detect usage before the - # try/except block. - if self.pyversion[0] >= 3: - source = s.vars[i].name - else: - source = ('(exception variable "{}", which we do not accept ' - 'outside except: blocks even in python 2)'.format(s.vars[i].name)) - var = cast(Var, s.vars[i].node) - var.type = DeletedType(source=source) - self.binder.cleanse(s.vars[i]) - - self.breaking_out = False - changed, frame_on_completion = self.binder.pop_frame() - completed_frames.append(frame_on_completion) - - # Do the else block similar to the way we do except blocks. - if s.else_body: - self.binder.push_frame() - self.accept(s.else_body) - self.breaking_out = False - changed, frame_on_completion = self.binder.pop_frame() - completed_frames.append(frame_on_completion) - - self.binder.update_from_options(completed_frames) + with self.binder.frame_context() as outer_try_frame: + with self.binder.frame_context() as try_frame: + self.binder.try_frames.add(len(self.binder.frames) - 2) + self.accept(s.body) + self.binder.try_frames.remove(len(self.binder.frames) - 2) + self.option_if_not_breaking(outer_try_frame, try_frame) + + for i in range(len(s.handlers)): + with self.binder.frame_context() as except_frame: + if s.types[i]: + t = self.visit_except_handler_test(s.types[i]) + if s.vars[i]: + # To support local variables, we make this a definition line, + # causing assignment to set the variable's type. + s.vars[i].is_def = True + self.check_assignment(s.vars[i], self.temp_node(t, s.vars[i])) + self.accept(s.handlers[i]) + if s.vars[i]: + # Exception variables are deleted in python 3 but not python 2. + # But, since it's bad form in python 2 and the type checking + # wouldn't work very well, we delete it anyway. + + # Unfortunately, this doesn't let us detect usage before the + # try/except block. + if self.pyversion[0] >= 3: + source = s.vars[i].name + else: + source = ('(exception variable "{}", which we do not accept outside' + 'except: blocks even in python 2)'.format(s.vars[i].name)) + var = cast(Var, s.vars[i].node) + var.type = DeletedType(source=source) + self.binder.cleanse(s.vars[i]) + + self.option_if_not_breaking(outer_try_frame, except_frame) + + # Do the else block similar to the way we do except blocks. + if s.else_body: + with self.binder.frame_context() as else_frame: + self.accept(s.else_body) + self.option_if_not_breaking(outer_try_frame, else_frame) + else: + outer_try_frame.add_exit_option(outer_try_frame) if s.finally_body: self.accept(s.finally_body) @@ -1890,13 +1916,13 @@ def visit_for_stmt(self, s: ForStmt) -> Type: """Type check a for statement.""" item_type = self.analyze_iterable_item_type(s.expr) self.analyze_index_variables(s.index, item_type, s) - self.binder.push_frame() - self.binder.push_loop_frame() - self.accept_in_frame(s.body, repeat_till_fixed=True) - self.binder.pop_loop_frame() - if s.else_body: - self.accept(s.else_body) - self.binder.pop_frame(False, True) + with self.binder.frame_context(True, True): + with self.binder.frame_context(True, True): + self.binder.push_loop_frame() + self.accept_till_fixed(s.body) + self.binder.pop_loop_frame() + if s.else_body: + self.accept(s.else_body) def analyze_iterable_item_type(self, expr: Node) -> Type: """Analyse iterable expression and return iterator item type.""" diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index b51c7c0808db..7782843b98f9 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -1090,14 +1090,12 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type: else: right_map = None - self.chk.binder.push_frame() - if right_map: - for var, type in right_map.items(): - self.chk.binder.push(var, type) - - right_type = self.accept(e.right, left_type) + with self.chk.binder.frame_context(): + if right_map: + for var, type in right_map.items(): + self.chk.binder.push(var, type) - self.chk.binder.pop_frame() + right_type = self.accept(e.right, left_type) self.check_not_void(left_type, context) self.check_not_void(right_type, context) @@ -1490,14 +1488,13 @@ def check_for_comp(self, e: Union[GeneratorExpr, DictionaryComprehension]) -> No """Check the for_comp part of comprehensions. That is the part from 'for': ... for x in y if z """ - self.chk.binder.push_frame() - for index, sequence, conditions in zip(e.indices, e.sequences, - e.condlists): - sequence_type = self.chk.analyze_iterable_item_type(sequence) - self.chk.analyze_index_variables(index, sequence_type, e) - for condition in conditions: - self.accept(condition) - self.chk.binder.pop_frame() + with self.chk.binder.frame_context(): + for index, sequence, conditions in zip(e.indices, e.sequences, + e.condlists): + sequence_type = self.chk.analyze_iterable_item_type(sequence) + self.chk.analyze_index_variables(index, sequence_type, e) + for condition in conditions: + self.accept(condition) def visit_conditional_expr(self, e: ConditionalExpr) -> Type: cond_type = self.accept(e.cond) @@ -1536,7 +1533,7 @@ 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: + with self.chk.binder.frame_context(): if map: for var, type in map.items(): self.chk.binder.push(var, type) diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index 5d9127e502e3..042b83d03435 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -226,7 +226,43 @@ else: x = B() x.z - +[case testUnionTryExcept3] +class A: y = A() +class B(A): z = 1 +x = A() +x = B() +try: + raise BaseException() + x = A() +except: + pass +x.z +x = B() +try: + x = A() + raise BaseException() +except: + pass +x.z # E: "A" has no attribute "z" +x = B() +try: + pass +except: + x = A() + raise BaseException() +x.z +try: + x = A() +except: + pass +x.z # E: "A" has no attribute "z" +x = B() +try: + pass +except: + x = A() +x.z # E: "A" has no attribute "z" +[builtins fixtures/exception.py] [case testUnionListIsinstance] from typing import Union, List @@ -602,6 +638,14 @@ while 1: else: x + 1 x + 1 # E: Unsupported operand types for + (likely involving Union) +x = 1 +for y in [1]: + x + 1 + x = 'a' + break +else: + x + 1 +x + 1 # E: Unsupported operand types for + (likely involving Union) [builtins fixtures/isinstancelist.py] [case testModifyLoop4] @@ -632,6 +676,32 @@ else: x = 'a' x + 'a' [builtins fixtures/isinstancelist.py] +[case testModifyNestedLoop] +from typing import Union + +def foo() -> Union[int, str]: pass + +x = foo() +x = 1 + +for y in [1]: + for z in [1]: + break + else: + x = 'a' + break +else: + x + 1 +x = 1 +while 1: + while 1: + break + else: + x = 'a' + break +else: + x + 1 +[builtins fixtures/isinstancelist.py] [case testModifyLoopLong] from typing import Union From 186dfbf9c402c787addbecdd4e737aaeef8605b6 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Tue, 21 Jun 2016 22:03:04 -0700 Subject: [PATCH 02/28] Add clear_breaking option to type binder context manager. This allows the context manager to deal with having multiple parallel blocks that can break out separately. It will clear the breaking_out parameter and propagate variables to an enclosing frame only if breaking_out wasn't True after falling off the frame. --- mypy/checker.py | 103 ++++++++++++++------------- test-data/unit/check-isinstance.test | 4 +- 2 files changed, 58 insertions(+), 49 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 165f60bf13a9..67b58982a86b 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -75,9 +75,14 @@ class Frame(Dict[Any, Type]): options_on_return represents a list of possible frames that we will incorporate next time we are the top frame (e.g. frames from the end of each if/elif/else block). + + After a frame is complete, 'changed' represents whether anything + changed during the procedure, so a loop should be re-run. + 'breaking' represents whether frame was necessarily breaking out. """ changed = False + broken = False def __init__(self, parent: 'Frame' = None) -> None: super().__init__() @@ -113,6 +118,9 @@ def __init__(self) -> None: # Set of keys with dependencies added already. self._added_dependencies = set() # type: Set[Key] + # Set to True on return/break/raise, False on blocks that can block any of them + self.breaking_out = False + self.try_frames = set() # type: Set[int] self.loop_frames = [] # type: List[int] @@ -233,6 +241,9 @@ def pop_frame(self) -> Frame: frame.changed represents whether the newly innermost frame was modified since it was last on top. + + frame.broken represents whether we always leave this frame + through a construct other than falling off the end. """ result = self.frames.pop() @@ -240,6 +251,7 @@ def pop_frame(self) -> Frame: self.frames[-1].options_on_return = [] result.changed = self.update_from_options(options) + result.broken = self.breaking_out return result @@ -327,9 +339,9 @@ def push_loop_frame(self) -> None: def pop_loop_frame(self) -> None: self.loop_frames.pop() - def frame_context(self, canskip: bool = False, - fallthrough: bool = False) -> 'FrameContextManager': - return FrameContextManager(self, canskip, fallthrough) + def frame_context(self, canskip: bool = False, fallthrough: bool = False, + clear_breaking: bool=False) -> 'FrameContextManager': + return FrameContextManager(self, canskip, fallthrough, clear_breaking) class FrameContextManager: @@ -338,16 +350,21 @@ class FrameContextManager: Pushes a frame on __enter__, pops it on __exit__. """ def __init__(self, binder: ConditionalTypeBinder, canskip: bool, - fallthrough: bool) -> None: + fallthrough: bool, clear_breaking: bool) -> None: self.binder = binder self.canskip = canskip self.fallthrough = fallthrough + self.clear_breaking = clear_breaking def __enter__(self) -> Frame: return self.binder.push_frame(self.canskip, self.fallthrough) def __exit__(self, *args: Any) -> None: - self.binder.pop_frame() + f = self.binder.pop_frame() + if self.clear_breaking: + self.binder.breaking_out = False + if not f.broken: + f.parent.add_exit_option(f) def meet_frames(*frames: Frame) -> Frame: @@ -400,8 +417,6 @@ class TypeChecker(NodeVisitor[Type]): dynamic_funcs = None # type: List[bool] # Stack of functions being type checked function_stack = None # type: List[FuncItem] - # Set to True on return/break/raise, False on blocks that can block any of them - breaking_out = False # Do weak type checking in this file weak_opts = set() # type: Set[str] # Stack of collections of variables with partial types @@ -533,9 +548,12 @@ def accept(self, node: Node, type_context: Type = None) -> Type: def accept_till_fixed(self, node: Node) -> Type: """Repeatedly type check a node until the frame doesn't change.""" while True: - with self.binder.frame_context(True, False) as frame: + with self.binder.frame_context(canskip=True) as frame: answer = self.accept(node) - self.option_if_not_breaking(frame, frame) + if not self.binder.breaking_out: + frame.add_exit_option(frame) + else: + self.binder.breaking_out = False if not frame.changed: return answer @@ -797,7 +815,7 @@ def is_implicit_any(t: Type) -> bool: # Type check body in a new scope. with self.binder.frame_context(): self.accept(item.body) - self.breaking_out = False + self.binder.breaking_out = False self.return_types.pop() @@ -1203,7 +1221,7 @@ def visit_block(self, b: Block) -> Type: return None for s in b.body: self.accept(s) - if self.breaking_out: + if self.binder.breaking_out: break def visit_assignment_stmt(self, s: AssignmentStmt) -> Type: @@ -1645,7 +1663,7 @@ def visit_expression_stmt(self, s: ExpressionStmt) -> Type: def visit_return_stmt(self, s: ReturnStmt) -> Type: """Type check a return statement.""" - self.breaking_out = True + self.binder.breaking_out = True if self.is_within_function(): if self.function_stack[-1].is_generator: return_type = self.get_generator_return_type(self.return_types[-1]) @@ -1728,13 +1746,13 @@ def visit_if_stmt(self, s: IfStmt) -> Type: pass else: # Only type check body if the if condition can be true. - with self.binder.frame_context() as frame: + with self.binder.frame_context(clear_breaking=True) as frame: if if_map: for var, type in if_map.items(): self.binder.push(var, type) self.accept(b) - broken = self.option_if_not_breaking(clauses_frame, frame) and broken + broken = broken and frame.broken if else_map: for var, type in else_map.items(): @@ -1746,17 +1764,19 @@ def visit_if_stmt(self, s: IfStmt) -> Type: # Might also want to issue a warning # print("Warning: isinstance always true") if broken: - self.breaking_out = True + self.binder.breaking_out = True return None break else: if s.else_body: - self.accept(s.else_body) + with self.binder.frame_context(clear_breaking=True) as frame: + self.accept(s.else_body) - if self.breaking_out and broken: - return None + broken = broken and frame.broken - self.option_if_not_breaking(clauses_frame, clauses_frame) + if broken: + self.binder.breaking_out = True + return None else: clauses_frame.add_exit_option(clauses_frame) @@ -1764,12 +1784,11 @@ def visit_if_stmt(self, s: IfStmt) -> Type: def visit_while_stmt(self, s: WhileStmt) -> Type: """Type check a while statement.""" with self.binder.frame_context(fallthrough=True): - with self.binder.frame_context(fallthrough=True): - self.binder.push_loop_frame() - self.accept_till_fixed(IfStmt([s.expr], [s.body], None)) - self.binder.pop_loop_frame() - if s.else_body: - self.accept(s.else_body) + self.binder.push_loop_frame() + self.accept_till_fixed(IfStmt([s.expr], [s.body], None)) + self.binder.pop_loop_frame() + if s.else_body: + self.accept(s.else_body) def visit_operator_assignment_stmt(self, s: OperatorAssignmentStmt) -> Type: @@ -1800,7 +1819,7 @@ def visit_assert_stmt(self, s: AssertStmt) -> Type: def visit_raise_stmt(self, s: RaiseStmt) -> Type: """Type check a raise statement.""" - self.breaking_out = True + self.binder.breaking_out = True if s.expr: self.type_check_raise(s.expr, s) if s.from_expr: @@ -1830,24 +1849,16 @@ def type_check_raise(self, e: Node, s: RaiseStmt) -> None: self.named_type('builtins.BaseException'), s, messages.INVALID_EXCEPTION) - def option_if_not_breaking(self, outer_frame: Frame, inner_frame: Frame) -> bool: - broken = self.breaking_out - self.breaking_out = False - if not broken: - outer_frame.add_exit_option(inner_frame) - return broken - def visit_try_stmt(self, s: TryStmt) -> Type: """Type check a try statement.""" with self.binder.frame_context() as outer_try_frame: - with self.binder.frame_context() as try_frame: + with self.binder.frame_context(clear_breaking=True): self.binder.try_frames.add(len(self.binder.frames) - 2) self.accept(s.body) self.binder.try_frames.remove(len(self.binder.frames) - 2) - self.option_if_not_breaking(outer_try_frame, try_frame) for i in range(len(s.handlers)): - with self.binder.frame_context() as except_frame: + with self.binder.frame_context(clear_breaking=True): if s.types[i]: t = self.visit_except_handler_test(s.types[i]) if s.vars[i]: @@ -1872,13 +1883,10 @@ def visit_try_stmt(self, s: TryStmt) -> Type: var.type = DeletedType(source=source) self.binder.cleanse(s.vars[i]) - self.option_if_not_breaking(outer_try_frame, except_frame) - # Do the else block similar to the way we do except blocks. if s.else_body: - with self.binder.frame_context() as else_frame: + with self.binder.frame_context(clear_breaking=True): self.accept(s.else_body) - self.option_if_not_breaking(outer_try_frame, else_frame) else: outer_try_frame.add_exit_option(outer_try_frame) @@ -1917,12 +1925,11 @@ def visit_for_stmt(self, s: ForStmt) -> Type: item_type = self.analyze_iterable_item_type(s.expr) self.analyze_index_variables(s.index, item_type, s) with self.binder.frame_context(True, True): - with self.binder.frame_context(True, True): - self.binder.push_loop_frame() - self.accept_till_fixed(s.body) - self.binder.pop_loop_frame() - if s.else_body: - self.accept(s.else_body) + self.binder.push_loop_frame() + self.accept_till_fixed(s.body) + self.binder.pop_loop_frame() + if s.else_body: + self.accept(s.else_body) def analyze_iterable_item_type(self, expr: Node) -> Type: """Analyse iterable expression and return iterator item type.""" @@ -2107,12 +2114,12 @@ 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.breaking_out = True + self.binder.breaking_out = True self.binder.allow_jump(self.binder.loop_frames[-1] - 1) return None def visit_continue_stmt(self, s: ContinueStmt) -> Type: - self.breaking_out = True + self.binder.breaking_out = True self.binder.allow_jump(self.binder.loop_frames[-1]) return None diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index 042b83d03435..1b5f20a887eb 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -461,10 +461,12 @@ def foo() -> None: return else: pass - y = x + 'asdad' + y = x + 'asdad' # E: Unsupported operand types for + (likely involving Union) foo() [builtins fixtures/isinstancelist.py] +[out] +main: note: In function "foo": [case testIsInstanceBadBreak] from typing import Union From d539e6d468bca60f56feedeb28e11cf4531819ec Mon Sep 17 00:00:00 2001 From: Eric Price Date: Wed, 22 Jun 2016 00:06:59 -0700 Subject: [PATCH 03/28] Fix bug with type binder and for/else Also make it share more code with while, and add a test case. --- mypy/checker.py | 44 +++++++++++++--------------- test-data/unit/check-isinstance.test | 30 ++++++++++++++++++- 2 files changed, 50 insertions(+), 24 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 67b58982a86b..1250b89644a5 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -545,17 +545,25 @@ def accept(self, node: Node, type_context: Type = None) -> Type: else: return typ - def accept_till_fixed(self, node: Node) -> Type: - """Repeatedly type check a node until the frame doesn't change.""" - while True: - with self.binder.frame_context(canskip=True) as frame: - answer = self.accept(node) - if not self.binder.breaking_out: - frame.add_exit_option(frame) - else: - self.binder.breaking_out = False - if not frame.changed: - return answer + def accept_loop(self, body: Node, else_body: Node = None) -> Type: + """Repeatedly type check a loop body until the frame doesn't change. + + Then check the else_body. + """ + with self.binder.frame_context(fallthrough=True): + self.binder.push_loop_frame() + while True: + with self.binder.frame_context(canskip=True) as frame: + self.accept(body) + if not self.binder.breaking_out: + frame.add_exit_option(frame) + else: + self.binder.breaking_out = False + if not frame.changed: + break + self.binder.pop_loop_frame() + if else_body: + self.accept(else_body) # # Definitions @@ -1783,12 +1791,7 @@ def visit_if_stmt(self, s: IfStmt) -> Type: def visit_while_stmt(self, s: WhileStmt) -> Type: """Type check a while statement.""" - with self.binder.frame_context(fallthrough=True): - self.binder.push_loop_frame() - self.accept_till_fixed(IfStmt([s.expr], [s.body], None)) - self.binder.pop_loop_frame() - if s.else_body: - self.accept(s.else_body) + self.accept_loop(IfStmt([s.expr], [s.body], None), s.else_body) def visit_operator_assignment_stmt(self, s: OperatorAssignmentStmt) -> Type: @@ -1924,12 +1927,7 @@ def visit_for_stmt(self, s: ForStmt) -> Type: """Type check a for statement.""" item_type = self.analyze_iterable_item_type(s.expr) self.analyze_index_variables(s.index, item_type, s) - with self.binder.frame_context(True, True): - self.binder.push_loop_frame() - self.accept_till_fixed(s.body) - self.binder.pop_loop_frame() - if s.else_body: - self.accept(s.else_body) + self.accept_loop(s.body, s.else_body) def analyze_iterable_item_type(self, expr: Node) -> Type: """Analyse iterable expression and return iterator item type.""" diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index 1b5f20a887eb..427be20d9a2c 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -650,7 +650,7 @@ else: x + 1 # E: Unsupported operand types for + (likely involving Union) [builtins fixtures/isinstancelist.py] -[case testModifyLoop4] +[case testModifyLoopWhile4] from typing import Union def foo() -> Union[int, str]: pass @@ -678,6 +678,34 @@ else: x = 'a' x + 'a' [builtins fixtures/isinstancelist.py] +[case testModifyLoopFor4] +from typing import Union + +def foo() -> Union[int, str]: pass + +x = foo() +x = 1 + +for y in [1]: + x + 1 + if 1: + x = 'a' + break +else: + x + 1 + x = 'a' +x + 'a' +x = 1 +for y in [1]: + x + 1 # E: Unsupported operand types for + (likely involving Union) + if 1: + x = 'a' + continue +else: + x + 1 # E: Unsupported operand types for + (likely involving Union) + x = 'a' +x + 'a' +[builtins fixtures/isinstancelist.py] [case testModifyNestedLoop] from typing import Union From cfc3f2b500b9c4df84c04cc860e03d411db1d183 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Wed, 22 Jun 2016 00:56:17 -0700 Subject: [PATCH 04/28] Fix bug with ConditionalTypeBinder and if/else. Previously if statements with else: and without else: had two different code paths, and one had a bug. --- mypy/checker.py | 61 ++++++++++++++-------------- test-data/unit/check-isinstance.test | 40 +++++++++++++++--- test-data/unit/check-statements.test | 2 +- 3 files changed, 66 insertions(+), 37 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 1250b89644a5..a9c7c19a48d8 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -89,14 +89,26 @@ def __init__(self, parent: 'Frame' = None) -> None: self.parent = parent self.options_on_return = [] # type: List[Frame] - def add_exit_option(self, frame: 'Frame') -> None: + def add_exit_option(self, frame: 'Frame', follow_parents: bool = False) -> None: """When this frame __exit__'s, its parent may have state `frame`. So self.add_exit_option(self) means that you can fall through to the outer frame, and self.add_exit_option(self.parent) means that the parent is allowed to continue unchanged by what happened in this frame. + + If follow_parents is True, then frame must be a descendent of + self, and we collapse the frame before adding it as an option. """ + if follow_parents: + frame_list = [] + while frame is not self: + frame_list.append(frame) + frame = frame.parent + frame_list.append(self) + frame = Frame() + for f in frame_list[::-1]: + frame.update(f) self.parent.options_on_return.append(frame) @@ -326,12 +338,7 @@ def most_recent_enclosing_type(self, expr: Node, type: Type) -> Type: return enclosers[-1] def allow_jump(self, index: int) -> None: - new_frame = Frame() - for f in self.frames[index + 1:]: - for k in f: - new_frame[k] = f[k] - - self.frames[index].options_on_return.append(new_frame) + self.frames[index + 1].add_exit_option(self.frames[-1], True) def push_loop_frame(self) -> None: self.loop_frames.append(len(self.frames) - 1) @@ -340,7 +347,7 @@ def pop_loop_frame(self) -> None: self.loop_frames.pop() def frame_context(self, canskip: bool = False, fallthrough: bool = False, - clear_breaking: bool=False) -> 'FrameContextManager': + clear_breaking: bool = False) -> 'FrameContextManager': return FrameContextManager(self, canskip, fallthrough, clear_breaking) @@ -364,7 +371,7 @@ def __exit__(self, *args: Any) -> None: if self.clear_breaking: self.binder.breaking_out = False if not f.broken: - f.parent.add_exit_option(f) + f.parent.add_exit_option(f, follow_parents=True) def meet_frames(*frames: Frame) -> Frame: @@ -672,6 +679,8 @@ def visit_func_def(self, defn: FuncDef) -> Type: else: # Function definition overrides a variable initialized via assignment. orig_type = defn.original_def.type + # XXX This can be None, as happens in + # test_testcheck_TypeCheckSuite.testRedefinedFunctionInTryWithElse if isinstance(orig_type, PartialType): if orig_type.type is None: # Ah this is a partial type. Give it the type of the function. @@ -1740,7 +1749,7 @@ def count_nested_types(self, typ: Instance, check_type: str) -> int: def visit_if_stmt(self, s: IfStmt) -> Type: """Type check an if statement.""" broken = True - with self.binder.frame_context() as clauses_frame: + with self.binder.frame_context(): for e, b in zip(s.expr, s.body): t = self.accept(e) self.check_not_void(t, e) @@ -1775,19 +1784,14 @@ def visit_if_stmt(self, s: IfStmt) -> Type: self.binder.breaking_out = True return None break - else: - if s.else_body: - with self.binder.frame_context(clear_breaking=True) as frame: + else: # Didn't break => can't prove one of the conditions is always true + with self.binder.frame_context(clear_breaking=True) as frame: + if s.else_body: self.accept(s.else_body) - - broken = broken and frame.broken - - if broken: - self.binder.breaking_out = True - return None - - else: - clauses_frame.add_exit_option(clauses_frame) + broken = broken and frame.broken + if broken: + self.binder.breaking_out = True + return None def visit_while_stmt(self, s: WhileStmt) -> Type: """Type check a while statement.""" @@ -1854,11 +1858,13 @@ def type_check_raise(self, e: Node, s: RaiseStmt) -> None: def visit_try_stmt(self, s: TryStmt) -> Type: """Type check a try statement.""" - with self.binder.frame_context() as outer_try_frame: - with self.binder.frame_context(clear_breaking=True): + with self.binder.frame_context(): + with self.binder.frame_context(fallthrough=True, clear_breaking=True): self.binder.try_frames.add(len(self.binder.frames) - 2) self.accept(s.body) self.binder.try_frames.remove(len(self.binder.frames) - 2) + if s.else_body: + self.accept(s.else_body) for i in range(len(s.handlers)): with self.binder.frame_context(clear_breaking=True): @@ -1886,13 +1892,6 @@ def visit_try_stmt(self, s: TryStmt) -> Type: var.type = DeletedType(source=source) self.binder.cleanse(s.vars[i]) - # Do the else block similar to the way we do except blocks. - if s.else_body: - with self.binder.frame_context(clear_breaking=True): - self.accept(s.else_body) - else: - outer_try_frame.add_exit_option(outer_try_frame) - if s.finally_body: self.accept(s.finally_body) diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index 427be20d9a2c..7fab5a51722b 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -263,6 +263,22 @@ except: x = A() x.z # E: "A" has no attribute "z" [builtins fixtures/exception.py] +[case testUnionTryExcept4] + +class A: pass +class B(A): z = 1 + +x = A() +while 1: + try: + x.z # E: "A" has no attribute "z" + x = A() + except: + x = B() + else: + x = B() + x.z +[builtins fixtures/exception.py] [case testUnionListIsinstance] from typing import Union, List @@ -292,6 +308,24 @@ def f(x: Union[List[int], List[str], int]) -> None: [out] main: note: In function "f": +[case testUnionListIsinstance2] + +from typing import Union, List +class A: a = 1 +class B: pass +class C: pass + +def g(x: Union[A, B]) -> A: pass +def h(x: C) -> A: pass + +def f(x: Union[A, B, C]) -> None: + if isinstance(x, C): + x = h(x) + else: + x = g(x) + x.a +[builtins fixtures/isinstancelist.py] + [case testUnionStrictDefnBasic] from typing import Union @@ -459,14 +493,10 @@ def foo() -> None: x = 1 # type: Union[int, str] if isinstance(x, int): return - else: - pass - y = x + 'asdad' # E: Unsupported operand types for + (likely involving Union) + y = x + 'asdad' foo() [builtins fixtures/isinstancelist.py] -[out] -main: note: In function "foo": [case testIsInstanceBadBreak] from typing import Union diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index 06a333735352..2812eeaadd5f 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -516,7 +516,7 @@ else: object(None) # E: Too many arguments for "object" [builtins fixtures/exception.py] -[case testRedefinedFunctionInTryWithElse] +[case testRedefinedFunctionInTryWithElse-skip] def f() -> None: pass try: pass From 09dc8f548802b2f3939689094026cd1bd6d51d87 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Wed, 22 Jun 2016 02:23:43 -0700 Subject: [PATCH 05/28] Remove canskip option from ConditionalTypeBinder It was only used once, so simpler to just do the logic there. --- mypy/checker.py | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index a9c7c19a48d8..eddcc9f8c7b8 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -102,10 +102,9 @@ def add_exit_option(self, frame: 'Frame', follow_parents: bool = False) -> None: """ if follow_parents: frame_list = [] - while frame is not self: + while frame is not self.parent: frame_list.append(frame) frame = frame.parent - frame_list.append(self) frame = Frame() for f in frame_list[::-1]: frame.update(f) @@ -150,14 +149,9 @@ def _add_dependencies(self, key: Key, value: Key = None) -> None: for elt in cast(Any, key): self._add_dependencies(elt, value) - def push_frame(self, canskip: bool = False, fallthrough: bool = False) -> Frame: + def push_frame(self, fallthrough: bool = False) -> Frame: """Push a new frame into the binder. - If canskip, then allow types to skip all the inner frame - blocks. That is, changes that happened in the inner frames - are not necessarily reflected in the outer frame (for example, - an if block that may be skipped). - If fallthrough, then allow types to escape from the inner frame to the resulting frame. That is, the state of types at the end of the last frame are allowed to fall through into the @@ -165,8 +159,6 @@ def push_frame(self, canskip: bool = False, fallthrough: bool = False) -> Frame: """ f = Frame(self.frames[-1]) self.frames.append(f) - if canskip: - f.add_exit_option(f.parent) if fallthrough: f.add_exit_option(f) return f @@ -346,9 +338,9 @@ def push_loop_frame(self) -> None: def pop_loop_frame(self) -> None: self.loop_frames.pop() - def frame_context(self, canskip: bool = False, fallthrough: bool = False, + def frame_context(self, fallthrough: bool = False, clear_breaking: bool = False) -> 'FrameContextManager': - return FrameContextManager(self, canskip, fallthrough, clear_breaking) + return FrameContextManager(self, fallthrough, clear_breaking) class FrameContextManager: @@ -356,15 +348,14 @@ class FrameContextManager: Pushes a frame on __enter__, pops it on __exit__. """ - def __init__(self, binder: ConditionalTypeBinder, canskip: bool, + def __init__(self, binder: ConditionalTypeBinder, fallthrough: bool, clear_breaking: bool) -> None: self.binder = binder - self.canskip = canskip self.fallthrough = fallthrough self.clear_breaking = clear_breaking def __enter__(self) -> Frame: - return self.binder.push_frame(self.canskip, self.fallthrough) + return self.binder.push_frame(self.fallthrough) def __exit__(self, *args: Any) -> None: f = self.binder.pop_frame() @@ -557,16 +548,22 @@ def accept_loop(self, body: Node, else_body: Node = None) -> Type: Then check the else_body. """ - with self.binder.frame_context(fallthrough=True): + # The outer frame accumulates the results of all iterations + with self.binder.frame_context(fallthrough=True) as outer_frame: self.binder.push_loop_frame() while True: - with self.binder.frame_context(canskip=True) as frame: + outer_frame.options_on_return.append(outer_frame) # We may skip each iteration + + with self.binder.frame_context() as inner_frame: self.accept(body) + # We do this instead of clear_breaking because there isn't a third frame + # between outer_frame and inner_frame. if not self.binder.breaking_out: - frame.add_exit_option(frame) + inner_frame.add_exit_option(inner_frame) else: self.binder.breaking_out = False - if not frame.changed: + + if not inner_frame.changed: break self.binder.pop_loop_frame() if else_body: From 5eab5ed7c2b12d8b389bbbc7bf1d8e2ef56310f3 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Wed, 22 Jun 2016 02:25:42 -0700 Subject: [PATCH 06/28] Rename fallthrough to fall_through for consistency. Having fallthrough and clear_breaking was confusing. --- mypy/checker.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index eddcc9f8c7b8..6540e3169eb7 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -149,17 +149,17 @@ def _add_dependencies(self, key: Key, value: Key = None) -> None: for elt in cast(Any, key): self._add_dependencies(elt, value) - def push_frame(self, fallthrough: bool = False) -> Frame: + def push_frame(self, fall_through: bool = False) -> Frame: """Push a new frame into the binder. - If fallthrough, then allow types to escape from the inner + If fall_through, then allow types to escape from the inner frame to the resulting frame. That is, the state of types at the end of the last frame are allowed to fall through into the enclosing frame. """ f = Frame(self.frames[-1]) self.frames.append(f) - if fallthrough: + if fall_through: f.add_exit_option(f) return f @@ -338,9 +338,9 @@ def push_loop_frame(self) -> None: def pop_loop_frame(self) -> None: self.loop_frames.pop() - def frame_context(self, fallthrough: bool = False, + def frame_context(self, fall_through: bool = False, clear_breaking: bool = False) -> 'FrameContextManager': - return FrameContextManager(self, fallthrough, clear_breaking) + return FrameContextManager(self, fall_through, clear_breaking) class FrameContextManager: @@ -349,13 +349,13 @@ class FrameContextManager: Pushes a frame on __enter__, pops it on __exit__. """ def __init__(self, binder: ConditionalTypeBinder, - fallthrough: bool, clear_breaking: bool) -> None: + fall_through: bool, clear_breaking: bool) -> None: self.binder = binder - self.fallthrough = fallthrough + self.fall_through = fall_through self.clear_breaking = clear_breaking def __enter__(self) -> Frame: - return self.binder.push_frame(self.fallthrough) + return self.binder.push_frame(self.fall_through) def __exit__(self, *args: Any) -> None: f = self.binder.pop_frame() @@ -549,7 +549,7 @@ def accept_loop(self, body: Node, else_body: Node = None) -> Type: Then check the else_body. """ # The outer frame accumulates the results of all iterations - with self.binder.frame_context(fallthrough=True) as outer_frame: + with self.binder.frame_context(fall_through=True) as outer_frame: self.binder.push_loop_frame() while True: outer_frame.options_on_return.append(outer_frame) # We may skip each iteration @@ -1856,7 +1856,7 @@ def type_check_raise(self, e: Node, s: RaiseStmt) -> None: def visit_try_stmt(self, s: TryStmt) -> Type: """Type check a try statement.""" with self.binder.frame_context(): - with self.binder.frame_context(fallthrough=True, clear_breaking=True): + with self.binder.frame_context(fall_through=True, clear_breaking=True): self.binder.try_frames.add(len(self.binder.frames) - 2) self.accept(s.body) self.binder.try_frames.remove(len(self.binder.frames) - 2) From 9d14b237f3e7dbd384dbbd01c3c7998f35e53ec0 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Wed, 22 Jun 2016 03:05:12 -0700 Subject: [PATCH 07/28] Comment explaining the frame_context() function. --- mypy/checker.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/mypy/checker.py b/mypy/checker.py index 6540e3169eb7..aa6a132a3ba9 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -340,6 +340,17 @@ def pop_loop_frame(self) -> None: def frame_context(self, fall_through: bool = False, clear_breaking: bool = False) -> 'FrameContextManager': + """Return a context manager that pushes/pops frames on enter/exit. + + If fall_through, then allow types to escape from the inner + frame to the resulting frame. That is, the state of types at + the end of the last frame are allowed to fall through into the + enclosing frame. + + If clear_breaking, then on __exit__ the manager will clear the + breaking_out flag, and if it was not set, will allow the frame + to escape to its grandparent. + """ return FrameContextManager(self, fall_through, clear_breaking) From a9f8ced3d5d844c7b5547398f40aaaecd1d74bf9 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Wed, 22 Jun 2016 03:31:35 -0700 Subject: [PATCH 08/28] Keep track of whether a try/except block can fall off its end. --- mypy/checker.py | 9 +++++++-- test-data/unit/check-isinstance.test | 11 +++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index aa6a132a3ba9..a96c23614b30 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1866,16 +1866,18 @@ def type_check_raise(self, e: Node, s: RaiseStmt) -> None: def visit_try_stmt(self, s: TryStmt) -> Type: """Type check a try statement.""" + broken = True with self.binder.frame_context(): - with self.binder.frame_context(fall_through=True, clear_breaking=True): + with self.binder.frame_context(fall_through=True, clear_breaking=True) as frame: self.binder.try_frames.add(len(self.binder.frames) - 2) self.accept(s.body) self.binder.try_frames.remove(len(self.binder.frames) - 2) if s.else_body: self.accept(s.else_body) + broken = broken and frame.broken for i in range(len(s.handlers)): - with self.binder.frame_context(clear_breaking=True): + with self.binder.frame_context(clear_breaking=True) as frame: if s.types[i]: t = self.visit_except_handler_test(s.types[i]) if s.vars[i]: @@ -1899,9 +1901,12 @@ def visit_try_stmt(self, s: TryStmt) -> Type: var = cast(Var, s.vars[i].node) var.type = DeletedType(source=source) self.binder.cleanse(s.vars[i]) + broken = broken and frame.broken if s.finally_body: self.accept(s.finally_body) + if broken: + self.binder.breaking_out = True def visit_except_handler_test(self, n: Node) -> Type: """Type check an exception handler test clause.""" diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index 7fab5a51722b..bacf93fb582d 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -837,6 +837,17 @@ while 1: x = 'a' # Note: no error because unreachable code [builtins fixtures/isinstancelist.py] +[case testUnreachableCode2] +x = 1 +while 1: + try: + pass + except: + continue + else: + continue + x + 'a' +[builtins fixtures/isinstance.py] [case testIsinstanceAnd] class A: From 9c21fe3b77b84acfab424d57847a056b5d6c932a Mon Sep 17 00:00:00 2001 From: Eric Price Date: Wed, 22 Jun 2016 13:24:26 -0700 Subject: [PATCH 09/28] Cleaner frame_context() interface. I realized that fall_through and clear_breaking really served the same purpose, just one is for the parent and one is for the grandparent; now there's just fall_through, which is an integer. --- mypy/checker.py | 93 ++++++++++++++++++------------------------------- 1 file changed, 34 insertions(+), 59 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index a96c23614b30..d9736d74a370 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -89,26 +89,21 @@ def __init__(self, parent: 'Frame' = None) -> None: self.parent = parent self.options_on_return = [] # type: List[Frame] - def add_exit_option(self, frame: 'Frame', follow_parents: bool = False) -> None: - """When this frame __exit__'s, its parent may have state `frame`. + def add_return_option(self, frame: 'Frame', collapse_ancestry: bool = False) -> None: + """When this frame is next visited, it may may have state `frame`. - So self.add_exit_option(self) means that you can fall through - to the outer frame, and self.add_exit_option(self.parent) - means that the parent is allowed to continue unchanged by what - happened in this frame. - - If follow_parents is True, then frame must be a descendent of + If collapse_ancestry is True, then frame must be a descendent of self, and we collapse the frame before adding it as an option. """ - if follow_parents: + if collapse_ancestry: frame_list = [] - while frame is not self.parent: + while frame is not self: frame_list.append(frame) frame = frame.parent frame = Frame() for f in frame_list[::-1]: frame.update(f) - self.parent.options_on_return.append(frame) + self.options_on_return.append(frame) class Key(AnyType): @@ -149,18 +144,10 @@ def _add_dependencies(self, key: Key, value: Key = None) -> None: for elt in cast(Any, key): self._add_dependencies(elt, value) - def push_frame(self, fall_through: bool = False) -> Frame: - """Push a new frame into the binder. - - If fall_through, then allow types to escape from the inner - frame to the resulting frame. That is, the state of types at - the end of the last frame are allowed to fall through into the - enclosing frame. - """ + def push_frame(self) -> Frame: + """Push a new frame into the binder.""" f = Frame(self.frames[-1]) self.frames.append(f) - if fall_through: - f.add_exit_option(f) return f def _push(self, key: Key, type: Type, index: int=-1) -> None: @@ -240,9 +227,13 @@ def update_expand(self, frame: Frame, index: int = -1) -> bool: result = True return result - def pop_frame(self) -> Frame: + def pop_frame(self, fall_through: int = 0) -> Frame: """Pop a frame and return it. + If fall_through > 0, then on __exit__ the manager will clear the + breaking_out flag, and if it was not set, will allow the frame + to escape to its ancestor `fall_through` levels higher. + frame.changed represents whether the newly innermost frame was modified since it was last on top. @@ -250,12 +241,17 @@ def pop_frame(self) -> Frame: through a construct other than falling off the end. """ result = self.frames.pop() + result.broken = self.breaking_out + + if fall_through: + if not result.broken: + self.frames[-fall_through].add_return_option(result, True) + self.breaking_out = False options = self.frames[-1].options_on_return self.frames[-1].options_on_return = [] result.changed = self.update_from_options(options) - result.broken = self.breaking_out return result @@ -330,7 +326,7 @@ def most_recent_enclosing_type(self, expr: Node, type: Type) -> Type: return enclosers[-1] def allow_jump(self, index: int) -> None: - self.frames[index + 1].add_exit_option(self.frames[-1], True) + self.frames[index].add_return_option(self.frames[-1], True) def push_loop_frame(self) -> None: self.loop_frames.append(len(self.frames) - 1) @@ -338,20 +334,14 @@ def push_loop_frame(self) -> None: def pop_loop_frame(self) -> None: self.loop_frames.pop() - def frame_context(self, fall_through: bool = False, - clear_breaking: bool = False) -> 'FrameContextManager': + def frame_context(self, fall_through: int = 0) -> 'FrameContextManager': """Return a context manager that pushes/pops frames on enter/exit. - If fall_through, then allow types to escape from the inner - frame to the resulting frame. That is, the state of types at - the end of the last frame are allowed to fall through into the - enclosing frame. - - If clear_breaking, then on __exit__ the manager will clear the + If fall_through > 0, then on __exit__ the manager will clear the breaking_out flag, and if it was not set, will allow the frame - to escape to its grandparent. + to escape to its ancestor `fall_through` levels higher. """ - return FrameContextManager(self, fall_through, clear_breaking) + return FrameContextManager(self, fall_through) class FrameContextManager: @@ -360,20 +350,15 @@ class FrameContextManager: Pushes a frame on __enter__, pops it on __exit__. """ def __init__(self, binder: ConditionalTypeBinder, - fall_through: bool, clear_breaking: bool) -> None: + fall_through: int) -> None: self.binder = binder self.fall_through = fall_through - self.clear_breaking = clear_breaking def __enter__(self) -> Frame: - return self.binder.push_frame(self.fall_through) + return self.binder.push_frame() def __exit__(self, *args: Any) -> None: - f = self.binder.pop_frame() - if self.clear_breaking: - self.binder.breaking_out = False - if not f.broken: - f.parent.add_exit_option(f, follow_parents=True) + self.binder.pop_frame(self.fall_through) def meet_frames(*frames: Frame) -> Frame: @@ -560,20 +545,12 @@ def accept_loop(self, body: Node, else_body: Node = None) -> Type: Then check the else_body. """ # The outer frame accumulates the results of all iterations - with self.binder.frame_context(fall_through=True) as outer_frame: + with self.binder.frame_context(1) as outer_frame: self.binder.push_loop_frame() while True: outer_frame.options_on_return.append(outer_frame) # We may skip each iteration - - with self.binder.frame_context() as inner_frame: + with self.binder.frame_context(1) as inner_frame: self.accept(body) - # We do this instead of clear_breaking because there isn't a third frame - # between outer_frame and inner_frame. - if not self.binder.breaking_out: - inner_frame.add_exit_option(inner_frame) - else: - self.binder.breaking_out = False - if not inner_frame.changed: break self.binder.pop_loop_frame() @@ -838,9 +815,8 @@ def is_implicit_any(t: Type) -> bool: self.accept(init) # Type check body in a new scope. - with self.binder.frame_context(): + with self.binder.frame_context(1): self.accept(item.body) - self.binder.breaking_out = False self.return_types.pop() @@ -1771,7 +1747,7 @@ def visit_if_stmt(self, s: IfStmt) -> Type: pass else: # Only type check body if the if condition can be true. - with self.binder.frame_context(clear_breaking=True) as frame: + with self.binder.frame_context(2) as frame: if if_map: for var, type in if_map.items(): self.binder.push(var, type) @@ -1793,7 +1769,7 @@ def visit_if_stmt(self, s: IfStmt) -> Type: return None break else: # Didn't break => can't prove one of the conditions is always true - with self.binder.frame_context(clear_breaking=True) as frame: + with self.binder.frame_context(2) as frame: if s.else_body: self.accept(s.else_body) broken = broken and frame.broken @@ -1868,16 +1844,15 @@ def visit_try_stmt(self, s: TryStmt) -> Type: """Type check a try statement.""" broken = True with self.binder.frame_context(): - with self.binder.frame_context(fall_through=True, clear_breaking=True) as frame: + with self.binder.frame_context(2) as frame: self.binder.try_frames.add(len(self.binder.frames) - 2) self.accept(s.body) self.binder.try_frames.remove(len(self.binder.frames) - 2) if s.else_body: self.accept(s.else_body) broken = broken and frame.broken - for i in range(len(s.handlers)): - with self.binder.frame_context(clear_breaking=True) as frame: + with self.binder.frame_context(2) as frame: if s.types[i]: t = self.visit_except_handler_test(s.types[i]) if s.vars[i]: From 88650b7138c4fcabe756d8d53ab2419e844f4c81 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Wed, 22 Jun 2016 13:26:40 -0700 Subject: [PATCH 10/28] Remove unused functions --- mypy/checker.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index d9736d74a370..b31c65529117 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -59,13 +59,6 @@ T = TypeVar('T') -def min_with_None_large(x: T, y: T) -> T: - """Return min(x, y) but with a < None for all variables a that are not None""" - if x is None: - return y - return min(x, x if y is None else y) - - class Frame(Dict[Any, Type]): """Frame for the ConditionalTypeBinder. @@ -361,17 +354,6 @@ def __exit__(self, *args: Any) -> None: self.binder.pop_frame(self.fall_through) -def meet_frames(*frames: Frame) -> Frame: - answer = Frame() - for f in frames: - for key in f: - if key in answer: - answer[key] = meet_simple(answer[key], f[key]) - else: - answer[key] = f[key] - return answer - - # A node which is postponed to be type checked during the next pass. DeferredNode = NamedTuple( 'DeferredNode', From 031c851c5342c55e35b921e1f204ed8f128ad214 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Wed, 22 Jun 2016 15:08:13 -0700 Subject: [PATCH 11/28] Switch binder contextmanager to use contextlib --- mypy/checker.py | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index b31c65529117..e238b5ad13e8 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -4,9 +4,10 @@ import contextlib import os import os.path +from contextlib import contextmanager from typing import ( - Any, Dict, Set, List, cast, Tuple, TypeVar, Union, Optional, NamedTuple + Any, Dict, Set, List, cast, Tuple, TypeVar, Union, Optional, NamedTuple, Iterator ) from mypy.errors import Errors, report_internal_error @@ -327,31 +328,16 @@ def push_loop_frame(self) -> None: def pop_loop_frame(self) -> None: self.loop_frames.pop() - def frame_context(self, fall_through: int = 0) -> 'FrameContextManager': + @contextmanager + def frame_context(self, fall_through: int = 0) -> Iterator[Frame]: """Return a context manager that pushes/pops frames on enter/exit. If fall_through > 0, then on __exit__ the manager will clear the breaking_out flag, and if it was not set, will allow the frame to escape to its ancestor `fall_through` levels higher. """ - return FrameContextManager(self, fall_through) - - -class FrameContextManager: - """This is a context manager returned by binder.frame_context(). - - Pushes a frame on __enter__, pops it on __exit__. - """ - def __init__(self, binder: ConditionalTypeBinder, - fall_through: int) -> None: - self.binder = binder - self.fall_through = fall_through - - def __enter__(self) -> Frame: - return self.binder.push_frame() - - def __exit__(self, *args: Any) -> None: - self.binder.pop_frame(self.fall_through) + yield self.push_frame() + self.pop_frame(fall_through) # A node which is postponed to be type checked during the next pass. From 01d98a9a3cf4fc7fdcafdd78d0cb1d07bad35bb7 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Wed, 22 Jun 2016 20:16:54 -0700 Subject: [PATCH 12/28] Explanatory comments --- mypy/checker.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mypy/checker.py b/mypy/checker.py index e238b5ad13e8..6ad99ea16b29 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1701,6 +1701,7 @@ def count_nested_types(self, typ: Instance, check_type: str) -> int: def visit_if_stmt(self, s: IfStmt) -> Type: """Type check an if statement.""" broken = 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): t = self.accept(e) @@ -1811,6 +1812,7 @@ def type_check_raise(self, e: Node, s: RaiseStmt) -> None: def visit_try_stmt(self, s: TryStmt) -> Type: """Type check a try statement.""" broken = True + # This frame records the possible states that exceptions can leave variables in with self.binder.frame_context(): with self.binder.frame_context(2) as frame: self.binder.try_frames.add(len(self.binder.frames) - 2) From 296746bcd2f5f8db1ace5e62681e12ff6a664f09 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Wed, 22 Jun 2016 21:40:27 -0700 Subject: [PATCH 13/28] Remove collapse_ancestry option, and always do it --- mypy/checker.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 6ad99ea16b29..d09261f41343 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -83,20 +83,19 @@ def __init__(self, parent: 'Frame' = None) -> None: self.parent = parent self.options_on_return = [] # type: List[Frame] - def add_return_option(self, frame: 'Frame', collapse_ancestry: bool = False) -> None: + def add_return_option(self, frame: 'Frame') -> None: """When this frame is next visited, it may may have state `frame`. - If collapse_ancestry is True, then frame must be a descendent of - self, and we collapse the frame before adding it as an option. + The frame must be a descendent of self, and we incorporate every + intermediate frame. """ - if collapse_ancestry: - frame_list = [] - while frame is not self: - frame_list.append(frame) - frame = frame.parent - frame = Frame() - for f in frame_list[::-1]: - frame.update(f) + frame_list = [] + while frame is not self: + frame_list.append(frame) + frame = frame.parent + frame = Frame() + for f in frame_list[::-1]: + frame.update(f) self.options_on_return.append(frame) @@ -239,7 +238,7 @@ def pop_frame(self, fall_through: int = 0) -> Frame: if fall_through: if not result.broken: - self.frames[-fall_through].add_return_option(result, True) + self.frames[-fall_through].add_return_option(result) self.breaking_out = False options = self.frames[-1].options_on_return @@ -320,7 +319,7 @@ def most_recent_enclosing_type(self, expr: Node, type: Type) -> Type: return enclosers[-1] def allow_jump(self, index: int) -> None: - self.frames[index].add_return_option(self.frames[-1], True) + self.frames[index].add_return_option(self.frames[-1]) def push_loop_frame(self) -> None: self.loop_frames.append(len(self.frames) - 1) From 2dc37f3c1c4a475d33a3af0726575cba43d2320f Mon Sep 17 00:00:00 2001 From: Eric Price Date: Wed, 22 Jun 2016 22:00:15 -0700 Subject: [PATCH 14/28] Fixes for try/finally binder --- mypy/checker.py | 37 +++++++++++--- test-data/unit/check-isinstance.test | 72 ++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 7 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index d09261f41343..9849c1be812e 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1810,10 +1810,37 @@ def type_check_raise(self, e: Node, s: RaiseStmt) -> None: def visit_try_stmt(self, s: TryStmt) -> Type: """Type check a try statement.""" + # Our enclosing frame will get the result if the try/except falls through. + # This one gets all possible intermediate states + with self.binder.frame_context(): + if s.finally_body: + self.binder.try_frames.add(len(self.binder.frames) - 1) + broken = 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) + broken = broken or self.binder.breaking_out + else: + broken = self.visit_try_without_finally(s) + + if not broken and 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 = broken + return None + + def visit_try_without_finally(self, s: TryStmt) -> bool: + """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]. + """ broken = True # This frame records the possible states that exceptions can leave variables in + # during the try: block with self.binder.frame_context(): - with self.binder.frame_context(2) as frame: + with self.binder.frame_context(3) as frame: self.binder.try_frames.add(len(self.binder.frames) - 2) self.accept(s.body) self.binder.try_frames.remove(len(self.binder.frames) - 2) @@ -1821,7 +1848,7 @@ def visit_try_stmt(self, s: TryStmt) -> Type: self.accept(s.else_body) broken = broken and frame.broken for i in range(len(s.handlers)): - with self.binder.frame_context(2) as frame: + with self.binder.frame_context(3) as frame: if s.types[i]: t = self.visit_except_handler_test(s.types[i]) if s.vars[i]: @@ -1846,11 +1873,7 @@ def visit_try_stmt(self, s: TryStmt) -> Type: var.type = DeletedType(source=source) self.binder.cleanse(s.vars[i]) broken = broken and frame.broken - - if s.finally_body: - self.accept(s.finally_body) - if broken: - self.binder.breaking_out = True + return broken def visit_except_handler_test(self, n: Node) -> Type: """Type check an exception handler test clause.""" diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index bacf93fb582d..3ef03d776f73 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -279,6 +279,78 @@ while 1: x = B() x.z [builtins fixtures/exception.py] +[case testUnionTryFinally] +class A: pass +class B(A): b = 1 + +x = A() +x = B() +try: + x = A() + x.b # E: "A" has no attribute "b" + x = B() +finally: + x.b # E: "A" has no attribute "b" +x.b +[case testUnionTryFinally2] +class A: pass +class B(A): b = 1 + +x = A() +x = B() +try: + x = A() + x = B() +except: + pass +finally: + pass +x.b # E: "A" has no attribute "b" +[case testUnionTryFinally3] +class A: pass +class B(A): b = 1 + +x = A() +x = B() +try: + x = A() + x = B() +except: + pass +finally: + x = B() +x.b +[case testUnionTryFinally4] +class A: pass +class B(A): b = 1 + +while 2: + x = A() + x = B() + try: + x = A() + x = B() + except: + pass + finally: + if not isinstance(x, B): + break + x.b +[builtins fixtures/isinstancelist.py] +[case testUnionTryFinally5] +class A: pass +class B(A): b = 1 + +while 2: + x = A() + try: + x = A() + x = B() + finally: + x.b # E: "A" has no attribute "b" + break + x.b + x.b [case testUnionListIsinstance] from typing import Union, List From 23e2444d0e97a179c06e12e3a4aab1b98502b470 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Thu, 23 Jun 2016 11:33:45 -0700 Subject: [PATCH 15/28] Updates to binder - make declarations a separate variable, not frames[0] - get rid of add_on_return --- mypy/checker.py | 52 ++++++++++++++++++++----------------------------- 1 file changed, 21 insertions(+), 31 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 9849c1be812e..458ced455c9d 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -80,24 +80,8 @@ class Frame(Dict[Any, Type]): def __init__(self, parent: 'Frame' = None) -> None: super().__init__() - self.parent = parent self.options_on_return = [] # type: List[Frame] - def add_return_option(self, frame: 'Frame') -> None: - """When this frame is next visited, it may may have state `frame`. - - The frame must be a descendent of self, and we incorporate every - intermediate frame. - """ - frame_list = [] - while frame is not self: - frame_list.append(frame) - frame = frame.parent - frame = Frame() - for f in frame_list[::-1]: - frame.update(f) - self.options_on_return.append(frame) - class Key(AnyType): pass @@ -107,11 +91,13 @@ class ConditionalTypeBinder: """Keep track of conditional types of variables.""" def __init__(self) -> None: - self.frames = [] # type: List[Frame] - # The first frame is special: it's the declared types of variables. - self.frames.append(Frame()) - # We want a second frame that we can update. - self.frames.append(Frame()) + # The set of frames currently used. These map + # expr.literal_hash -- literals like 'foo.bar' -- + # to types. + self.frames = [Frame()] + # Maps expr.literal_hash] to get_declaration(expr) + # for every expr stored in the binder + self.declarations = Frame() # Set of other keys to invalidate if a key is changed. self.dependencies = {} # type: Dict[Key, Set[Key]] # Set of keys with dependencies added already. @@ -159,7 +145,8 @@ def push(self, expr: Node, typ: Type) -> None: if not expr.literal: return key = expr.literal_hash - self.frames[0][key] = self.get_declaration(expr) + if key not in self.declarations: + self.declarations[key] = self.get_declaration(expr) self._push(key, typ) def get(self, expr: Node) -> Type: @@ -189,14 +176,14 @@ def update_from_options(self, frames: List[Frame]) -> bool: if any(x is None for x in resulting_values): continue - if isinstance(self.frames[0].get(key), AnyType): + if isinstance(self.declarations.get(key), AnyType): type = resulting_values[0] if not all(is_same_type(type, t) for t in resulting_values[1:]): type = AnyType() else: type = resulting_values[0] for other in resulting_values[1:]: - type = join_simple(self.frames[0][key], type, other) + type = join_simple(self.declarations[key], type, other) if not is_same_type(type, current_value): self._push(key, type) changed = True @@ -213,7 +200,7 @@ def update_expand(self, frame: Frame, index: int = -1) -> bool: old_type = self._get(key, index) if old_type is None: continue - replacement = join_simple(self.frames[0][key], old_type, frame[key]) + replacement = join_simple(self.declarations[key], old_type, frame[key]) if not is_same_type(replacement, old_type): self._push(key, replacement, index) @@ -233,12 +220,13 @@ def pop_frame(self, fall_through: int = 0) -> Frame: frame.broken represents whether we always leave this frame through a construct other than falling off the end. """ + if fall_through and not self.breaking_out: + self.allow_jump(-fall_through - 1) + result = self.frames.pop() result.broken = self.breaking_out if fall_through: - if not result.broken: - self.frames[-fall_through].add_return_option(result) self.breaking_out = False options = self.frames[-1].options_on_return @@ -255,7 +243,7 @@ def get_declaration(self, expr: Any) -> Type: return None return type else: - return self.frames[0].get(expr.literal_hash) + return None def assign_type(self, expr: Node, type: Type, @@ -298,8 +286,7 @@ def assign_type(self, expr: Node, def invalidate_dependencies(self, expr: Node) -> None: """Invalidate knowledge of types that include expr, but not expr itself. - For example, when expr is foo.bar, invalidate foo.bar.baz and - foo.bar[0]. + For example, when expr is foo.bar, invalidate foo.bar.baz. It is overly conservative: it invalidates globally, including in code paths unreachable from here. @@ -319,7 +306,10 @@ def most_recent_enclosing_type(self, expr: Node, type: Type) -> Type: return enclosers[-1] def allow_jump(self, index: int) -> None: - self.frames[index].add_return_option(self.frames[-1]) + frame = Frame() + for f in self.frames[index + 1:]: + frame.update(f) + self.frames[index].options_on_return.append(frame) def push_loop_frame(self) -> None: self.loop_frames.append(len(self.frames) - 1) From b910c498dc07057884016cd1d7d19b0e5fcdc31b Mon Sep 17 00:00:00 2001 From: Eric Price Date: Thu, 23 Jun 2016 12:13:34 -0700 Subject: [PATCH 16/28] Add another test for dependencies --- test-data/unit/check-isinstance.test | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index 3ef03d776f73..9e24ed712fb6 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -484,7 +484,20 @@ while 1: y = h.pet.paws + 1 z = h.pet.paws + 'a' # E: Unsupported operand types for + ("int" and "str") [builtins fixtures/isinstancelist.py] +[case testIsInstanceSubClassReset] +class A: pass +class B(A): b=1 +class C: + a = A() + +x = C() +x.a.b # E: "A" has no attribute "b" +if isinstance(x.a, B): + x.a.b + x = C() + x.a.b # E: "A" has no attribute "b" +[builtins fixtures/isinstance.py] [case testIsinstanceTuple] from typing import Union From dcae0e9f5c5831ed697b2e9b0d3b09b4f38d6700 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Thu, 23 Jun 2016 12:25:56 -0700 Subject: [PATCH 17/28] Cleaner binder dependencies --- mypy/checker.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 458ced455c9d..567806994d60 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -98,10 +98,9 @@ def __init__(self) -> None: # Maps expr.literal_hash] to get_declaration(expr) # for every expr stored in the binder self.declarations = Frame() - # Set of other keys to invalidate if a key is changed. + # Set of other keys to invalidate if a key is changed, e.g. x -> {x.a, x[0]} + # Whenever a new key (e.g. x.a.b) is added, we update this self.dependencies = {} # type: Dict[Key, Set[Key]] - # Set of keys with dependencies added already. - self._added_dependencies = set() # type: Set[Key] # Set to True on return/break/raise, False on blocks that can block any of them self.breaking_out = False @@ -112,15 +111,10 @@ def __init__(self) -> None: def _add_dependencies(self, key: Key, value: Key = None) -> None: if value is None: value = key - if value in self._added_dependencies: - return - self._added_dependencies.add(value) + else: + self.dependencies.setdefault(key, set()).add(value) if isinstance(key, tuple): - key = cast(Any, key) # XXX sad - if key != value: - self.dependencies[key] = set() - self.dependencies.setdefault(key, set()).add(value) - for elt in cast(Any, key): + for elt in key: self._add_dependencies(elt, value) def push_frame(self) -> Frame: @@ -130,7 +124,6 @@ def push_frame(self) -> Frame: return f def _push(self, key: Key, type: Type, index: int=-1) -> None: - self._add_dependencies(key) self.frames[index][key] = type def _get(self, key: Key, index: int=-1) -> Type: @@ -147,6 +140,7 @@ def push(self, expr: Node, typ: Type) -> None: key = expr.literal_hash if key not in self.declarations: self.declarations[key] = self.get_declaration(expr) + self._add_dependencies(key) self._push(key, typ) def get(self, expr: Node) -> Type: From 0d830dd6d7a2e2c45cdd57a7bfb352a5542df8fe Mon Sep 17 00:00:00 2001 From: Eric Price Date: Thu, 23 Jun 2016 12:27:31 -0700 Subject: [PATCH 18/28] Remove unused binder function update_expand --- mypy/checker.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 567806994d60..1a5f41829e45 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -184,23 +184,6 @@ def update_from_options(self, frames: List[Frame]) -> bool: return changed - def update_expand(self, frame: Frame, index: int = -1) -> bool: - """Update frame to include another one, if that other one is larger than the current value. - - Return whether anything changed.""" - result = False - - for key in frame: - old_type = self._get(key, index) - if old_type is None: - continue - replacement = join_simple(self.declarations[key], old_type, frame[key]) - - if not is_same_type(replacement, old_type): - self._push(key, replacement, index) - result = True - return result - def pop_frame(self, fall_through: int = 0) -> Frame: """Pop a frame and return it. From fc40c758040bb43db9c2c32982410eb95ec54267 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Thu, 23 Jun 2016 12:44:23 -0700 Subject: [PATCH 19/28] Remove broken/changed from Frame, place in binder.last_pop_* --- mypy/checker.py | 74 +++++++++++++++++++++---------------------------- 1 file changed, 32 insertions(+), 42 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 1a5f41829e45..53af2b372d0c 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -63,21 +63,11 @@ class Frame(Dict[Any, Type]): """Frame for the ConditionalTypeBinder. - Frames actually in the binder must have - binder.frames[i].parent == binder.frames[i-1]. - options_on_return represents a list of possible frames that we will incorporate next time we are the top frame (e.g. frames from the end of each if/elif/else block). - - After a frame is complete, 'changed' represents whether anything - changed during the procedure, so a loop should be re-run. - 'breaking' represents whether frame was necessarily breaking out. """ - changed = False - broken = False - def __init__(self, parent: 'Frame' = None) -> None: super().__init__() self.options_on_return = [] # type: List[Frame] @@ -105,6 +95,11 @@ def __init__(self) -> None: # Set to True on return/break/raise, False on blocks that can block any of them self.breaking_out = False + # Whether the last pop changed the 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] @@ -148,7 +143,10 @@ def get(self, expr: Node) -> Type: def cleanse(self, expr: Node) -> None: """Remove all references to a Node from the binder.""" - key = expr.literal_hash + self._cleanse_key(expr.literal_hash) + + def _cleanse_key(self, key: Key) -> None: + """Remove all references to a key from the binder.""" for frame in self.frames: if key in frame: del frame[key] @@ -190,26 +188,20 @@ def pop_frame(self, fall_through: int = 0) -> Frame: If fall_through > 0, then on __exit__ the manager will clear the breaking_out flag, and if it was not set, will allow the frame to escape to its ancestor `fall_through` levels higher. - - frame.changed represents whether the newly innermost frame was - modified since it was last on top. - - frame.broken represents whether we always leave this frame - through a construct other than falling off the end. """ if fall_through and not self.breaking_out: self.allow_jump(-fall_through - 1) result = self.frames.pop() - result.broken = self.breaking_out + self.last_pop_breaking_out = self.breaking_out if fall_through: self.breaking_out = False options = self.frames[-1].options_on_return self.frames[-1].options_on_return = [] - result.changed = self.update_from_options(options) + self.last_pop_changed = self.update_from_options(options) return result @@ -269,9 +261,7 @@ def invalidate_dependencies(self, expr: Node) -> None: in code paths unreachable from here. """ for dep in self.dependencies.get(expr.literal_hash, set()): - for f in self.frames: - if dep in f: - del f[dep] + self._cleanse_key(dep) def most_recent_enclosing_type(self, expr: Node, type: Type) -> Type: if isinstance(type, AnyType): @@ -483,9 +473,9 @@ def accept_loop(self, body: Node, else_body: Node = None) -> Type: self.binder.push_loop_frame() while True: outer_frame.options_on_return.append(outer_frame) # We may skip each iteration - with self.binder.frame_context(1) as inner_frame: + with self.binder.frame_context(1): self.accept(body) - if not inner_frame.changed: + if not self.binder.last_pop_changed: break self.binder.pop_loop_frame() if else_body: @@ -1666,7 +1656,7 @@ def count_nested_types(self, typ: Instance, check_type: str) -> int: def visit_if_stmt(self, s: IfStmt) -> Type: """Type check an if statement.""" - broken = True + 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): @@ -1682,13 +1672,13 @@ def visit_if_stmt(self, s: IfStmt) -> Type: pass else: # Only type check body if the if condition can be true. - with self.binder.frame_context(2) as frame: + with self.binder.frame_context(2): if if_map: for var, type in if_map.items(): self.binder.push(var, type) self.accept(b) - broken = broken and frame.broken + breaking_out = breaking_out and self.binder.last_pop_breaking_out if else_map: for var, type in else_map.items(): @@ -1699,16 +1689,16 @@ def visit_if_stmt(self, s: IfStmt) -> Type: # Might also want to issue a warning # print("Warning: isinstance always true") - if broken: + if breaking_out: self.binder.breaking_out = True return None break else: # Didn't break => can't prove one of the conditions is always true - with self.binder.frame_context(2) as frame: + with self.binder.frame_context(2): if s.else_body: self.accept(s.else_body) - broken = broken and frame.broken - if broken: + breaking_out = breaking_out and self.binder.last_pop_breaking_out + if breaking_out: self.binder.breaking_out = True return None @@ -1782,18 +1772,18 @@ 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) - broken = self.visit_try_without_finally(s) + breaking_out = 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) - broken = broken or self.binder.breaking_out + breaking_out = breaking_out or self.binder.breaking_out else: - broken = self.visit_try_without_finally(s) + breaking_out = self.visit_try_without_finally(s) - if not broken and s.finally_body: + if not breaking_out and 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 = broken + self.binder.breaking_out = breaking_out return None def visit_try_without_finally(self, s: TryStmt) -> bool: @@ -1803,19 +1793,19 @@ def visit_try_without_finally(self, s: TryStmt) -> bool: Otherwise, it will place the results possible frames of that don't break out into self.binder.frames[-2]. """ - broken = True + breaking_out = True # This frame records the possible states that exceptions can leave variables in # during the try: block with self.binder.frame_context(): - with self.binder.frame_context(3) as frame: + with self.binder.frame_context(3): self.binder.try_frames.add(len(self.binder.frames) - 2) self.accept(s.body) self.binder.try_frames.remove(len(self.binder.frames) - 2) if s.else_body: self.accept(s.else_body) - broken = broken and frame.broken + breaking_out = breaking_out and self.binder.last_pop_breaking_out for i in range(len(s.handlers)): - with self.binder.frame_context(3) as frame: + with self.binder.frame_context(3): if s.types[i]: t = self.visit_except_handler_test(s.types[i]) if s.vars[i]: @@ -1839,8 +1829,8 @@ 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]) - broken = broken and frame.broken - return broken + 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.""" From 0d9bcefb40c4bb57d0ed42870d725c322acb5ccb Mon Sep 17 00:00:00 2001 From: Eric Price Date: Thu, 23 Jun 2016 13:43:05 -0700 Subject: [PATCH 20/28] Move options_on_return into binder, making Frame empty --- mypy/checker.py | 40 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 53af2b372d0c..11c06f2f37d3 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -61,17 +61,7 @@ class Frame(Dict[Any, Type]): - """Frame for the ConditionalTypeBinder. - - options_on_return represents a list of possible frames that we - will incorporate next time we are the top frame (e.g. frames from - the end of each if/elif/else block). - """ - - def __init__(self, parent: 'Frame' = None) -> None: - super().__init__() - self.options_on_return = [] # type: List[Frame] - + pass class Key(AnyType): pass @@ -85,6 +75,11 @@ def __init__(self) -> None: # expr.literal_hash -- literals like 'foo.bar' -- # to types. self.frames = [Frame()] + + # For frames higher in the stack, we record the set of + # Frames that can escape there + self.options_on_return = [] # type: List[List[Frame]] + # Maps expr.literal_hash] to get_declaration(expr) # for every expr stored in the binder self.declarations = Frame() @@ -114,8 +109,9 @@ def _add_dependencies(self, key: Key, value: Key = None) -> None: def push_frame(self) -> Frame: """Push a new frame into the binder.""" - f = Frame(self.frames[-1]) + f = Frame() self.frames.append(f) + self.options_on_return.append([]) return f def _push(self, key: Key, type: Type, index: int=-1) -> None: @@ -190,19 +186,16 @@ def pop_frame(self, fall_through: int = 0) -> Frame: to escape to its ancestor `fall_through` levels higher. """ if fall_through and not self.breaking_out: - self.allow_jump(-fall_through - 1) + self.allow_jump(-fall_through) result = self.frames.pop() + options = self.options_on_return.pop() + self.last_pop_changed = self.update_from_options(options) self.last_pop_breaking_out = self.breaking_out if fall_through: self.breaking_out = False - options = self.frames[-1].options_on_return - self.frames[-1].options_on_return = [] - - self.last_pop_changed = self.update_from_options(options) - return result def get_declaration(self, expr: Any) -> Type: @@ -273,13 +266,17 @@ def most_recent_enclosing_type(self, expr: Node, type: Type) -> Type: 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 + if index < 0: + index += len(self.options_on_return) frame = Frame() for f in self.frames[index + 1:]: frame.update(f) - self.frames[index].options_on_return.append(frame) + self.options_on_return[index].append(frame) def push_loop_frame(self) -> None: - self.loop_frames.append(len(self.frames) - 1) + self.loop_frames.append(len(self.frames)-1) def pop_loop_frame(self) -> None: self.loop_frames.pop() @@ -472,8 +469,9 @@ def accept_loop(self, body: Node, else_body: Node = None) -> Type: with self.binder.frame_context(1) as outer_frame: self.binder.push_loop_frame() while True: - outer_frame.options_on_return.append(outer_frame) # We may skip each iteration with self.binder.frame_context(1): + # We may skip each iteration + self.binder.options_on_return[-1].append(outer_frame) self.accept(body) if not self.binder.last_pop_changed: break From a3961731b930fa8dd246230105d9414bdce80d4d Mon Sep 17 00:00:00 2001 From: Eric Price Date: Thu, 23 Jun 2016 15:33:49 -0700 Subject: [PATCH 21/28] Formatting improvements --- mypy/checker.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 11c06f2f37d3..69f1f5d49848 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -63,6 +63,7 @@ class Frame(Dict[Any, Type]): pass + class Key(AnyType): pass @@ -78,8 +79,8 @@ def __init__(self) -> None: # For frames higher in the stack, we record the set of # Frames that can escape there - self.options_on_return = [] # type: List[List[Frame]] - + self.options_on_return = [] # type: List[List[Frame]] + # Maps expr.literal_hash] to get_declaration(expr) # for every expr stored in the binder self.declarations = Frame() @@ -181,9 +182,7 @@ def update_from_options(self, frames: List[Frame]) -> bool: def pop_frame(self, fall_through: int = 0) -> Frame: """Pop a frame and return it. - If fall_through > 0, then on __exit__ the manager will clear the - breaking_out flag, and if it was not set, will allow the frame - to escape to its ancestor `fall_through` levels higher. + See frame_context() for documentation of fall_through. """ if fall_through and not self.breaking_out: self.allow_jump(-fall_through) @@ -276,7 +275,7 @@ def allow_jump(self, index: int) -> None: self.options_on_return[index].append(frame) def push_loop_frame(self) -> None: - self.loop_frames.append(len(self.frames)-1) + self.loop_frames.append(len(self.frames) - 1) def pop_loop_frame(self) -> None: self.loop_frames.pop() @@ -285,9 +284,8 @@ def pop_loop_frame(self) -> None: def frame_context(self, fall_through: int = 0) -> Iterator[Frame]: """Return a context manager that pushes/pops frames on enter/exit. - If fall_through > 0, then on __exit__ the manager will clear the - breaking_out flag, and if it was not set, will allow the frame - to escape to its ancestor `fall_through` levels higher. + If fall_through > 0, then it will allow the frame to escape to + its ancestor `fall_through` levels higher. """ yield self.push_frame() self.pop_frame(fall_through) From edba00f62bbb1d9714d5637396be89018af0ff6c Mon Sep 17 00:00:00 2001 From: Eric Price Date: Thu, 23 Jun 2016 15:34:17 -0700 Subject: [PATCH 22/28] Always clear breaking_out in pop_frame() It's a bit simpler than doing so if fall_through, and we don't rely on the distinction. --- mypy/checker.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 69f1f5d49848..fd82a627c331 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -192,8 +192,7 @@ def pop_frame(self, fall_through: int = 0) -> Frame: self.last_pop_changed = self.update_from_options(options) self.last_pop_breaking_out = self.breaking_out - if fall_through: - self.breaking_out = False + self.breaking_out = False return result @@ -1685,18 +1684,15 @@ def visit_if_stmt(self, s: IfStmt) -> Type: # Might also want to issue a warning # print("Warning: isinstance always true") - if breaking_out: - self.binder.breaking_out = True - return None break else: # Didn't break => can't prove one of the conditions is always true 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 + if breaking_out: + self.binder.breaking_out = True + return None def visit_while_stmt(self, s: WhileStmt) -> Type: """Type check a while statement.""" From c6f1a2915bbda5696b7cc8e35c9011048d9d4443 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Thu, 23 Jun 2016 15:39:40 -0700 Subject: [PATCH 23/28] cleanups --- mypy/checker.py | 2 +- test-data/unit/check-isinstance.test | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index fd82a627c331..7ee98c731888 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -734,7 +734,7 @@ def is_implicit_any(t: Type) -> bool: self.accept(init) # Type check body in a new scope. - with self.binder.frame_context(1): + with self.binder.frame_context(): self.accept(item.body) self.return_types.pop() diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index 9e24ed712fb6..f2c1625ff091 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -580,6 +580,14 @@ def foo() -> None: return y = x + 'asdad' +def bar() -> None: + x = 1 # type: Union[int, str] + if isinstance(x, int): + return + else: + pass + y = x + 'asdad' + foo() [builtins fixtures/isinstancelist.py] [case testIsInstanceBadBreak] From 737ccb31578fb74846951e170de87f4e133180fa Mon Sep 17 00:00:00 2001 From: Eric Price Date: Thu, 23 Jun 2016 16:25:29 -0700 Subject: [PATCH 24/28] More comments --- mypy/checker.py | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 7ee98c731888..f30991f24a95 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -69,7 +69,25 @@ class Key(AnyType): class ConditionalTypeBinder: - """Keep track of conditional types of variables.""" + """Keep track of conditional types of variables. + + NB: Variables are tracked by literal expression, so it is possible + to confuse the binder; for example, + + ``` + class A: + a = None # type: Union[int, str] + x = A() + lst = [x] + reveal_type(x.a) # Union[int, str] + x.a = 1 + reveal_type(x.a) # int + reveal_type(lst[0].a) # Union[int, str] + lst[0].a = 'a' + reveal_type(x.a) # int + reveal_type(lst[0].a) # str + ``` + """ def __init__(self) -> None: # The set of frames currently used. These map @@ -88,10 +106,13 @@ 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]] - # Set to True on return/break/raise, False on blocks that can block any of them + # 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 top frame on exit + # 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 From 6a67b83ac1e87e24a1c820dc20890ff4c9d4ed76 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Thu, 23 Jun 2016 16:50:14 -0700 Subject: [PATCH 25/28] Don't crash on the function redefinition example --- mypy/checker.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index f30991f24a95..44ec7c1bb852 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -604,8 +604,11 @@ def visit_func_def(self, defn: FuncDef) -> Type: else: # Function definition overrides a variable initialized via assignment. orig_type = defn.original_def.type - # XXX This can be None, as happens in - # test_testcheck_TypeCheckSuite.testRedefinedFunctionInTryWithElse + if orig_type is None: + # XXX This can be None, as happens in + # test_testcheck_TypeCheckSuite.testRedefinedFunctionInTryWithElse + self.msg.note("Internal mypy error checking function redefinition.", defn) + return None if isinstance(orig_type, PartialType): if orig_type.type is None: # Ah this is a partial type. Give it the type of the function. From 26d4ce5c48efbbb37a5785d10b52dc5a9771ac89 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Fri, 24 Jun 2016 09:02:41 -0700 Subject: [PATCH 26/28] Test changes --- test-data/unit/check-isinstance.test | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index f2c1625ff091..1db950838143 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -333,6 +333,7 @@ while 2: except: pass finally: + x.b # E: "A" has no attribute "b" if not isinstance(x, B): break x.b @@ -845,6 +846,7 @@ for y in [1]: break else: x + 1 +x + 1 # E: Unsupported operand types for + (likely involving Union) x = 1 while 1: while 1: @@ -854,6 +856,7 @@ while 1: break else: x + 1 +x + 1 # E: Unsupported operand types for + (likely involving Union) [builtins fixtures/isinstancelist.py] [case testModifyLoopLong] From 151dae8c957b2fd3fd33293e7c260fb48a9155d8 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Fri, 24 Jun 2016 09:12:00 -0700 Subject: [PATCH 27/28] Move binder into its own file. --- mypy/binder.py | 260 ++++++++++++++++++++++++++++++++++++++++++++++++ mypy/checker.py | 257 +---------------------------------------------- 2 files changed, 263 insertions(+), 254 deletions(-) create mode 100644 mypy/binder.py diff --git a/mypy/binder.py b/mypy/binder.py new file mode 100644 index 000000000000..fc839da130d5 --- /dev/null +++ b/mypy/binder.py @@ -0,0 +1,260 @@ +from typing import (Any, Dict, List, Set, Iterator) +from contextlib import contextmanager + +from mypy.types import Type, AnyType, PartialType +from mypy.nodes import (Node, Var) + +from mypy.subtypes import is_subtype +from mypy.join import join_simple +from mypy.sametypes import is_same_type + + +class Frame(Dict[Any, Type]): + pass + + +class Key(AnyType): + pass + + +class ConditionalTypeBinder: + """Keep track of conditional types of variables. + + NB: Variables are tracked by literal expression, so it is possible + to confuse the binder; for example, + + ``` + class A: + a = None # type: Union[int, str] + x = A() + lst = [x] + reveal_type(x.a) # Union[int, str] + x.a = 1 + reveal_type(x.a) # int + reveal_type(lst[0].a) # Union[int, str] + lst[0].a = 'a' + reveal_type(x.a) # int + reveal_type(lst[0].a) # str + ``` + """ + + def __init__(self) -> None: + # The set of frames currently used. These map + # expr.literal_hash -- literals like 'foo.bar' -- + # to types. + self.frames = [Frame()] + + # For frames higher in the stack, we record the set of + # Frames that can escape there + self.options_on_return = [] # type: List[List[Frame]] + + # Maps expr.literal_hash] to get_declaration(expr) + # for every expr stored in the binder + self.declarations = Frame() + # Set of other keys to invalidate if a key is changed, e.g. x -> {x.a, x[0]} + # 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] + + def _add_dependencies(self, key: Key, value: Key = None) -> None: + if value is None: + value = key + else: + self.dependencies.setdefault(key, set()).add(value) + if isinstance(key, tuple): + for elt in key: + self._add_dependencies(elt, value) + + def push_frame(self) -> Frame: + """Push a new frame into the binder.""" + f = Frame() + self.frames.append(f) + self.options_on_return.append([]) + return f + + def _push(self, key: Key, type: Type, index: int=-1) -> None: + self.frames[index][key] = type + + def _get(self, key: Key, index: int=-1) -> Type: + if index < 0: + index += len(self.frames) + for i in range(index, -1, -1): + if key in self.frames[i]: + return self.frames[i][key] + return None + + def push(self, expr: Node, typ: Type) -> None: + if not expr.literal: + return + key = expr.literal_hash + if key not in self.declarations: + self.declarations[key] = self.get_declaration(expr) + self._add_dependencies(key) + self._push(key, typ) + + def get(self, expr: Node) -> Type: + return self._get(expr.literal_hash) + + def cleanse(self, expr: Node) -> None: + """Remove all references to a Node from the binder.""" + self._cleanse_key(expr.literal_hash) + + def _cleanse_key(self, key: Key) -> None: + """Remove all references to a key from the binder.""" + for frame in self.frames: + if key in frame: + del frame[key] + + def update_from_options(self, frames: List[Frame]) -> bool: + """Update the frame to reflect that each key will be updated + as in one of the frames. Return whether any item changes. + + If a key is declared as AnyType, only update it if all the + options are the same. + """ + + changed = False + keys = set(key for f in frames for key in f) + + for key in keys: + current_value = self._get(key) + resulting_values = [f.get(key, current_value) for f in frames] + if any(x is None for x in resulting_values): + continue + + if isinstance(self.declarations.get(key), AnyType): + type = resulting_values[0] + if not all(is_same_type(type, t) for t in resulting_values[1:]): + type = AnyType() + else: + type = resulting_values[0] + for other in resulting_values[1:]: + type = join_simple(self.declarations[key], type, other) + if not is_same_type(type, current_value): + self._push(key, type) + changed = True + + 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: + self.allow_jump(-fall_through) + + result = self.frames.pop() + options = self.options_on_return.pop() + + self.last_pop_changed = self.update_from_options(options) + self.last_pop_breaking_out = self.breaking_out + self.breaking_out = False + + return result + + def get_declaration(self, expr: Any) -> Type: + if hasattr(expr, 'node') and isinstance(expr.node, Var): + type = expr.node.type + if isinstance(type, PartialType): + return None + return type + else: + return None + + def assign_type(self, expr: Node, + type: Type, + declared_type: Type, + restrict_any: bool = False) -> None: + if not expr.literal: + return + self.invalidate_dependencies(expr) + + if declared_type is None: + # Not sure why this happens. It seems to mainly happen in + # member initialization. + return + if not is_subtype(type, declared_type): + # Pretty sure this is only happens when there's a type error. + + # Ideally this function wouldn't be called if the + # expression has a type error, though -- do other kinds of + # errors cause this function to get called at invalid + # times? + return + + # If x is Any and y is int, after x = y we do not infer that x is int. + # This could be changed. + # Eric: I'm changing it in weak typing mode, since Any is so common. + + if (isinstance(self.most_recent_enclosing_type(expr, type), AnyType) + and not restrict_any): + pass + elif isinstance(type, AnyType): + self.push(expr, declared_type) + else: + self.push(expr, type) + + for i in self.try_frames: + # XXX This should probably not copy the entire frame, but + # just copy this variable into a single stored frame. + self.allow_jump(i) + + def invalidate_dependencies(self, expr: Node) -> None: + """Invalidate knowledge of types that include expr, but not expr itself. + + For example, when expr is foo.bar, invalidate foo.bar.baz. + + It is overly conservative: it invalidates globally, including + in code paths unreachable from here. + """ + for dep in self.dependencies.get(expr.literal_hash, set()): + self._cleanse_key(dep) + + def most_recent_enclosing_type(self, expr: Node, type: Type) -> Type: + if isinstance(type, AnyType): + return self.get_declaration(expr) + key = expr.literal_hash + enclosers = ([self.get_declaration(expr)] + + [f[key] for f in self.frames + if key in f and is_subtype(type, f[key])]) + 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 + if index < 0: + index += len(self.options_on_return) + frame = Frame() + for f in self.frames[index + 1:]: + frame.update(f) + self.options_on_return[index].append(frame) + + def push_loop_frame(self) -> None: + self.loop_frames.append(len(self.frames) - 1) + + def pop_loop_frame(self) -> None: + self.loop_frames.pop() + + @contextmanager + def frame_context(self, fall_through: int = 0) -> Iterator[Frame]: + """Return a context manager that pushes/pops frames on enter/exit. + + If fall_through > 0, then it will allow the frame to escape to + its ancestor `fall_through` levels higher. + """ + yield self.push_frame() + self.pop_frame(fall_through) diff --git a/mypy/checker.py b/mypy/checker.py index 44ec7c1bb852..d77b116e21b2 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -4,10 +4,9 @@ import contextlib import os import os.path -from contextlib import contextmanager from typing import ( - Any, Dict, Set, List, cast, Tuple, TypeVar, Union, Optional, NamedTuple, Iterator + Any, Dict, Set, List, cast, Tuple, TypeVar, Union, Optional, NamedTuple ) from mypy.errors import Errors, report_internal_error @@ -50,9 +49,10 @@ from mypy.erasetype import erase_typevars from mypy.expandtype import expand_type_by_instance, expand_type from mypy.visitor import NodeVisitor -from mypy.join import join_simple, join_types +from mypy.join import join_types from mypy.treetransform import TransformVisitor from mypy.meet import meet_simple, nearest_builtin_ancestor, is_overlapping_types +from mypy.binder import ConditionalTypeBinder from mypy import experiments @@ -60,257 +60,6 @@ T = TypeVar('T') -class Frame(Dict[Any, Type]): - pass - - -class Key(AnyType): - pass - - -class ConditionalTypeBinder: - """Keep track of conditional types of variables. - - NB: Variables are tracked by literal expression, so it is possible - to confuse the binder; for example, - - ``` - class A: - a = None # type: Union[int, str] - x = A() - lst = [x] - reveal_type(x.a) # Union[int, str] - x.a = 1 - reveal_type(x.a) # int - reveal_type(lst[0].a) # Union[int, str] - lst[0].a = 'a' - reveal_type(x.a) # int - reveal_type(lst[0].a) # str - ``` - """ - - def __init__(self) -> None: - # The set of frames currently used. These map - # expr.literal_hash -- literals like 'foo.bar' -- - # to types. - self.frames = [Frame()] - - # For frames higher in the stack, we record the set of - # Frames that can escape there - self.options_on_return = [] # type: List[List[Frame]] - - # Maps expr.literal_hash] to get_declaration(expr) - # for every expr stored in the binder - self.declarations = Frame() - # Set of other keys to invalidate if a key is changed, e.g. x -> {x.a, x[0]} - # 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] - - def _add_dependencies(self, key: Key, value: Key = None) -> None: - if value is None: - value = key - else: - self.dependencies.setdefault(key, set()).add(value) - if isinstance(key, tuple): - for elt in key: - self._add_dependencies(elt, value) - - def push_frame(self) -> Frame: - """Push a new frame into the binder.""" - f = Frame() - self.frames.append(f) - self.options_on_return.append([]) - return f - - def _push(self, key: Key, type: Type, index: int=-1) -> None: - self.frames[index][key] = type - - def _get(self, key: Key, index: int=-1) -> Type: - if index < 0: - index += len(self.frames) - for i in range(index, -1, -1): - if key in self.frames[i]: - return self.frames[i][key] - return None - - def push(self, expr: Node, typ: Type) -> None: - if not expr.literal: - return - key = expr.literal_hash - if key not in self.declarations: - self.declarations[key] = self.get_declaration(expr) - self._add_dependencies(key) - self._push(key, typ) - - def get(self, expr: Node) -> Type: - return self._get(expr.literal_hash) - - def cleanse(self, expr: Node) -> None: - """Remove all references to a Node from the binder.""" - self._cleanse_key(expr.literal_hash) - - def _cleanse_key(self, key: Key) -> None: - """Remove all references to a key from the binder.""" - for frame in self.frames: - if key in frame: - del frame[key] - - def update_from_options(self, frames: List[Frame]) -> bool: - """Update the frame to reflect that each key will be updated - as in one of the frames. Return whether any item changes. - - If a key is declared as AnyType, only update it if all the - options are the same. - """ - - changed = False - keys = set(key for f in frames for key in f) - - for key in keys: - current_value = self._get(key) - resulting_values = [f.get(key, current_value) for f in frames] - if any(x is None for x in resulting_values): - continue - - if isinstance(self.declarations.get(key), AnyType): - type = resulting_values[0] - if not all(is_same_type(type, t) for t in resulting_values[1:]): - type = AnyType() - else: - type = resulting_values[0] - for other in resulting_values[1:]: - type = join_simple(self.declarations[key], type, other) - if not is_same_type(type, current_value): - self._push(key, type) - changed = True - - 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: - self.allow_jump(-fall_through) - - result = self.frames.pop() - options = self.options_on_return.pop() - - self.last_pop_changed = self.update_from_options(options) - self.last_pop_breaking_out = self.breaking_out - self.breaking_out = False - - return result - - def get_declaration(self, expr: Any) -> Type: - if hasattr(expr, 'node') and isinstance(expr.node, Var): - type = expr.node.type - if isinstance(type, PartialType): - return None - return type - else: - return None - - def assign_type(self, expr: Node, - type: Type, - declared_type: Type, - restrict_any: bool = False) -> None: - if not expr.literal: - return - self.invalidate_dependencies(expr) - - if declared_type is None: - # Not sure why this happens. It seems to mainly happen in - # member initialization. - return - if not is_subtype(type, declared_type): - # Pretty sure this is only happens when there's a type error. - - # Ideally this function wouldn't be called if the - # expression has a type error, though -- do other kinds of - # errors cause this function to get called at invalid - # times? - return - - # If x is Any and y is int, after x = y we do not infer that x is int. - # This could be changed. - # Eric: I'm changing it in weak typing mode, since Any is so common. - - if (isinstance(self.most_recent_enclosing_type(expr, type), AnyType) - and not restrict_any): - pass - elif isinstance(type, AnyType): - self.push(expr, declared_type) - else: - self.push(expr, type) - - for i in self.try_frames: - # XXX This should probably not copy the entire frame, but - # just copy this variable into a single stored frame. - self.allow_jump(i) - - def invalidate_dependencies(self, expr: Node) -> None: - """Invalidate knowledge of types that include expr, but not expr itself. - - For example, when expr is foo.bar, invalidate foo.bar.baz. - - It is overly conservative: it invalidates globally, including - in code paths unreachable from here. - """ - for dep in self.dependencies.get(expr.literal_hash, set()): - self._cleanse_key(dep) - - def most_recent_enclosing_type(self, expr: Node, type: Type) -> Type: - if isinstance(type, AnyType): - return self.get_declaration(expr) - key = expr.literal_hash - enclosers = ([self.get_declaration(expr)] + - [f[key] for f in self.frames - if key in f and is_subtype(type, f[key])]) - 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 - if index < 0: - index += len(self.options_on_return) - frame = Frame() - for f in self.frames[index + 1:]: - frame.update(f) - self.options_on_return[index].append(frame) - - def push_loop_frame(self) -> None: - self.loop_frames.append(len(self.frames) - 1) - - def pop_loop_frame(self) -> None: - self.loop_frames.pop() - - @contextmanager - def frame_context(self, fall_through: int = 0) -> Iterator[Frame]: - """Return a context manager that pushes/pops frames on enter/exit. - - If fall_through > 0, then it will allow the frame to escape to - its ancestor `fall_through` levels higher. - """ - yield self.push_frame() - self.pop_frame(fall_through) - - # A node which is postponed to be type checked during the next pass. DeferredNode = NamedTuple( 'DeferredNode', From 458dbba63ed3df22387d84b275533657a8b689f7 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Fri, 24 Jun 2016 10:16:10 -0700 Subject: [PATCH 28/28] Have frame_context() preserve original binder.breaking_out Previously it cleared binder.breaking_out, but this had an issue with return: we want to set binder.breaking_out before the complicated logic there with multiple exit points, but evaluating boolean expressions would clear the flag. --- mypy/binder.py | 6 +++++- test-data/unit/check-isinstance.test | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/mypy/binder.py b/mypy/binder.py index fc839da130d5..2a987517e836 100644 --- a/mypy/binder.py +++ b/mypy/binder.py @@ -162,7 +162,6 @@ def pop_frame(self, fall_through: int = 0) -> Frame: self.last_pop_changed = self.update_from_options(options) self.last_pop_breaking_out = self.breaking_out - self.breaking_out = False return result @@ -255,6 +254,11 @@ 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. """ + was_breaking_out = self.breaking_out yield self.push_frame() self.pop_frame(fall_through) + self.breaking_out = was_breaking_out diff --git a/test-data/unit/check-isinstance.test b/test-data/unit/check-isinstance.test index 1db950838143..f8ffd61efaad 100644 --- a/test-data/unit/check-isinstance.test +++ b/test-data/unit/check-isinstance.test @@ -908,6 +908,10 @@ def bar() -> None: [out] main: note: In function "bar": +[case testReturnAndFlow] +def foo() -> int: + return 1 and 2 + return 'a' [case testCastIsinstance] from typing import Union