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

Handle TypeVarTupleType when checking overload constraints #16428

Merged
merged 3 commits into from Nov 10, 2023

Conversation

robjhornby
Copy link
Contributor

@robjhornby robjhornby commented Nov 9, 2023

Fixes #16427

The test case added in the first commit crashes.

The second commit addresses the crash - I don't know whether this fix is
correct, it just happens to stop the crash but it leads to a code branch
which just continues out of a for loop iteration, so it might be
bypassing something it shouldn't. I don't completely understand it.

@robjhornby robjhornby changed the title Check for TypeVarTupleType in Handle TypeVarTupleType when checking overload constraints Nov 9, 2023

This comment has been minimized.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for the PR.

I have some minor suggestions that I am going to apply now.

mypy/constraints.py Outdated Show resolved Hide resolved
test-data/unit/check-typevar-tuple.test Outdated Show resolved Hide resolved
test-data/unit/check-typevar-tuple.test Outdated Show resolved Hide resolved
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

spack (https://github.com/spack/spack): typechecking got 1.26x slower (56.1s -> 70.7s)
(Performance measurements are based on a single noisy sample)

@ilevkivskyi ilevkivskyi merged commit b67dc53 into python:master Nov 10, 2023
17 of 18 checks passed
@ilevkivskyi ilevkivskyi mentioned this pull request Nov 10, 2023
2 tasks
@robjhornby
Copy link
Contributor Author

Thanks for the quick turnaround, looking forward to the new typevar tuple features being released!

JukkaL pushed a commit that referenced this pull request Nov 10, 2023
Fixes #16427

The test case added in the first commit crashes.

The second commit addresses the crash - I don't know whether this fix is
correct, it just happens to stop the crash but it leads to a code branch
which just `continue`s out of a for loop iteration, so it might be
bypassing something it shouldn't. I don't completely understand it.

---------

Co-authored-by: Ivan Levkivskyi <levkivskyi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overloaded def including TypeVarTuple and variable length Tuple crash
2 participants