Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash with experimental TypeVarTuple support #15241

Closed
erictraut opened this issue May 14, 2023 · 0 comments · Fixed by #15924
Closed

Crash with experimental TypeVarTuple support #15241

erictraut opened this issue May 14, 2023 · 0 comments · Fixed by #15924
Labels
crash topic-pep-646 PEP 646 (TypeVarTuple, Unpack)

Comments

@erictraut
Copy link

I'm investigating a bug reported against pyright, and I was curious how mypy would handle the code. I tried it with mypy's experimental TypeVarTuple and Unpack support (--enable-incomplete-feature=TypeVarTuple --enable-incomplete-feature=Unpack) using mypy 1.3.0. The result was an "INTERNAL ERROR". That's not surprising given that the TypeVarTuple support is still under development. I'm reporting the crash in case those who are working on TypeVarTuple support in mypy find it useful.

This piece of code incorporates multiple variadic type variables, recursive function calls, tuple unpacking, nested calls that require bidirectional type inference, and higher-order functions. That makes it a potentially interesting test case.

Here's the code with the * replaced with Unpack (since mypy doesn't appear to support the new * syntax currently).

from typing import TypeVar, TypeVarTuple, Unpack, Callable

X = TypeVar("X")
Y = TypeVar("Y")
Xs = TypeVarTuple("Xs")
Ys = TypeVarTuple("Ys")

def nil() -> tuple[()]:
    return ()

def cons(
    f: Callable[[X], Y],
    g: Callable[[Unpack[Xs]], tuple[Unpack[Ys]]],
) -> Callable[[X, Unpack[Xs]], tuple[Y, Unpack[Ys]]]:
    def wrapped(x: X, *xs: Unpack[Xs]) -> tuple[Y, Unpack[Ys]]:
        y, ys = f(x), g(*xs)
        return y, *ys

    return wrapped

def star(f: Callable[[X], Y]) -> Callable[[Unpack[tuple[X, ...]]], tuple[Y, ...]]:
    def wrapped(*xs: X):
        if not xs:
            return nil()
        return cons(f, star(f))(*xs)

    return wrapped

Here's the stack trace for the crash:

Traceback (most recent call last):
  File "mypy/checkexpr.py", line 4890, in accept
  File "mypy/nodes.py", line 1889, in accept
  File "mypy/checkexpr.py", line 429, in visit_call_expr
  File "mypy/checkexpr.py", line 549, in visit_call_expr_inner
  File "mypy/checkexpr.py", line 1209, in check_call_expr_with_callee_type
  File "mypy/checkexpr.py", line 1292, in check_call
  File "mypy/checkexpr.py", line 1483, in check_callable_call
  File "mypy/checkexpr.py", line 2101, in check_argument_types
AssertionError: 
@AlexWaygood AlexWaygood added the topic-pep-646 PEP 646 (TypeVarTuple, Unpack) label May 14, 2023
ilevkivskyi added a commit that referenced this issue Aug 23, 2023
Fixes #13981
Fixes #15241
Fixes #15495
Fixes #15633
Fixes #15667
Fixes #15897
Fixes #15929

OK, I started following the plan outlined in
#15879. In this PR I focused mostly
on "kinematics". Here are some notes (in random order):
* I decided to normalize `TupleType` and `Instance` items in
`semanal_typeargs.py` (not in the type constructors, like for unions).
It looks like a simpler way to normalize for now. After this, we can
rely on the fact that only non-trivial (more below on what is trivial)
variadic items in a type list is either `*Ts` or `*tuple[X, ...]`. A
single top-level `TupleType` can appear in an unpack only as type of
`*args`.
* Callables turned out to be tricky. There is certain tight coupling
between `FuncDef.type` and `FuncDef.arguments` that makes it hard to
normalize prefix to be expressed as individual arguments _at
definition_. I faced exactly the same problem when I implemented `**`
unpacking for TypedDicts. So we have two choices: either handle prefixes
everywhere, or use normalization helper in relevant code. I propose to
go with the latter (it worked well for `**` unpacking).
* I decided to switch `Unpack` to be disallowed by default in
`typeanal.py`, only very specific potions are allowed now. Although this
required plumbing `allow_unpack` all the way from `semanal.py`,
conceptually it is simple. This is similar to how `ParamSpec` is
handled.
* This PR fixes all currently open crash issues (some intentionally,
some accidentally) plus a bunch of TODOs I found in the tests (but not
all).
* I decided to simplify `TypeAliasExpr` (and made it simple reference to
the `SymbolNode`, like e.g. `TypedDictExpr` and `NamedTupleExpr`). This
is not strictly necessary for this PR, but it makes some parts of it a
bit simpler, and I wanted to do it for long time.

Here is a more detailed plan of what I am leaving for future PRs (in
rough order of priority):
* Close non-crash open issues (it looks like there are only three, and
all seem to be straightforward)
* Handle trivial items in `UnpackType` gracefully. These are `<nothing>`
and `Any` (and also potentially `object`). They can appear e.g. after a
user error. Currently they can cause assert crashes. (Not sure what is
the best way to do this).
* Go over current places where `Unpack` is handled, and verify both
possible variadic items are handled.
* Audit variadic `Instance` constrains and subtyping (the latter is
probably OK, but the former may be broken).
* Audit `Callable` and `Tuple` subtyping for variadic-related edge cases
(constraints seem OK for these).
* Figure out story about `map_instance_to_supertype()` (if no changes
are needed, add tests for subclassing).
* Clear most remaining TODOs.
* Go once more over the large scale picture and check whether we have
some important parts missing (or unhandled interactions between those).
* Verify various "advanced" typing features work well with
`TypeVarTuple`s (and add some support if missing but looks important).
* Enable this feature by default.

I hope to finish these in next few weeks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crash topic-pep-646 PEP 646 (TypeVarTuple, Unpack)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants