-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[ty] Distinguish "unconstrained" from "constrained to any type" #21539
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
Changes from all commits
7e98d29
465d53f
a4226fd
8a0b4c2
de391ee
07636a4
d3b2fd4
c1aeca1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,8 +22,10 @@ from ty_extensions import ConstraintSet, generic_context | |
| # fmt: off | ||
|
|
||
| def unbounded[T](): | ||
| # revealed: ty_extensions.Specialization[T@unbounded = object] | ||
| # revealed: ty_extensions.Specialization[T@unbounded = Unknown] | ||
| reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.always())) | ||
| # revealed: ty_extensions.Specialization[T@unbounded = object] | ||
| reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(Never, T, object))) | ||
| # revealed: None | ||
| reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.never())) | ||
|
|
||
|
|
@@ -88,6 +90,7 @@ that makes the test succeed. | |
| from typing import Any | ||
|
|
||
| def bounded_by_gradual[T: Any](): | ||
| # TODO: revealed: ty_extensions.Specialization[T@bounded_by_gradual = Any] | ||
| # revealed: ty_extensions.Specialization[T@bounded_by_gradual = object] | ||
| reveal_type(generic_context(bounded_by_gradual).specialize_constrained(ConstraintSet.always())) | ||
| # revealed: None | ||
|
|
@@ -168,27 +171,31 @@ from typing import Any | |
| # fmt: off | ||
|
|
||
| def constrained_by_gradual[T: (Base, Any)](): | ||
| # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = Unknown] | ||
| # revealed: ty_extensions.Specialization[T@constrained_by_gradual = Base] | ||
| reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.always())) | ||
|
Comment on lines
+175
to
+176
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would expect this to solve to the typevar default, in this case
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is a side effect of the change I added from this discussion #21414 (comment). If there are multiple solutions, my original plan was to produce the union of them. But I don't think that's correct, so instead we either:
Here, the constraint of |
||
| # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = Any] | ||
| # revealed: ty_extensions.Specialization[T@constrained_by_gradual = object] | ||
| reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.always())) | ||
| reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.range(Never, T, object))) | ||
|
Comment on lines
177
to
+179
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here I think either
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, this one already has
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (This one I think the right TODO is |
||
| # revealed: None | ||
| reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.never())) | ||
|
|
||
| # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = Any] | ||
| # revealed: ty_extensions.Specialization[T@constrained_by_gradual = Base] | ||
| reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.range(Never, T, Base))) | ||
| # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = Any] | ||
| # revealed: ty_extensions.Specialization[T@constrained_by_gradual = Unrelated] | ||
| reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.range(Never, T, Unrelated))) | ||
|
|
||
| # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = Any] | ||
| # revealed: ty_extensions.Specialization[T@constrained_by_gradual = Super] | ||
| # revealed: ty_extensions.Specialization[T@constrained_by_gradual = Base] | ||
| reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.range(Never, T, Super))) | ||
|
Comment on lines
+191
to
192
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above cases, I think this should also solve to
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added TODO |
||
| # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = Any] | ||
| # revealed: ty_extensions.Specialization[T@constrained_by_gradual = Super] | ||
| reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.range(Super, T, Super))) | ||
|
|
||
| # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = Any] | ||
| # revealed: ty_extensions.Specialization[T@constrained_by_gradual = object] | ||
| # revealed: ty_extensions.Specialization[T@constrained_by_gradual = Base] | ||
| reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.range(Sub, T, object))) | ||
|
Comment on lines
+198
to
199
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And again here.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added TODO |
||
| # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = Any] | ||
| # revealed: ty_extensions.Specialization[T@constrained_by_gradual = Sub] | ||
|
|
@@ -288,15 +295,15 @@ class Unrelated: ... | |
| # fmt: off | ||
|
|
||
| def mutually_bound[T: Base, U](): | ||
| # revealed: ty_extensions.Specialization[T@mutually_bound = Base, U@mutually_bound = object] | ||
| # revealed: ty_extensions.Specialization[T@mutually_bound = Base, U@mutually_bound = Unknown] | ||
| reveal_type(generic_context(mutually_bound).specialize_constrained(ConstraintSet.always())) | ||
| # revealed: None | ||
| reveal_type(generic_context(mutually_bound).specialize_constrained(ConstraintSet.never())) | ||
|
|
||
| # revealed: ty_extensions.Specialization[T@mutually_bound = Base, U@mutually_bound = Base] | ||
| reveal_type(generic_context(mutually_bound).specialize_constrained(ConstraintSet.range(Never, U, T))) | ||
|
|
||
| # revealed: ty_extensions.Specialization[T@mutually_bound = Sub, U@mutually_bound = object] | ||
| # revealed: ty_extensions.Specialization[T@mutually_bound = Sub, U@mutually_bound = Unknown] | ||
| reveal_type(generic_context(mutually_bound).specialize_constrained(ConstraintSet.range(Never, T, Sub))) | ||
| # revealed: ty_extensions.Specialization[T@mutually_bound = Sub, U@mutually_bound = Sub] | ||
| reveal_type(generic_context(mutually_bound).specialize_constrained(ConstraintSet.range(Never, T, Sub) & ConstraintSet.range(Never, U, T))) | ||
|
|
||
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 weird case, and the more I think about it, the weirder it gets.
I think the principles of constrained type var should probably be:
UnknownorAny). This is the core invariant of constrained type variables.def f[T: (Base, Sub)](x: T)we should prefer solvingf(typed_as sub)toSubrather thanBase, even though both solutions would be possible.But if we agree with these principles, then we can never solve this constrained type var to
Base, because any set of constraints that would allow us to solve toBasewould also allow solving toAny, therefore we can't narrow it down to one of the choices.mypy seems to just always solve such a typevar to
Any. (Mypy doesn't support typevar defaults, nor distinguishAnyvsUnknown, so I can't really experiment with how it handles fallback-to-default.)pyright also always solves to
Any, unless the typevar is totally unconstrained, in which case it falls back to the typevar default (which I've explicitly specified to beBasein this example, but could also just implicitly beUnknown.) I'm not sure how pyright decides thatf(base)orf(sub)should beAnyrather than "the typevar default", given that those calls are compatible with either constrained type.pyrefly does the same thing you originally did here (always solve to a fully static type), which I'm pretty sure is wrong. It fails both the promises of a constrained type var and of the gradual guarantee, and I suspect will cause false positive errors in practice (the fact that pyright and mypy both prefer solving to
Anystrengthens this intuition.) If theAnyconstrained type means "there is really some static type here, but we don't know what it is" (imagine its anUnknownfrom a failed import of a type), then it's wrong to solve to some arbitrary fully static type, because we don't know that's what the constrained type was "supposed" to be - and it could cause a lot of cascading errors.(I think it's fine to punt all of this, but I will suggest some different TODOs below based on this analysis.)
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 it worth trying to consider which materializations of
Anyare valid specializations? IfBaseis a valid specialization but no subtypes are (if that were to violate some other requirement of the specialization), then theAnycould not materialize to anything that would take precedence according to your rule (2).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, that makes sense! So if we have a lower bound of
Base(e.g. because we got a call that provided an instance ofBaseas argument for this typevar), then we can solve toBasebecause it must be the most precise option.