-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix crash with tuple unpack inside TypeVar default #20456
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
This comment has been minimized.
This comment has been minimized.
mypy/typeops.py
Outdated
| and unpacked_type.type.fullname == "builtins.tuple" | ||
| ): | ||
| items.append(unpacked_type.args[0]) | ||
| elif isinstance(unpacked_type, TupleType): |
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.
IIRC the idea was that it should never be true when we call tuple_fallback - we have several places where things like tuple[*tuple[X,Y]] are normalized to a plain tuple, and the original intent was to get rid of such constructs as early as possible. This is fine as a quickfix, but I'm afraid that this will manifest again when such nested tuple is used elsewhere.
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 entirely sure about that but isn't the function already doing the flatting? E.g. just the line above flattens builtins.tuple Instances.
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.
builtins.tuple is already normalized: you can't rewrite e.g. tuple[str, *tuple[int, ...]] as a single tuple[...] construct. This is only about expanding unpacks of fixed-size TupleType types (which is always possible), see flatten_nested_tuples
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.
Think I found the right place now. The normalization happens in the TypeArgumentAnalyzer.
Lines 110 to 115 in b69309b
| def visit_tuple_type(self, t: TupleType) -> None: | |
| t.items = flatten_nested_tuples(t.items) | |
| # We could also normalize Tuple[*tuple[X, ...]] -> tuple[X, ...] like in | |
| # expand_type() but we can't do this here since it is not a translator visitor, | |
| # and we need to return an Instance instead of TupleType. | |
| super().visit_tuple_type(t) |
The visitor wasn't traversing TypeVar defaults.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Fixes #20451