[ty] Subtyping for bidirectional inference#21930
Merged
ibraheemdev merged 2 commits intomainfrom Dec 30, 2025
Merged
Conversation
ibraheemdev
force-pushed
the
ibraheem/bidi-subtyping2
branch
from
0fdea5a to
78f2194
Compare
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-12-30 21:45:19.892689916 +0000
+++ new-output.txt 2025-12-30 21:45:20.209690328 +0000
@@ -805,7 +805,7 @@
overloads_evaluation.py:291:33: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `T@example6`
protocols_class_objects.py:58:16: error[invalid-assignment] Object of type `<class 'ConcreteA'>` is not assignable to `ProtoA1`
protocols_class_objects.py:59:16: error[invalid-assignment] Object of type `<class 'ConcreteA'>` is not assignable to `ProtoA2`
-protocols_definition.py:30:11: error[invalid-argument-type] Argument to function `close_all` is incorrect: Expected `Iterable[SupportsClose]`, found `list[Unknown | int]`
+protocols_definition.py:30:11: error[invalid-argument-type] Argument to function `close_all` is incorrect: Expected `Iterable[SupportsClose]`, found `list[int]`
protocols_definition.py:114:22: error[invalid-assignment] Object of type `Concrete2_Bad1` is not assignable to `Template2`
protocols_definition.py:115:22: error[invalid-assignment] Object of type `Concrete2_Bad2` is not assignable to `Template2`
protocols_definition.py:116:22: error[invalid-assignment] Object of type `Concrete2_Bad3` is not assignable to `Template2`
|
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
14 | 96 | 200 |
type-assertion-failure |
0 | 128 | 18 |
invalid-assignment |
4 | 21 | 63 |
invalid-return-type |
1 | 7 | 11 |
possibly-missing-attribute |
1 | 7 | 1 |
unresolved-attribute |
9 | 0 | 0 |
invalid-await |
0 | 0 | 6 |
no-matching-overload |
1 | 5 | 0 |
not-subscriptable |
0 | 4 | 0 |
unsupported-operator |
1 | 0 | 3 |
unused-ignore-comment |
1 | 2 | 0 |
not-iterable |
0 | 1 | 1 |
| Total | 32 | 271 | 303 |
CodSpeed Performance ReportMerging #21930 will degrade performance by 4.81%Comparing Summary
Benchmarks breakdown
Footnotes
|
ibraheemdev
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
Member
Author
|
Still going through the ecosystem report, but it looks more promising than #21747. |
This was referenced Dec 18, 2025
ibraheemdev
changed the base branch from
main
to
ibraheem/failed-multi-inference
ibraheemdev
added a commit
that referenced
this pull request
Dec 19, 2025
## Summary This should make revealed types a little nicer, as well as avoid confusing the constraint solver in some cases (which were showing up in #21930).
ibraheemdev
force-pushed
the
ibraheem/bidi-subtyping2
branch
3 times, most recently
from
3977d9f to
fd811c4
Compare
Member
Author
|
Ecosystem report looks a lot better now! |
Contributor
|
I went through all the examples in astral-sh/ty#1576 (and everything closed as a duplicate of it.) Most of them are fixed by this PR. astral-sh/ty#1648 is a bug in the user code snippet. Ironically the one case which is not fixed by this PR is the original snippet reported by @AlexWaygood in astral-sh/ty#1576: from typing import reveal_type, Iterable
x: Iterable[list[int]] = [[42], [56]]
reveal_type(x)Even with this PR, we still reveal Overall this PR is great and we should get it merged. So if that's easy to fix here, great. If not, let's leave astral-sh/ty#1576 open (or open a new issue) to track that case. |
carljm
reviewed
Dec 20, 2025
Contributor
carljm
left a comment
carljm
left a comment
There was a problem hiding this comment.
(I know I said we should get this merged, but I'm out of time right now to complete the code review; will try to get back to it Monday.)
Comment on lines
+440
to
+446
| # TODO: We could choose a concrete type here. | ||
| x10: list[int | str] | list[int | None] = [1, 2, 3] | ||
| reveal_type(x10) # revealed: list[Unknown | int] | ||
|
|
||
| # TODO: And here similarly. | ||
| x11: Sequence[int | str] | Sequence[int | None] = [1, 2, 3] | ||
| reveal_type(x11) # revealed: list[Unknown | int] |
Contributor
There was a problem hiding this comment.
I've actually come around to the point of view that the right resolution to astral-sh/ty#136 is to always prefer the declared type in an annotated assignment, and consider it a feature that x: int | str = 1 has different behavior from x: int | str followed by x = 1, since that "inconsistency" actually gives the user more control, and is visually intuitive (in the sense that a directly annotated assignment looks more like a "cast" of the RHS, not just setting an upper bound.)
Which means that when we make that change, we'd want to reveal the full union type in both of these cases.
But I think that's kind of unrelated to the point of the test. We'd probably just want to rewrite all these tests to separate the declaration (type context) onto a separate line from the assignment, so we are still checking how type context influences type inference, not just verifying our preference for the declared type.
Member
Author
There was a problem hiding this comment.
We could also put the reveal_type on the RHS expression directly.
Member
Author
|
@carljm that example should be fixed now, thanks. |
Member
|
Looks like we're timing out on |
ibraheemdev
force-pushed
the
ibraheem/bidi-subtyping2
branch
3 times, most recently
from
7bdeee0 to
01cd956
Compare
Member
Author
|
I simplified the implementation a bit, and that seems to have resolved the timeout and performance regressions. |
This was referenced Dec 23, 2025
carljm
approved these changes
Dec 30, 2025
| let value_type = visitor.visit(self, || alias.raw_value_type(db).apply_type_mapping_impl(db, type_mapping, tcx, visitor)); | ||
| let value_type = visitor.visit(self, || { | ||
| match type_mapping { | ||
| TypeMapping::UniqueSpecialization { .. } => alias.raw_value_type(db), |
Contributor
There was a problem hiding this comment.
Why is it correct to just return the raw value type without applying the UniqueSpecialization to it at all?
Member
Author
There was a problem hiding this comment.
We apply the type mapping just below after specializing the type alias, doing it here would cause the type mapping to be applied twice, and the unique specialization would be performed on a synthetic type variable the second time, instead of the actual type alias specialization.
ibraheemdev
force-pushed
the
ibraheem/bidi-subtyping2
branch
2 times, most recently
from
060993f to
3814ca4
Compare
ibraheemdev
force-pushed
the
ibraheem/bidi-subtyping2
branch
from
3814ca4 to
2cd2a50
Compare
carljm
reviewed
Dec 30, 2025
| // not the raw value type. | ||
| TypeMapping::UniqueSpecialization { .. } => alias.raw_value_type(db), | ||
|
|
||
| _ => alias.raw_value_type(db).apply_type_mapping_impl(db, type_mapping, tcx, visitor), |
Contributor
There was a problem hiding this comment.
Does that mean we are applying all other type mappings twice?
Member
Author
There was a problem hiding this comment.
I believe so, but removing this (or even just adding TypeMapping::Specialization(_) to the branch above) seems to cause stack overflows and incorrect Divergent types, so I think this is necessary for at least some type mappings.
Member
Author
|
FWIW I discussed the core ideas behind this PR with @dcreager, so this should be good to land! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
Supersedes #21747. This version uses the constraint solver directly, which means we should benefit from constraint solver improvements for free.
Resolves astral-sh/ty#1576.