-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[red-knot] Flatten Type::Callable into four Type variants
#17126
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
|
4cfc2e3 to
ae40234
Compare
|
Some other notes:
|
carljm
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.
Nice, yeah I think this structure is less error-prone: avoids grouping very different types together in a way that makes it too easy to wrongly handle them all identically.
db872b1 to
ff29d69
Compare
ff29d69 to
3853d0f
Compare
|
Thanks Alex for doing this! I still want to take a stab at merging the types to reduce the number of variants but that's something for later. |
* main: [red-knot] Add property tests for callable types (#17006) [red-knot] Disjointness for callable types (#17094) [red-knot] Flatten `Type::Callable` into four `Type` variants (#17126) mdtest.py: do a full mdtest run immediately when the script is executed (#17128) [red-knot] Fix callable subtyping for standard parameters (#17125) [red-knot] Fix more `redundant-cast` false positives (#17119) Sync vendored typeshed stubs (#17106) [red-knot] support Any as a class in typeshed (#17107) Visit `Identifier` node as part of the `SourceOrderVisitor` (#17110) [red-knot] Don't infer Todo for quite so many tuple type expressions (#17116) CI: Run pre-commit on depot machine (#17120) Error instead of `panic!` when running Ruff from a deleted directory (#16903) (#17054) Control flow graph: setup (#17064) [red-knot] Playground improvements (#17109) [red-knot] IDE crate (#17045) Update dependency vite to v6.2.4 (#17104) [red-knot] Add redundant-cast error (#17100) [red-knot] Narrowing on `in tuple[...]` and `in str` (#17059)
It would be great if we could postpone this until after #17017 has been merged; that PR makes major changes to two of these callable types, and rebasing on top of this branch here was a significant amount of work already. But in general, that sounds like a good idea 👍 |
Summary
Currently our
Type::Callablewraps a four-variantCallableTypeenum. But as time has gone on, I think we've found that the four variants inCallableTypeare really more different to each other than they are similar to each other:GeneralCallableTypeis a structural type describing all callable types with a certain signature, but the other three types are "literal types", more similar to theFunctionLiteralvariantGeneralCallableTypeis not a singleton or a single-valued type, but the other three are all single-valued types (WrapperDescriptorDunderGetis even a singleton type)GeneralCallableTypehas (or should have) ambiguous truthiness, but all possible inhabitants of the other three types are always truthy.GeneralCallableTypecan contain inner unions and intersections that must be sorted in some contexts in our internal model, but this is not true for the other three variants.This PR flattens
Type::Callableinto four distinctType::variants. In the process, it fixes a number of latent bugs that were concealed by the current architecture but are laid bare by the refactor. Unit tests for these bugs are included in the PR.