[ty] Use 'unknown' specialization for upper bound on Self#20325
Conversation
sharkdp
added
bug
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
sharkdp
force-pushed
the
david/fix-1156
branch
from
53b3605 to
70a825d
Compare
| let instance = Type::ClassLiteral(class).to_instance(db).expect( | ||
| "nearest_enclosing_class must return type that can be instantiated", | ||
| ); |
There was a problem hiding this comment.
This would previously use the "default" specialization, and so we would end up with an upper bound of Container[bytes] for the example in the mdtest. With the change here, we instead use Container[Unknown] as the upper bound.
| ```toml | ||
| [environment] | ||
| python-version = "3.11" | ||
| python-version = "3.13" |
There was a problem hiding this comment.
Default values for type parameters have only been introduced recently (https://peps.python.org/pep-0696/)
sharkdp
requested review from
AlexWaygood,
carljm and
dcreager
as code owners
| let instance = Type::ClassLiteral(class).to_instance(db).expect( | ||
| "nearest_enclosing_class must return type that can be instantiated", | ||
| ); | ||
| let instance = Type::instance(db, class.unknown_specialization(db)); |
There was a problem hiding this comment.
I wondered if class.top_materialization() would work here, but it looks like under the hood that uses default_specialization(), so it suffers from the same issue as our current logic on main. (It probably shouldn't use default_specialization()?)
There was a problem hiding this comment.
It probably shouldn't use
default_specialization()
I think you're right — it should translate typevars into the bound/constraints in covariant position, and to Never (the implicit lower bound of every typevar) in contravariant position. (With the caveat that we can't really express the constraints of a constrained typevar as a single type — we'd need a "one-of" connective instead of union, and an "instance of this type but not any subtypes".)
dcreager
left a comment
dcreager
left a comment
There was a problem hiding this comment.
Thank you for tracking this down!
| let instance = Type::ClassLiteral(class).to_instance(db).expect( | ||
| "nearest_enclosing_class must return type that can be instantiated", | ||
| ); | ||
| let instance = Type::instance(db, class.unknown_specialization(db)); |
There was a problem hiding this comment.
It probably shouldn't use
default_specialization()
I think you're right — it should translate typevars into the bound/constraints in covariant position, and to Never (the implicit lower bound of every typevar) in contravariant position. (With the caveat that we can't really express the constraints of a constrained typevar as a single type — we'd need a "one-of" connective instead of union, and an "instance of this type but not any subtypes".)
3 participants
Summary
closes astral-sh/ty#1156
Test Plan
Added a regression test