-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Lenient handling of *tuple[Any, ...] (part 3) #22014
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
Merged
+138
−20
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This case isn't covered by tests and the behavior looks wrong:
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.
This was already broken before (but in a different way). Most likely this should be an easy fix with the new logic.
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.
Oh wow, this uncovered (a quite embarrassing) bug in
split_with_prefix_and_suffix(), which is like ultra-foundation in the wholeTypeVarTuplestory. I am surprised it didn't cause other problems so far.To give some more context, initially I prohibited partial overlap for variadic unpacks:
at instance creation level. But relatively late in the process I decided to lift this restriction (and keep it only for the cases where partial overlap is genuinely ambiguous, e.g. where actual type arguments have
*Us). The reason is that, unlike in "type dynamics" (i.e.is_subtype()), in "type kinematics" (i.e.expand_type()) there is no ambiguity w.r.t. what*tuple[X, ...]actually means (i.e. there is no strict vs lenient story).However, now we need to be more careful in situations where
split_with_prefix_and_suffix()is called with synthetic/ad-hoc type lists, i.e. those not directly coming from instance arguments. A more prudent way would be to either add an assert or make return type ofsplit_with_prefix_and_suffix()optional. I however don't like either. I will probably just spot-check the most important call sites (and update if needed).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.
OK, I think I handled everything except meet/join (I will handle those in a separate PR to limit the scope):
constraints.pywith a test.checkpattern.pyshould be a pure refactoring (as it was hard to reason about).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.
Actually it looks like meet/join do not need anything (i.e. I don't see any bugs, and no places where
Anyrequires special handling now). There are still couple TODOs (because we inferobject/Neverwhere we can infer something more precise), but those are quite tedious, so I would do this if/when someone asks about it.