-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[red-knot] Super-basic generic inference at call sites #17301
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
Merged
Changes from 106 commits
Commits
Show all changes
117 commits
Select commit
Hold shift + click to select a range
3be8257
Build context for generic classes
dcreager 4c76cb5
Handle explicit specialization before outputting lints
dcreager 8048604
Explicitly specialize classes
dcreager fe76d56
Track those structs
dcreager 4346e7d
Add Specialization
dcreager 2380ade
Add CallableType::Specialized
dcreager 178ee89
Use union to hold typevar constraints
dcreager feb1ea9
Add Type::TypeVar variant
dcreager 3c1ad79
Fix failing tests
dcreager 59430b6
doc Type::TypeVar
dcreager 77ffa80
More correct handling of final bounds/constraints
dcreager 6f86720
use list[T] so generic funcs are callable even with Never
dcreager cf81967
lint
dcreager 6615df1
Add (currently failing) narrowing tests
dcreager b2f5a2a
Typevars _can_ be fully static I guess
dcreager 590680c
Simplify intersections with constrained typevars
dcreager e6b7d40
Merge branch 'main' into dcreager/typevar-type
dcreager debd60a
Fix tests
dcreager aa391fd
lint
dcreager e57e62e
Update crates/red_knot_python_semantic/src/types/type_ordering.rs
dcreager 3df79cc
Clarify that typevar is subtype of object too
dcreager 5b08e93
Clarify non-fully-static bounded typevars aren't subtypes
dcreager 82e810f
Add more tests for constrained gradual typevars
dcreager a3d7253
Update crates/red_knot_python_semantic/src/types.rs
dcreager 15682d5
Simplify intersections with constrained typevars w/o glossing into union
dcreager 9e07efe
Simplify positive intersections too
dcreager fb63c22
Intersection of constraints is subtype of typevar
dcreager 3459056
Better descriptions of intersections of constrained typevars
dcreager 71d425e
Add multiple narrowing example
dcreager 233e938
lint
dcreager 9785202
Sort typevar constraints
dcreager c86af50
Remove moot todo
dcreager aa00895
Fold typevar match arms back into main match statement
dcreager d99d1d7
Remove moot comment
dcreager 58f0995
Remove moot todo
dcreager 42fd54a
Add more TODOs about OneOf connector
dcreager 6bd69f1
add todos for unary/binary ops
dcreager 1d6a917
Merge remote-tracking branch 'origin/main' into dcreager/typevar-type
dcreager 37692f1
Merge remote-tracking branch 'origin/dcreager/typevar-type' into dcre…
dcreager 3869dc6
Fix tests
dcreager 0c1745b
Fix tests better
dcreager 8bdd9e2
Support unary and binary ops
dcreager 39244dd
Merge remote-tracking branch 'origin/main' into dcreager/typevar-type
dcreager 5694b3d
Merge remote-tracking branch 'origin/main' into dcreager/special-class
dcreager 123b920
Fix merge conflicts
dcreager 9e1767f
Rename to SpecializedCallable{,Type}
dcreager 23ac0b6
Merge branch 'dcreager/typevar-type' into dcreager/special-class
dcreager 6b27947
Specialize types
dcreager 669aa21
Merge remote-tracking branch 'origin/main' into dcreager/special-class
dcreager 575998e
Fix tests in ide crate
dcreager f02fefa
Add GenericClass, NonGenericClass, and GenericAlias
dcreager cc3a3df
Add GenericAlias
dcreager 18af8b6
Apply specializations
dcreager af52fd1
Use class literal for self member lookups
dcreager d3fd822
Display specializations and generic aliases
dcreager aa64990
Specialize generic base class in generic subclass
dcreager e41f889
Merge branch 'main' into dcreager/special-class
dcreager 7eb7c28
Only specialize function literals, not all callables
dcreager adb4aba
Fix descriptor protocol tests
dcreager 968e637
Merge branch 'main' into dcreager/special-class
dcreager fd7914a
Remove unused methods
dcreager 7ebda98
clippy
dcreager 6257e89
Apply specialization to signature in-place
dcreager 78cd92a
typo
dcreager 530e2bc
Fix test case
dcreager 637fd48
lint
dcreager ec5a588
Defer class definitions with string literals in base classes
dcreager 27cb208
fix lint fix
dcreager ea12548
skip failing test for now
dcreager c376dad
clippy and lint
dcreager 311dc59
add xfail for specializing to union of constraints
dcreager 5fc8425
specialize property types
dcreager 7aaeb47
Don't specialize function twice
dcreager 43554e2
fix docs
dcreager aec3392
Add (optional) generic context to overload signature
dcreager 595ecae
Super-basic inference at call sites
dcreager 58830fa
Add comment
dcreager 0d656db
Merge remote-tracking branch 'origin/main' into dcreager/special-class
dcreager 7c3405a
Better TODO fallback type
dcreager 6593d90
Generic aliases are literals in type display
dcreager 7ca6a60
Explain self_instance not being specialized
dcreager dea493e
Comment other non-specializations
dcreager 048bb8b
Add xfail for generic method inside generic class
dcreager 06859fa
More Python-like displays for specializations
dcreager d138405
Narrow type(generic) better
dcreager 0997eca
Better todos
dcreager 6ab1b90
Add TODO about property test data
dcreager bcb147f
lint
dcreager 78f73fc
Merge branch 'dcreager/special-class' into dcreager/infer-function-calls
dcreager 145a776
Merge branch 'main' into dcreager/infer-function-calls
dcreager 3a7b638
Clean up union examples
dcreager cccc77d
Use with_self instead of prepend_synthetic
dcreager 993c6e3
Use try_call_dunder_with_policy
dcreager 2169282
Try to infer class specialization from constructor arguments
dcreager 5a4bd28
TODO: constructor is getting specialized with unknown
dcreager 756ce9a
Add uninitialized instance variant
dcreager 1f67883
Create uninitialized instance when constructing
dcreager 3cf9559
Merge branch 'main' into dcreager/infer-function-calls
dcreager beb64b8
clippy
dcreager c1e3470
Only extract specializations for generic classes
dcreager 4bd8128
Use identity specialization for new/init
dcreager 3734599
Revert new instance variant
dcreager 836d02c
Remove unneeded helper method
dcreager 0744540
Add comment explaining the identity specialization
dcreager 701cfd1
Merge branch 'main' into dcreager/infer-function-calls
dcreager d3067b0
Revert unneeded policy change
dcreager 4823f43
Remove TODOs
dcreager 87f3f62
Add TODO about nested contexts
dcreager beed962
Add comment about potential class method inference
dcreager 94bf3f6
Real function body
dcreager 8f43e30
Remove unneeded special case lookup logic
dcreager f38b43e
Add tests for new and init
dcreager 54fb8a2
Merge branch 'main' into dcreager/infer-function-calls
dcreager b852485
Merge remote-tracking branch 'origin/dcreager/infer-function-calls' i…
dcreager 394c762
Add failing test case for generic __init__
dcreager 347547b
Merge branch 'main' into dcreager/infer-function-calls
dcreager 523ddcb
clean up known class detection
dcreager 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
Oops, something went wrong.
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.
I am not actually convinced there's a strong rationale for this TODO. I don't think there is any function body for
two_paramsthat would type-check and allow it to return anything other than"a"or"b". Same for a number of similar TODOs below.But I could be wrong; not asking to have these TODOs removed in this PR, just putting it out there that I'm not convinced :)
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.
if revealed type of return value of function most be
Literal["a"] | Literal["b"], then it's incorrect type checking, since there is only one type var:T.in other words, it should get
Literal["a"]only when calling bytwo_params("a", "a"). but when callingtwo_params("a", "b"), both of "a" and "b" are different shape of literal strings and it should show error. but if revealed type becamestr, the call oftwo_params("a", "b")is acceptable.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 don't agree. There's nothing special about a union type in a set-theoretic type system.
Literal["a", "b"]is a well-defined single type, just as much asstris, and the single typevarTcan bind to the union type just as well as it can bind to any other 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.
IIUC,
Twould beLiteral["a", "b"]when call it byreveal_type(two_params("a", "b")).It now makes sense to me, thanks for clarification.
What I was thinking is that when first parameter passed (
"a"), thenTtypevar locks toLiteral["a"]and by second parameter which isLiteral["b"]it will invalidate theT. Actually, I had rust backgrounding of generics in my mind:BTW, it's valid in Python generics.
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 is a good way to think about how the implementation works here, and how it will need to grow to support some of the other mdtests that still have TODOs.
Right now, the only "solving" that we do is to see that the param is a typevar, and the argument is "some type", and add to the pending specialization that the typevar maps to the type. But if we've already seen that typevar in a different parameter, instead of replacing the previous type, or requiring the previous and new types to be the same (as you thought might be the case), we merge them together into a union. This is the only "unification" that the implementation in this PR does.
This first example in this section is one that this PR doesn't addres, where we'll need to not just blindly union everything together — the type annotation is meant to be an actual restriction that we should enforce.
Uh oh!
There was an error while loading. Please reload this page.
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.
Yes, I think this is a very useful example, thank you! But (to rephrase what @dcreager already mentioned), no Python type checker actually supports the general principle that if
two_params(T1, T2)is OK, thereforef = curry_two_params(T1); f(T2)should also be OK. In both mypy and pyright,two_params("a", 1)is fine (Tis solved toint | str), butf = curry_two_params("a"); f(1)is a type error (fis(str) -> str).In other words, there's no difference in principle here, just a difference in semi-arbitrary widening heuristics, that makes the same issue appear at a different level of type granularity.
Perhaps the mypy/pyright approach is the best one available, in practice! But it would certainly be satisfying if we can find a more general answer that doesn't depend on heuristics.
One approach to solve this kind of issue is unification of a constraint system across the entire scope. But this is quite hard to reconcile with flow typing.
Uh oh!
There was an error while loading. Please reload this page.
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 think this is another form of the same problem:
The common thread in the examples is that a type is returned from a generic function in which a type parameter appears in an invariant/contravariant position.
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.
Is this another place where we'd lean on
OneOf? The type oflist_of_what(and maybe the type of a literal in general?) would belist[OneOf[Literal[1], int]]. i.e.OneOfmight be shaping up to be the way that we handle "cross-expression" constraints in general.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 do think it's plausible that we can leverage gradual typing here, though I'm not sure we need
OneOfin this case, I think "union withUnknown" suffices. (UsingOneOfstill requires us to make the arbitrary judgment that you aren't supposed to add strings to this list.) The unionUnknown | Literal[1]expresses the gradual type "some type at least as large asLiteral[1]but possibly larger", which has the nice property that if you ever try to use it somewhere you can't use an integer, it'll error -- but it'll allow things other thanLiteral[1]to be assigned to it.I think at least two things we would need to make this approach work would be narrowing such that if you have a list
lof typelist[Unknown | Literal[1]]and you dol.append("foo"), we subsequently consider it to be of typelist[Unknown | Literal[1] | Literal["foo"].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.
Removed the TODOs. If we decide to make this change, test failures will tell us all the places that need to be updated.