Tighten FakeInfo and fix crashes in --quick mode

[ilevkivskyi]

The idea was proposed by Jukka in #3285 (comment)

This could (hopefully) fix #3281 for real.
Also fixes #3278
Guido, could you please try to crash this with your code base?

[gvanrossum]

No, it's not fixed. Using the recipe from #3281 I still get a crash on the second --quick run, this time ending in

  File "/Users/guido/src/mypy/mypy/types.py", line 411, in accept
    return visitor.visit_instance(self)
  File "/Users/guido/src/mypy/mypy/subtypes.py", line 156, in visit_instance
    zip(t.args, right.args, right.type.defn.type_vars))
  File "/Users/guido/src/mypy/mypy/subtypes.py", line 155, in <genexpr>
    for lefta, righta, tvar in
  File "/Users/guido/src/mypy/mypy/subtypes.py", line 34, in check_type_parameter
    return is_equivalent(lefta, righta, check_type_parameter)
  File "/Users/guido/src/mypy/mypy/subtypes.py", line 94, in is_equivalent
    is_subtype(a, b, type_parameter_checker, ignore_pos_arg_names=ignore_pos_arg_names)
  File "/Users/guido/src/mypy/mypy/subtypes.py", line 75, in is_subtype
    elif is_named_instance(right, 'builtins.type'):
  File "/Users/guido/src/mypy/mypy/types.py", line 1641, in is_named_instance
    t.type.fullname() == fullname)
  File "/Users/guido/src/mypy/mypy/nodes.py", line 2231, in __getattribute__
    raise AssertionError('De-serialization failure: TypeInfo not fixed')
AssertionError: De-serialization failure: TypeInfo not fixed

I can dig into this more if you want to.

[ilevkivskyi]

