-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
gh-20542: Fix crash in split_for_callable when passing values to variadic generics #20543
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
gh-20542: Fix crash in split_for_callable when passing values to variadic generics #20543
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
mypy/checkexpr.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if you add or not t.is_type_obj()? Does mypy error later? The error message you added isn't very clear IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking if we added or not t.is_type_obj() to the guard it would return more generic errors. In the test below, the return is Invalid type: try using Literal[1] instead? but with the additional guard I think it would be something like error: Type expected?, which is correct, but not as specific as it could be.
I probably should use the messagebuilder for the error though. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which one is the current? Neither matches what you have ATM?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! I'm sorry, I misread what line you were commenting on. You're saying why not turn if not vars or not any(isinstance(v, TypeVarTupleType) for v in vars): to if not vars or not any(isinstance(v, TypeVarTupleType) for v in vars) or not t.is_type_object().
I tried this and we produce the Type application is only supported for generic classes and Invalid type: try using Literal[1] instead? in the test against this part of the code. Looking at it, I'm thinking this is likely a lot cleaner and I would just suggest striking my current changes and changing to simply appending or not t.is_type_object() to line 4966. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that seems like a better error output, if I'm understanding correctly.
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅
@hauntsaninja
hauntsaninja
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Fixes gh-20542
Small change, previously passing a literal value (fn[1] in the repro case) to a function using
TypeVarTupleassertcaused a crash in split_for_callable incheckexpr.pydue to an assert statement. This PR just replaces the assertion with a proper validation check and error message.