NamedTuple joined with Tuple crashes mypy

[pkch]

(based on #3111)

join of Tuple and namedtuple:

class X(NamedTuple):
    x: int
    y: str

reveal_type([(3, 'b'), X(1, 'a')])  # E: Revealed type is 'builtins.list[Tuple[builtins.int, builtins.str]]'
reveal_type([X(1, 'a'), (3, 'b')])  # E: Revealed type is 'builtins.list[Tuple[builtins.int, builtins.str]]'

results in

File "c:\users\maxmo\downloads\mypy\mypy\join.py", line 293, in join_instances
    args.append(join_types(t.args[i], s.args[i]))
IndexError: list index out of range

[gvanrossum]

How is that funny unicode character you used in the title pronounced?

[pkch]

Hmm, in Unicode it's called "square cup", but I suppose in math it's just pronounced as "join"? I only saw it for the first time a couple weeks ago, and liked it a lot :D

[pkch]

This is not a big deal (it's not the root cause of the crash), but still: I think it might help new contributors to mypy, and reduce the complexity of the code base, if we cleaned up fallback usage a bit. In particular:

• clarify its meaning and keep it consistent
• split into several different names if it turns out that its usage isn't uniform
• better document it in the source code

What I know about fallback comes from some comments on gitter and in source code:

source code:

class TupleType(Type):
    """The tuple type Tuple[T1, ..., Tn] (at least one type argument).

    Instance variables:
        items: tuple item types
        fallback: the underlying instance type that is used for non-tuple methods
            (this is currently always builtins.tuple, but it could be different for named
            tuples, for example)
        implicit: if True, derived from a tuple expression (t,....) instead of Tuple[t, ...]
    """

@ilevkivskyi

fallback is used for callable types with .it_type_obj() returning True, for example for callable types returned by type_object_type() in checkmemser.py. The logic is following: class object C is an instance of Callable[<__init__ or __new__ args>, C], but also is an instance of type or corresponding metaclass. The latter is stored in fallback attribute of CallableType.

@elazarg

Another use is in TupleType - t.fallback is also used for namedtule instance. The idea behind fallback, as I understand it, is similar to an intersection type.

@JukkaL

yes, fallback is used for additional methods and attributes that a type supports. for example, we special case a few operations for TupleType in the type checker, but for a lot of methods we just "fall back" to methods defined the stub for the tuple class.

As I'm fixing this issue, I found that two fallbacks are joined, and they have a different number of type arguments. What do type arguments even mean for fallback? I thought TupleType.fallback is only used to look up method/attributes, so why is it of type Instance rather than of type TypeInfo? If they it needs to allow type args, then we should update the code that joins fallbacks (fallback = join_instances(self.s.fallback, t.fallback) in TypeJoinVisitor.visit_tuple_types) to do something meaningful when fallback X (which is a NamedTuple) and fallback Tuple[object] are being joined. Currently, the result of the join is builtins.object, which is then passed as a fallback to the new TupleType, leading to a weird type Tuple[builtins.int, builtins.str, fallback=builtins.object] instead of just Tuple[builtins.int, builtins.str].

Anyway, for now I just custom-cased this (it only affects revealed type print out).

[gvanrossum]

I'm happy to see more comments added, and if there really are different unrelated things with different behavior all named fallback I agree we should rename at least some of them. But maybe the comments should come first.

[JukkaL]

I think that all fallbacks are similar, and the issue is with comments that describe them in multiple different ways. It would make sense to have a single detailed description of fallbacks and have comments of fallbacks include a reference to this description.

[refi64]

(FWIW, the symbol is usually just called union.)

[pkch]

The original crash I reported no longer occurs on current master. The PR to fix this issue is repurposed to make somewhat more realistic stubs and put in safety asserts as per @ilevkivskyi suggestions.

[gvanrossum]

So can we close this issue?

[pkch]

I think so! @ilevkivskyi should I create a new issue to explain what the PR is doing?

[ilevkivskyi]

@pkch I don't think we need another issue, the discussion in the PR is already "self-documenting".