Mypy daemon crash related to decorator

[JukkaL]

This mypy daemon test case triggers a crash:

[case testUndefinedDecoratorRefersToInit]
import m

[file m.py]
class C:
    def __init__(self, x: int) -> None: ...

[file m.py.2]
from typing_extensions import Self

class C:
    def __init__(self, x: int) -> None: ...

    @classmethod
    @copy_only_params_from_method(__init__)
    async def create(cls, *args, **kwargs) -> Self:
        res = cls(*args, **kwargs)
        await res.async_init()
        return res

    async def async_init(self) -> None: ...
[builtins fixtures/tuple.pyi]
[out]
<omitted>

I'm not yet sure what is going on, and it may be possible to simplify this further.

Traceback:

/Users/jukka/src/mypy/mypy/test/testfinegrained.py:116: in run_case
    output, triggered = self.perform_step(
/Users/jukka/src/mypy/mypy/test/testfinegrained.py:220: in perform_step
    new_messages = self.run_check(server, sources)
/Users/jukka/src/mypy/mypy/test/testfinegrained.py:170: in run_check
    response = server.check(sources, export_types=False, is_tty=False, terminal_width=-1)
/Users/jukka/src/mypy/mypy/dmypy_server.py:431: in check
    messages = self.fine_grained_increment(sources, explicit_export_types=export_types)
/Users/jukka/src/mypy/mypy/dmypy_server.py:589: in fine_grained_increment
    messages = self.fine_grained_manager.update(changed, removed)
/Users/jukka/src/mypy/mypy/server/update.py:265: in update
    result = self.update_one(
/Users/jukka/src/mypy/mypy/server/update.py:367: in update_one
    result = self.update_module(next_id, next_path, next_id in removed_set, followed)
/Users/jukka/src/mypy/mypy/server/update.py:432: in update_module
    result = update_module_isolated(
/Users/jukka/src/mypy/mypy/server/update.py:668: in update_module_isolated
    state.type_check_first_pass()
/Users/jukka/src/mypy/mypy/build.py:2344: in type_check_first_pass
    self.type_checker().check_first_pass()
/Users/jukka/src/mypy/mypy/checker.py:524: in check_first_pass
    self.accept(d)
/Users/jukka/src/mypy/mypy/checker.py:637: in accept
    report_internal_error(err, self.errors.file, stmt.line, self.errors, self.options)
/Users/jukka/src/mypy/mypy/errors.py:1331: in report_internal_error
    raise err
/Users/jukka/src/mypy/mypy/checker.py:635: in accept
    stmt.accept(self)
/Users/jukka/src/mypy/mypy/nodes.py:1445: in accept
    return visitor.visit_class_def(self)
/Users/jukka/src/mypy/mypy/checker.py:2665: in visit_class_def
    self.accept(defn.defs)
/Users/jukka/src/mypy/mypy/checker.py:637: in accept
    report_internal_error(err, self.errors.file, stmt.line, self.errors, self.options)
/Users/jukka/src/mypy/mypy/errors.py:1331: in report_internal_error
    raise err
/Users/jukka/src/mypy/mypy/checker.py:635: in accept
    stmt.accept(self)
/Users/jukka/src/mypy/mypy/nodes.py:1542: in accept
    return visitor.visit_block(self)
/Users/jukka/src/mypy/mypy/checker.py:3126: in visit_block
    self.accept(s)
/Users/jukka/src/mypy/mypy/checker.py:637: in accept
    report_internal_error(err, self.errors.file, stmt.line, self.errors, self.options)
/Users/jukka/src/mypy/mypy/errors.py:1331: in report_internal_error
    raise err
/Users/jukka/src/mypy/mypy/checker.py:635: in accept
    stmt.accept(self)
/Users/jukka/src/mypy/mypy/nodes.py:1127: in accept
    return visitor.visit_decorator(self)
/Users/jukka/src/mypy/mypy/checker.py:5412: in visit_decorator
    self.visit_decorator_inner(e)
/Users/jukka/src/mypy/mypy/checker.py:5419: in visit_decorator_inner
    self.check_func_item(e.func, name=e.func.name, allow_empty=allow_empty)
/Users/jukka/src/mypy/mypy/checker.py:1231: in check_func_item
    self.check_func_def(defn, typ, name, allow_empty)
/Users/jukka/src/mypy/mypy/checker.py:1504: in check_func_def
    self.accept(item.body)
/Users/jukka/src/mypy/mypy/checker.py:637: in accept
    report_internal_error(err, self.errors.file, stmt.line, self.errors, self.options)
/Users/jukka/src/mypy/mypy/errors.py:1331: in report_internal_error
    raise err
/Users/jukka/src/mypy/mypy/checker.py:635: in accept
    stmt.accept(self)
/Users/jukka/src/mypy/mypy/nodes.py:1542: in accept
    return visitor.visit_block(self)
/Users/jukka/src/mypy/mypy/checker.py:3126: in visit_block
    self.accept(s)
/Users/jukka/src/mypy/mypy/checker.py:637: in accept
    report_internal_error(err, self.errors.file, stmt.line, self.errors, self.options)
/Users/jukka/src/mypy/mypy/errors.py:1331: in report_internal_error
    raise err
/Users/jukka/src/mypy/mypy/checker.py:635: in accept
    stmt.accept(self)
/Users/jukka/src/mypy/mypy/nodes.py:1629: in accept
    return visitor.visit_assignment_stmt(self)
/Users/jukka/src/mypy/mypy/checker.py:3177: in visit_assignment_stmt
    self.check_assignment(s.lvalues[-1], s.rvalue, s.type is None, s.new_syntax)
/Users/jukka/src/mypy/mypy/checker.py:3389: in check_assignment
    rvalue_type = self.expr_checker.accept(rvalue, type_context=type_context)
/Users/jukka/src/mypy/mypy/checkexpr.py:6050: in accept
    report_internal_error(
/Users/jukka/src/mypy/mypy/errors.py:1331: in report_internal_error
    raise err
/Users/jukka/src/mypy/mypy/checkexpr.py:6046: in accept
    typ = self.accept_maybe_cache(node, type_context=type_context)
/Users/jukka/src/mypy/mypy/checkexpr.py:6081: in accept_maybe_cache
    typ = node.accept(self)
/Users/jukka/src/mypy/mypy/nodes.py:2253: in accept
    return visitor.visit_call_expr(self)
/Users/jukka/src/mypy/mypy/checkexpr.py:494: in visit_call_expr
    return self.visit_call_expr_inner(e, allow_none_return=allow_none_return)
/Users/jukka/src/mypy/mypy/checkexpr.py:631: in visit_call_expr_inner
    ret_type = self.check_call_expr_with_callee_type(
/Users/jukka/src/mypy/mypy/checkexpr.py:1481: in check_call_expr_with_callee_type
    ret_type, callee_type = self.check_call(
/Users/jukka/src/mypy/mypy/checkexpr.py:1629: in check_call
    item = self.analyze_type_type_callee(callee.item, context)
/Users/jukka/src/mypy/mypy/checkexpr.py:1885: in analyze_type_type_callee
    callee = self.analyze_type_type_callee(get_proper_type(item.upper_bound), context)
/Users/jukka/src/mypy/mypy/checkexpr.py:1863: in analyze_type_type_callee
    res = type_object_type(item.type, self.named_type)
/Users/jukka/src/mypy/mypy/typeops.py:177: in type_object_type
    init_index = info.mro.index(init_method.node.info)
E   ValueError: <TypeInfo m.C> is not in list
---------------------------------------------------- Captured stdout call ----------------------------------------------------
tmp/m.py:7: error: Name "copy_only_params_from_method" is not defined
---------------------------------------------------- Captured stderr call ----------------------------------------------------
tmp/m.py:9: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 1.19.0+dev.fd0526545419028090f064ba4c1fa6e576ccdd6b.dirty
tmp/m.py:9: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 1.19.0+dev.fd0526545419028090f064ba4c1fa6e576ccdd6b.dirty
tmp/m.py:9: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 1.19.0+dev.fd0526545419028090f064ba4c1fa6e576ccdd6b.dirty
tmp/m.py:6: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 1.19.0+dev.fd0526545419028090f064ba4c1fa6e576ccdd6b.dirty
tmp/m.py:4: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 1.19.0+dev.fd0526545419028090f064ba4c1fa6e576ccdd6b.dirty
tmp/m.py:3: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 1.19.0+dev.fd0526545419028090f064ba4c1fa6e576ccdd6b.dirty
tmp/m.py: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 1.19.0+dev.fd0526545419028090f064ba4c1fa6e576ccdd6b.dirty

[JukkaL]

This reproduces on 1.17.0 so not a regression.

[ilevkivskyi]

Errors like TypeInfo is not in list usually mean there is a missing dependency, or more rarely an AST merge flaw. After an increment some expressions may point to old symbols, thus causing this crash. In you case it is probably the latter (AST merge flaw), so that some expressions point to the previous version of C TypeInfo (that is obviously not in the MRO of the new version of C TypeInfo)

[ilevkivskyi]

Looking more at the example, it seems to me the real problem is cls beeing Self. Self has an upper bound of C and I bet that one (the upper bound) is not fixed.

[ilevkivskyi]

Actually a simpler repro

[case testReferenceToInit]
import m

[file m.py]
class C:
    def __init__(self, x: int) -> None: ...

[file m.py.2]
class C:
    def __init__(self, x: int) -> None: ...

    __init__

    @classmethod
    def test(cls) -> None:
        cls(42)
[builtins fixtures/tuple.pyi]
[out]

[ilevkivskyi]

Even simpler

[case testReferenceToInit]
import m

[file m.py]
class C:
    def __init__(self) -> None: ...

[file m.py.2]
class C:
    def __init__(self) -> None: ...

    __init__

C()
[builtins fixtures/tuple.pyi]
[out]

Probably a problem with fixup order.

[sterliakov]

This?

diff --git a/mypy/server/astmerge.py b/mypy/server/astmerge.py
index cda1d20fb..08dc2f7da 100644
--- a/mypy/server/astmerge.py
+++ b/mypy/server/astmerge.py
@@ -278,8 +278,7 @@ class NodeReplaceVisitor(TraverserVisitor):
     def visit_ref_expr(self, node: RefExpr) -> None:
         if node.node is not None:
             node.node = self.fixup(node.node)
-            if isinstance(node.node, Var):
-                # The Var node may be an orphan and won't otherwise be processed.
+            if not isinstance(node.node, TypeInfo):
                 node.node.accept(self)
 
     def visit_namedtuple_expr(self, node: NamedTupleExpr) -> None:

Methods in RHS of assignments are not guaranteed to be equal by identity (is) to the original funcs they refer to, and in fact they aren't. Probably there are other similar cases where assignment stores something defined earlier in the same class. Why is this only limited to Vars?

[ilevkivskyi]

@sterliakov I am not sure what assignment you are talking about.

The real problem is: even after working on coarse-grained incremental recently, and mentioning how sketchy some parts are, just a quick look at fine grained makes it look much worse. I think I already mentioned it a while ago, but IMO replace_object_state() should fly out of the window (well, we may keep it for Options only). There are just 6 SymbolNode classes, how hard would it be to write a custom .copy_state_from(other) method for each? IMO much less effort than we already spent chasing all the weird daemon bugs.

To give a bit more context to those who need it, the original "copy state but preserve identity of symbols" idea has a big flaw: symbols can (and do) refer to other symbols, e.g. OverloadedFuncDef.items[0].var.info.special_alias (here every item in the chain is a different kind of symbol). These symbol -> symbol references make the two parts of the idea, "copy state" and "preserve identity", work against each other. Thus, everything needs to be very carefully ordered (especially because some of the references are cyclical, due to .info). This is the ordering problem I mentioned above. Implementing per-class copy methods will allow each symbol class to specify which attributes constitute genuine state, and which are cross-references to other symbols.

Btw another fishy thing I noticed while looking at the code is that replacement_map_from_symbol_table() doesn't enlarge prefix when recursing. IMO we should simply re-use MypyFile.local_definitions() there, which I trust much more.

[sterliakov]

I am not sure what assignment you are talking about.

Sorry, my bad, I went debugging a slightly different version to have another explicit node reference (it triggers the same crash):

[case testReferenceToInit]
import m

[file m.py]
class C:
    def __init__(self) -> None: ...

[file m.py.2]
class C:
    def __init__(self) -> None: ...

    foo = __init__

C()
[builtins fixtures/tuple.pyi]
[out]

and the assignment I was talking about is foo = __init__. I see my mistake now - the problem is indeed in the replacements order, initially the info of the replacement func is correct, but gets replaced with a stale one later. However, extending the merger for refexpr should still resolve this issue (although I agree a proper solution would be avoiding replace_object_state altogether)

[ilevkivskyi]

@sterliakov

However, extending the merger for refexpr should still resolve this issue

TBH the current logic is really hard to reason about. If you are sure this will not make things worse in some other cases, this may be a decent hot fix.

[sterliakov]

@ilevkivskyi

this will not make things worse

Daemon server is probably the most scary part of mypy codebase for me, but I do believe that you can't "overpatch", this conversion will be just a no-op in most cases. Doing that may, however, have performance impact in other cases.