[red-knot] Implement disjointness for Instance types where the underlying class is @final
#15539
Conversation
…rlying class is `@final`
| (Type::SubclassOf(subclass_of_ty), instance @ Type::Instance(_)) | ||
| | (instance @ Type::Instance(_), Type::SubclassOf(subclass_of_ty)) => { | ||
| // `type[T]` is disjoint from `S`, where `S` is an instance type, | ||
| // if `U` is disjoint from `S`, | ||
| // where `U` represents all instances of `T`'s metaclass | ||
| let metaclass_instance = subclass_of_ty | ||
| .subclass_of() | ||
| .into_class() | ||
| .map(|class| class.metaclass(db).to_instance(db)) | ||
| .unwrap_or_else(|| KnownClass::Type.to_instance(db)); | ||
| instance.is_disjoint_from(db, metaclass_instance) | ||
| } |
There was a problem hiding this comment.
@carljm I realised almost as soon as our pairing session here that we were missing some subtle edge cases where a type[T] type could still be disjoint from an instance type even if the instance type was a subtype of type. So this logic is a little fiddlier than what we initially implemented. See the mdtest I've added for examples!
There was a problem hiding this comment.
Did a property test reveal this, or you just realized it looking at the code/tests?
There was a problem hiding this comment.
Just me reading the diff on github prior to filing the PR!
|
| (Type::SubclassOf(subclass_of_ty), instance @ Type::Instance(_)) | ||
| | (instance @ Type::Instance(_), Type::SubclassOf(subclass_of_ty)) => { | ||
| // `type[T]` is disjoint from `S`, where `S` is an instance type, | ||
| // if `U` is disjoint from `S`, | ||
| // where `U` represents all instances of `T`'s metaclass | ||
| let metaclass_instance = subclass_of_ty | ||
| .subclass_of() | ||
| .into_class() | ||
| .map(|class| class.metaclass(db).to_instance(db)) | ||
| .unwrap_or_else(|| KnownClass::Type.to_instance(db)); | ||
| instance.is_disjoint_from(db, metaclass_instance) | ||
| } |
There was a problem hiding this comment.
Did a property test reveal this, or you just realized it looking at the code/tests?
crates/red_knot_python_semantic/resources/mdtest/type_properties/is_disjoint_from.md
Outdated
Show resolved
Hide resolved
| // `type[T]` is disjoint from `S`, where `S` is an instance type, | ||
| // if `U` is disjoint from `S`, | ||
| // where `U` represents all instances of `T`'s metaclass | ||
| let metaclass_instance = subclass_of_ty | ||
| .subclass_of() | ||
| .into_class() | ||
| .map(|class| class.metaclass(db).to_instance(db)) | ||
| .unwrap_or_else(|| KnownClass::Type.to_instance(db)); | ||
| instance.is_disjoint_from(db, metaclass_instance) |
There was a problem hiding this comment.
Nice! I like that we can delegate to instance-vs-instance, rather than checking finality here also.
It seems right that T actually disappears from this check, apart from its metaclass. There's no difference between type[object] and type[OtherClass] as regards disjointness from an instance type, as long as the metaclass of OtherClass is just type.
…es/is_disjoint_from.md Co-authored-by: Carl Meyer <carl@astral.sh>
Summary
Closes #15508
For any two instance types
TandS, we know they are disjoint if eitherTis final andTis not a subclass ofSorSis final andSis not a subclass ofT.Correspondingly, for any two types
type[T]andSwhereSis an instance type,type[T]can be said to be disjoint fromSifSis disjoint fromU, whereUis the type that represents all instances ofT's metaclass.And a heterogeneous tuple type can be said to be disjoint from an instance type if the instance type is disjoint from
tuple(a type representing all instances of thetupleclass at runtime).Test Plan
is_disjoint_from()tests are not written as mdtests just yet, but it's pretty hard to test some of these edge cases from a Rust unit test!QUICKCHECK_TESTS=1000000 cargo test --release -p red_knot_python_semantic -- --ignored types::property_tests::stableCo-authored-by: Carl Meyer carl@astral.sh