-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix crash involving Unpack-ed TypeVarTuple #20323
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
Conversation
| @overload # E: Single overload definition, multiple required | ||
| def foo(x: int) -> None: ... | ||
|
|
||
| def foo(*args: int, **kw: str) -> None: # E: Overloaded function implementation does not accept all possible arguments of signature 1 |
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.
Not really related to this PR, just adds #19619 (comment)
mypy/types.py
Outdated
| typ = vararg.typ | ||
| if isinstance(typ, UnpackType): | ||
| # Similar to logic in ExpressionChecker.visit_tuple_index_helper | ||
| unpacked = get_proper_type(typ.type) |
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.
fixing this here could prevent crashes in other places that don't expect to be handling UnpackType, but may result in false positives because upper_bound is lossy
This comment has been minimized.
This comment has been minimized.
|
Hmm, maybe this approach is too ambitious for now (e.g. thinking about |
6012809 to
29d17c1
Compare
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
sterliakov
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.
Certainly looks OK as a hotfix for a crashing result. The false positive just remains intact and should be tracked separately. Thanks!
ilevkivskyi
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.
LG
Fixes #20093
This fixes the crash, but not the false positive (the false positive existed prior to the regression that introduced the crash)