Could you please show a larger part of the traceback? (It will be helpful since I can't reproduce this.)

[gvanrossum]

Sorry! Here it is.

Traceback (most recent call last):
  File "/usr/local/lib/python3.5/runpy.py", line 170, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/local/lib/python3.5/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/Users/guido/src/mypy/mypy/__main__.py", line 5, in <module>
    main(None)
  File "/Users/guido/src/mypy/mypy/main.py", line 46, in main
    res = type_check_only(sources, bin_dir, options)
  File "/Users/guido/src/mypy/mypy/main.py", line 93, in type_check_only
    options=options)
  File "/Users/guido/src/mypy/mypy/build.py", line 188, in build
    graph = dispatch(sources, manager)
  File "/Users/guido/src/mypy/mypy/build.py", line 1595, in dispatch
    process_graph(graph, manager)
  File "/Users/guido/src/mypy/mypy/build.py", line 1838, in process_graph
    process_stale_scc(graph, scc, manager)
  File "/Users/guido/src/mypy/mypy/build.py", line 1937, in process_stale_scc
    graph[id].type_check_first_pass()
  File "/Users/guido/src/mypy/mypy/build.py", line 1510, in type_check_first_pass
    self.type_checker.check_first_pass()
  File "/Users/guido/src/mypy/mypy/checker.py", line 177, in check_first_pass
    self.accept(d)
  File "/Users/guido/src/mypy/mypy/checker.py", line 264, in accept
    stmt.accept(self)
  File "/Users/guido/src/mypy/mypy/nodes.py", line 749, in accept
    return visitor.visit_class_def(self)
  File "/Users/guido/src/mypy/mypy/checker.py", line 1080, in visit_class_def
    self.accept(defn.defs)
  File "/Users/guido/src/mypy/mypy/checker.py", line 264, in accept
    stmt.accept(self)
  File "/Users/guido/src/mypy/mypy/nodes.py", line 810, in accept
    return visitor.visit_block(self)
  File "/Users/guido/src/mypy/mypy/checker.py", line 1173, in visit_block
    self.accept(s)
  File "/Users/guido/src/mypy/mypy/checker.py", line 264, in accept
    stmt.accept(self)
  File "/Users/guido/src/mypy/mypy/nodes.py", line 564, in accept
    return visitor.visit_func_def(self)
  File "/Users/guido/src/mypy/mypy/checker.py", line 510, in visit_func_def
    self.check_func_item(defn, name=defn.name())
  File "/Users/guido/src/mypy/mypy/checker.py", line 569, in check_func_item
    self.check_func_def(defn, typ, name)
  File "/Users/guido/src/mypy/mypy/checker.py", line 718, in check_func_def
    self.accept(item.body)
  File "/Users/guido/src/mypy/mypy/checker.py", line 264, in accept
    stmt.accept(self)
  File "/Users/guido/src/mypy/mypy/nodes.py", line 810, in accept
    return visitor.visit_block(self)
  File "/Users/guido/src/mypy/mypy/checker.py", line 1173, in visit_block
    self.accept(s)
  File "/Users/guido/src/mypy/mypy/checker.py", line 264, in accept
    stmt.accept(self)
  File "/Users/guido/src/mypy/mypy/nodes.py", line 969, in accept
    return visitor.visit_if_stmt(self)
  File "/Users/guido/src/mypy/mypy/checker.py", line 1921, in visit_if_stmt
    self.accept(b)
  File "/Users/guido/src/mypy/mypy/checker.py", line 264, in accept
    stmt.accept(self)
  File "/Users/guido/src/mypy/mypy/nodes.py", line 810, in accept
    return visitor.visit_block(self)
  File "/Users/guido/src/mypy/mypy/checker.py", line 1173, in visit_block
    self.accept(s)
  File "/Users/guido/src/mypy/mypy/checker.py", line 264, in accept
    stmt.accept(self)
  File "/Users/guido/src/mypy/mypy/nodes.py", line 854, in accept
    return visitor.visit_assignment_stmt(self)
  File "/Users/guido/src/mypy/mypy/checker.py", line 1180, in visit_assignment_stmt
    self.check_assignment(s.lvalues[-1], s.rvalue, s.type is None, s.new_syntax)
  File "/Users/guido/src/mypy/mypy/checker.py", line 1240, in check_assignment
    instance_type, lvalue_type, rvalue, lvalue)
  File "/Users/guido/src/mypy/mypy/checker.py", line 1731, in check_member_assignment
    rvalue_type = self.check_simple_assignment(attribute_type, rvalue, context)
  File "/Users/guido/src/mypy/mypy/checker.py", line 1704, in check_simple_assignment
    rvalue_type = self.expr_checker.accept(rvalue, lvalue_type)
  File "/Users/guido/src/mypy/mypy/checkexpr.py", line 2058, in accept
    typ = node.accept(self)
  File "/Users/guido/src/mypy/mypy/nodes.py", line 1302, in accept
    return visitor.visit_call_expr(self)
  File "/Users/guido/src/mypy/mypy/checkexpr.py", line 201, in visit_call_expr
    ret_type = self.check_call_expr_with_callee_type(callee_type, e)
  File "/Users/guido/src/mypy/mypy/checkexpr.py", line 341, in check_call_expr_with_callee_type
    e.arg_names, callable_node=e.callee)[0]
  File "/Users/guido/src/mypy/mypy/checkexpr.py", line 400, in check_call
    messages=arg_messages)
  File "/Users/guido/src/mypy/mypy/checkexpr.py", line 839, in check_argument_types
    actual + 1, i + 1, callee, context, messages)
  File "/Users/guido/src/mypy/mypy/checkexpr.py", line 870, in check_arg
    elif not is_subtype(caller_type, callee_type):
  File "/Users/guido/src/mypy/mypy/subtypes.py", line 78, in is_subtype
    ignore_pos_arg_names=ignore_pos_arg_names))
  File "/Users/guido/src/mypy/mypy/types.py", line 411, in accept
    return visitor.visit_instance(self)
  File "/Users/guido/src/mypy/mypy/subtypes.py", line 156, in visit_instance
    zip(t.args, right.args, right.type.defn.type_vars))
  File "/Users/guido/src/mypy/mypy/subtypes.py", line 155, in <genexpr>
    for lefta, righta, tvar in
  File "/Users/guido/src/mypy/mypy/subtypes.py", line 34, in check_type_parameter
    return is_equivalent(lefta, righta, check_type_parameter)
  File "/Users/guido/src/mypy/mypy/subtypes.py", line 94, in is_equivalent
    is_subtype(a, b, type_parameter_checker, ignore_pos_arg_names=ignore_pos_arg_names)
  File "/Users/guido/src/mypy/mypy/subtypes.py", line 75, in is_subtype
    elif is_named_instance(right, 'builtins.type'):
  File "/Users/guido/src/mypy/mypy/types.py", line 1641, in is_named_instance
    t.type.fullname() == fullname)
  File "/Users/guido/src/mypy/mypy/nodes.py", line 2231, in __getattribute__
    raise AssertionError('De-serialization failure: TypeInfo not fixed')
AssertionError: De-serialization failure: TypeInfo not fixed

[gvanrossum]

Maybe what also might help: the line where it fails is a call to a static method on a class that was locally imported (on the line before, inside a method).

[ilevkivskyi]

@gvanrossum
It looks like TypeInfos were not fixed in --quick mode in situations with import cycles. I fixed this by setting TypeInfo to a dummy value with a suggestive name, if we can't find it in cache in --quick mode. Does this fix your crash?

[gvanrossum]

Sadly, still a crash:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/local/lib/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/Users/guido/src/mypy/mypy/__main__.py", line 5, in <module>
    main(None)
  File "/Users/guido/src/mypy/mypy/main.py", line 46, in main
    res = type_check_only(sources, bin_dir, options)
  File "/Users/guido/src/mypy/mypy/main.py", line 93, in type_check_only
    options=options)
  File "/Users/guido/src/mypy/mypy/build.py", line 188, in build
    graph = dispatch(sources, manager)
  File "/Users/guido/src/mypy/mypy/build.py", line 1595, in dispatch
    process_graph(graph, manager)
  File "/Users/guido/src/mypy/mypy/build.py", line 1838, in process_graph
    process_stale_scc(graph, scc, manager)
  File "/Users/guido/src/mypy/mypy/build.py", line 1929, in process_stale_scc
    graph[id].fix_cross_refs()
  File "/Users/guido/src/mypy/mypy/build.py", line 1366, in fix_cross_refs
    self.manager.options.quick_and_dirty)
  File "/Users/guido/src/mypy/mypy/fixup.py", line 22, in fixup_module_pass_one
    node_fixer.visit_symbol_table(tree.names)
  File "/Users/guido/src/mypy/mypy/fixup.py", line 101, in visit_symbol_table
    self.visit_type_info(value.node)
  File "/Users/guido/src/mypy/mypy/fixup.py", line 56, in visit_type_info
    self.visit_symbol_table(info.names)
  File "/Users/guido/src/mypy/mypy/fixup.py", line 103, in visit_symbol_table
    value.node.accept(self)
  File "/Users/guido/src/mypy/mypy/nodes.py", line 564, in accept
    return visitor.visit_func_def(self)
  File "/Users/guido/src/mypy/mypy/fixup.py", line 111, in visit_func_def
    func.type.accept(self.type_fixer)
  File "/Users/guido/src/mypy/mypy/types.py", line 659, in accept
    return visitor.visit_callable_type(self)
  File "/Users/guido/src/mypy/mypy/fixup.py", line 186, in visit_callable_type
    argt.accept(self)
  File "/Users/guido/src/mypy/mypy/types.py", line 411, in accept
    return visitor.visit_instance(self)
  File "/Users/guido/src/mypy/mypy/fixup.py", line 175, in visit_instance
    a.accept(self)
  File "/Users/guido/src/mypy/mypy/types.py", line 879, in accept
    return visitor.visit_tuple_type(self)
  File "/Users/guido/src/mypy/mypy/fixup.py", line 216, in visit_tuple_type
    tt.fallback.accept(self)
  File "/Users/guido/src/mypy/mypy/types.py", line 411, in accept
    return visitor.visit_instance(self)
  File "/Users/guido/src/mypy/mypy/fixup.py", line 172, in visit_instance
    assert node is None and self.quick_and_dirty
AssertionError

[ilevkivskyi]

OK, but it is better now, it hits one of asserts that I just put there. I will investigate further.

[ilevkivskyi]

@gvanrossum It looks like I can reproduce now your crash, I have very similar traceback.
Could you please try the new version of the PR?

[gvanrossum]

No crash!

[ilevkivskyi]

No crash!

Have you tried only --quick mode or also combinations with -i and/or normal mode? Where do you see the complain that I added <stale cache: consider running mypy without --quick> in the place of a previous crash?

[gvanrossum]

Hm, I don't see that "stale cache" message at all in my output. Should I? Does this mean I'm not testing the right things? Here's how I test:

cd ~/src/client

export PYTHONPATH=~/src/mypy
export MYPY=$HOME/v36/bin/'python3 -m mypy'
rm -rf .mypy_cache
./ci/mypy3_all.sh -V
echo === Initial run ===
time ./ci/mypy3_all.sh --strict-optional -i "$@" >@crash1
echo === First quick run ===
time ./ci/mypy3_all.sh --strict-optional --quick "$@" >@crash2
echo === Second quick run ===
time ./ci/mypy3_all.sh --strict-optional --quick "$@" >@crash3
echo === End ===

(Where ci/mypy3_all.sh is a script that runs mypy with a specific set of files, and ~/src/mypy contains the mypy sources checked out as your PR.)

[ilevkivskyi]

Hm, I don't see that "stale cache" message at all in my output.

Interesting, in my crash scenario this message appeared where the crash previously happened.

Should I? Does this mean I'm not testing the right things?

Not necessarily, my idea is very simple: in --quick mode, fixup.lookup_qualified can return None. Previously this lead to a crash, since the corresponding instance was not fixed in this case, since there is nothing to substitute in Instance.type instead of None. I just create a dummy class for this case that has name <stale cache: consider running mypy without --quick>.

This might sound strange, but I think it is better than always giving an error. Maybe those "unfixed" instances are actually not interesting for user. The user will only see this message when doing something with "unfixed" types. Also, it is clear that --quick mode is inherently somewhat unsafe, so probably there is no point to remind about this every time. Finally, we need to put something in Instance.type, to make this a non-blocking error.

[gvanrossum]

We're getting close to the code freeze for the release. Do you think this should go in (then I can make time to review) or not (then I'll punt until after the release)?

[ilevkivskyi]

We're getting close to the code freeze for the release. Do you think this should go in (then I can make time to review) or not (then I'll punt until after the release)?

I think we probably should wait with this. On one hand it would be better to expose the "bug-searching" __getattribute__ to more code, but on the other hand this could put too much effort on users. So I would vote for postponing this after release. I will make another PR soon that will turn on asserts from #3305, I think it is also better to postpone this after release. We could also ask what @JukkaL thinks about this.

[gvanrossum]

I think that's a wise decision. If Jukka disagrees he can merge this when he emerges tomorrow morning.

[gvanrossum]

I can actually no longer repro the crash I saw, which could mean any number of things. I propose to finish the review and merge this, and then we'll see.