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 when using isinstance againt variadic tuple #15978

Closed
mehdigmira opened this issue Aug 28, 2023 · 4 comments · Fixed by #15991
Closed

crash when using isinstance againt variadic tuple #15978

mehdigmira opened this issue Aug 28, 2023 · 4 comments · Fixed by #15991
Labels
crash topic-pep-646 PEP 646 (TypeVarTuple, Unpack)

Comments

@mehdigmira
Copy link

Crash Report

mypy crashes when using isinstance againt a class that is a variadic tuple

Traceback

test.py:15: 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.6.0+dev.d7b24514d7301f86031b7d1e2215cf8c2476bec0
Traceback (most recent call last):
  File "/home/vscode/.local/bin/mypy", line 8, in <module>
    sys.exit(console_entry())
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/__main__.py", line 15, in console_entry
    main()
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/main.py", line 99, in main
    res, messages, blockers = run_build(sources, options, fscache, t0, stdout, stderr)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/main.py", line 178, in run_build
    res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/build.py", line 189, in build
    result = _build(
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/build.py", line 262, in _build
    graph = dispatch(sources, manager, stdout)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/build.py", line 2938, in dispatch
    process_graph(graph, manager)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/build.py", line 3336, in process_graph
    process_stale_scc(graph, scc, manager)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/build.py", line 3437, in process_stale_scc
    graph[id].type_check_first_pass()
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/build.py", line 2306, in type_check_first_pass
    self.type_checker().check_first_pass()
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checker.py", line 475, in check_first_pass
    self.accept(d)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checker.py", line 585, in accept
    stmt.accept(self)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/nodes.py", line 790, in accept
    return visitor.visit_func_def(self)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checker.py", line 994, in visit_func_def
    self._visit_func_def(defn)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checker.py", line 998, in _visit_func_def
    self.check_func_item(defn, name=defn.name)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checker.py", line 1071, in check_func_item
    self.check_func_def(defn, typ, name, allow_empty)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checker.py", line 1281, in check_func_def
    self.accept(item.body)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checker.py", line 585, in accept
    stmt.accept(self)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/nodes.py", line 1226, in accept
    return visitor.visit_block(self)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checker.py", line 2754, in visit_block
    self.accept(s)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checker.py", line 585, in accept
    stmt.accept(self)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/nodes.py", line 1246, in accept
    return visitor.visit_expression_stmt(self)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checker.py", line 4276, in visit_expression_stmt
    expr_type = self.expr_checker.accept(s.expr, allow_none_return=True, always_allow_any=True)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checkexpr.py", line 5362, in accept
    typ = self.visit_call_expr(node, allow_none_return=True)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checkexpr.py", line 452, in visit_call_expr
    return self.visit_call_expr_inner(e, allow_none_return=allow_none_return)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checkexpr.py", line 581, in visit_call_expr_inner
    ret_type = self.check_call_expr_with_callee_type(
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checkexpr.py", line 1420, in check_call_expr_with_callee_type
    ret_type, callee_type = self.check_call(
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checkexpr.py", line 1514, in check_call
    return self.check_callable_call(
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checkexpr.py", line 1702, in check_callable_call
    arg_types = self.infer_arg_types_in_context(callee, args, arg_kinds, formal_to_actual)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checkexpr.py", line 1874, in infer_arg_types_in_context
    res[ai] = self.accept(args[ai], arg_type)
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checkexpr.py", line 5370, in accept
    typ = node.accept(self)
          ^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/nodes.py", line 1809, in accept
    return visitor.visit_name_expr(self)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checkexpr.py", line 336, in visit_name_expr
    result = self.analyze_ref_expr(e)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checkexpr.py", line 368, in analyze_ref_expr
    result = type_object_type(node, self.named_type)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/checkmember.py", line 1293, in type_object_type
    return type_object_type_from_function(t, info, method.info, fallback, is_new)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/typeops.py", line 157, in type_object_type_from_function
    signature = cast(FunctionLike, map_type_from_supertype(signature, info, def_info))
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/typeops.py", line 239, in map_type_from_supertype
    inst_type = map_instance_to_supertype(inst_type, super_info)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/maptype.py", line 29, in map_instance_to_supertype
    tuple_type = expand_type(instance.type.tuple_type, env)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/expandtype.py", line 71, in expand_type
    return typ.accept(ExpandTypeVisitor(env))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/types.py", line 2382, in accept
    return visitor.visit_tuple_type(self)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/expandtype.py", line 409, in visit_tuple_type
    items = self.expand_types_with_unpack(t.items)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/expandtype.py", line 397, in expand_types_with_unpack
    unpacked_items = self.expand_unpack(item)
                     ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/mypy/expandtype.py", line 292, in expand_unpack
    raise RuntimeError(f"Invalid type replacement to expand: {repl}")
RuntimeError: Invalid type replacement to expand: Unpack[TP`1]
test.py:15: : note: use --pdb to drop into pdb

To Reproduce

from __future__ import annotations

from typing import Tuple, Unpack  # noqa

from typing_extensions import TypeVarTuple

TP = TypeVarTuple("TP")


class A(Tuple[Unpack[TP]]):  # noqa
    ...


def test(d: A[int, str]):
    isinstance(d, A)

Your Environment

  • Mypy version used: 1.6.0+dev.d7b24514d7301f86031b7d1e2215cf8c2476bec0
  • Mypy command-line flags: --enable-incomplete-feature=TypeVarTuple --enable-incomplete-feature=Unpack
  • Mypy configuration options from mypy.ini (and other config files): None
  • Python version used: 3.11.4
  • Operating system and version: MacOS
@TeamSpen210
Copy link
Contributor

Try adding --new-type-inference, the new system fixes a lot of issues.

@AlexWaygood AlexWaygood added the topic-pep-646 PEP 646 (TypeVarTuple, Unpack) label Aug 28, 2023
@mehdigmira
Copy link
Author

same issue with --new-type-inference

@ilevkivskyi
Copy link
Member

Just wanted to double-check: do you specifically want a subtype of Tuple? If you just want to create a variadic class, you can just write:

class A(Generic[Unpack[Ts]]):
    ...

Also what is the repository where you found this crash (and the previous one)? Is it open-source? If yes, we should probably add it to mypy_primer. cc @hauntsaninja

@mehdigmira
Copy link
Author

Yes, I do want to subtype Tuple. I'm trying to use typevartuple for SQLAlchemy for better type inference. So far since PEP646 is not yet supported by mypy, this is still experimental work.

ilevkivskyi added a commit that referenced this issue Sep 7, 2023
This PR closes the first part of support for `TypeVarTuple`: the
"static" analysis of types (of course everything is static in mypy, but
some parts are more static): `semanal`/`typeanal`, `expand_type()`,
`map_instance_to_supertype()`, `erase_type()` (things that precede
and/or form foundation for type inference and subtyping). This one was
quite tricky, supporting unpacks of forward references required some
thinking.

What is included in this PR:
* Moving argument count validation from `semanal_typeargs` to
`typeanal`. In one of previous PRs I mentioned that `get_proper_type()`
may be called during semantic analysis causing troubles if we have
invalid aliases. So we need to move validation to early stage. For
instances, this is not required, but I strongly prefer keeping instances
and aliases similar. And ideally at some point we can combine the logic,
since it gets more and more similar. At some point we may want to
prohibit using `get_proper_type()` during semantic analysis, but I don't
want to block `TypeVarTuple` support on this, since this may be a
significant refactoring.
* Fixing `map_instance_to_supertype()` and `erase_type()`. These two are
straightforward, we either use `expand_type()` logic directly (by
calling it), or following the same logic.
* Few simplifications in `expandtype` and `typeops` following previous
normalizations of representation, unless there is a flaw in my logic,
removed branches should be all dead code.
* Allow (only fixed) unpacks in argument lists for non-variadic types.
They were prohibited for no good reason.
* (Somewhat limited) support for forward references in unpacks. As I
mentioned this one is tricky because of how forward references are
represented. Usually they follow either a life cycle like: `Any` ->
`<known type>`, or `<Any>` -> `<placeholder>` -> `<known type>` (second
one is relatively rare and usually only appears for potentially
recursive things like base classes or type alias targets). It looks like
`<placeholder>` can never appear as a _valid_ unpack target, I don't
have a proof for this, but I was not able to trigger this, so I am not
handling it (possible downside is that there may be extra errors about
invalid argument count for invalid unpack targets). If I am wrong and
this can happen in some valid cases, we can add handling for unpacks of
placeholders later. Currently, the handling for `Any` stage of forward
references is following: if we detect it, we simply create a dummy valid
alias or instance. This logic should work for the same reason having
plain `Any` worked in the first place (and why all tests pass if we
delete `visit_placeholder_type()`): because (almost) each time we
analyze a type, it is either already complete, or we analyze it _from
scratch_, i.e. we call `expr_to_unanalyzed_type()`, then
`visit_unbound_type()` etc. We almost never store "partially analyzed"
types (there are guards against incomplete references and placeholders
in annotations), and when we do, it is done in a controlled way that
guarantees a type will be re-analyzed again. Since this is such a tricky
subject, I didn't add any complex logic to support more tricky use cases
(like multiple forward references to fixed unpacks in single list). I
propose that we release this, and then see what kind of bug reports we
will get.
* Additional validation for type arguments position to ensure that
`TypeVarTuple`s are never split. Total count is not enough to ban case
where we have type variables `[T, *Ts, S, U]` and arguments `[int, int,
*Us, int]`. We need to explicitly ensure that actual suffix and prefix
are longer or equal to formal ones. Such splitting would be very hard to
support, and is explicitly banned by the PEP.
* Few minor cleanups.

Some random comments:
* It is tricky to preserve valid parts of type arguments, if there is an
argument count error involving an unpack. So after such error I simply
set all arguments to `Any` (or `*tuple[Any, ...]` when needed).
* I know there is some code duplication. I tried to factor it away, but
it turned out non-trivial. I may do some de-duplication pass after
everything is done, and it is easier to see the big picture.
* Type applications (i.e. when we have `A[int, int]` in runtime context)
are wild west currently. I decided to postpone variadic support for them
to a separate PR, because there is already some support (we will just
need to handle edge cases and more error conditions) and I wanted
minimize size of this PR.
* Something I wanted to mention in one of previous PRs but forgot: Long
time ago I proposed to normalize away type aliases inside `Unpack`, but
I abandoned this idea, it doesn't really give us any benefits.

As I said, this is the last PR for the "static part", in the next PR I
will work on fixing subtyping and inference for variadic instances. And
then will continue with remaining items I mentioned in my master plan in
#15924

Fixes #15978

---------

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
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.

4 participants