From 8c887d78e170e71a53c46622542af57d52c2a048 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 11 Dec 2024 20:24:42 +0000 Subject: [PATCH 1/8] [red-knot] Make `is_subtype_of` exhaustive --- crates/red_knot_python_semantic/src/types.rs | 288 +++++++++++++------ 1 file changed, 195 insertions(+), 93 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 8292e9b14b127..6f79bae9ad577 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -615,104 +615,50 @@ impl<'db> Type<'db> { /// /// [subtype of]: https://typing.readthedocs.io/en/latest/spec/concepts.html#subtype-supertype-and-type-equivalence pub(crate) fn is_subtype_of(self, db: &'db dyn Db, target: Type<'db>) -> bool { + // Two equivalent types are always equal to each other. + // + // "Equivalent to" here means that the two types are both fully static + // and can be determined to point to exactly the same set of possible runtime objects. + // For example, `int` is a subtype of `int` because `int` and `int` are equivalent to each other. + // Equally, `type[object]` is a subtype of `type`, + // because the former type expresses "all subclasses of `object`" + // while the latter expresses "all instances of `type`", + // and these are exactly the same set of objects at runtime. if self.is_equivalent_to(db, target) { return true; } + + // Non-fully-static types do not participate in subtyping. + // + // Type `A` can only be a subtype of type `B` if the set of possible runtime objects + // that `A` represents is a subset of the set of possible runtime objects that `B` represents. + // But the set of objects that a fully static type represents is (either partially or wholly) unknown, + // so the question is simply unanswerable for fully static types. if !self.is_fully_static(db) || !target.is_fully_static(db) { return false; } + match (self, target) { + // We should have handled these immediately above. + (Type::Any | Type::Unknown | Type::Todo(_), _) + | (_, Type::Any | Type::Unknown | Type::Todo(_)) => { + unreachable!("Non-fully-static types do not participate in subtyping!") + } + + // `Never` is the bottom type, the empty set. + // It is a subtype of all other fully static types. + // No other fully static type is a subtype of `Never`. (Type::Never, _) => true, (_, Type::Never) => false, - (_, Type::Instance(InstanceType { class })) - if class.is_known(db, KnownClass::Object) => - { - true - } - (Type::Instance(InstanceType { class }), _) - if class.is_known(db, KnownClass::Object) => - { - false - } - (Type::BooleanLiteral(_), Type::Instance(InstanceType { class })) - if matches!(class.known(db), Some(KnownClass::Bool | KnownClass::Int)) => - { - true - } - (Type::IntLiteral(_), Type::Instance(InstanceType { class })) - if class.is_known(db, KnownClass::Int) => - { - true - } - (Type::StringLiteral(_), Type::LiteralString) => true, - ( - Type::StringLiteral(_) | Type::LiteralString, - Type::Instance(InstanceType { class }), - ) if class.is_known(db, KnownClass::Str) => true, - (Type::BytesLiteral(_), Type::Instance(InstanceType { class })) - if class.is_known(db, KnownClass::Bytes) => - { - true - } - (Type::Tuple(self_tuple), Type::Tuple(target_tuple)) => { - let self_elements = self_tuple.elements(db); - let target_elements = target_tuple.elements(db); - self_elements.len() == target_elements.len() - && self_elements.iter().zip(target_elements).all( - |(self_element, target_element)| { - self_element.is_subtype_of(db, *target_element) - }, - ) - } - ( - Type::ClassLiteral(ClassLiteralType { class: self_class }), - Type::SubclassOf(SubclassOfType { - base: ClassBase::Class(target_class), - }), - ) => self_class.is_subclass_of(db, target_class), - ( - Type::Instance(_), - Type::SubclassOf(SubclassOfType { - base: ClassBase::Class(target_class), - }), - ) if target_class.is_known(db, KnownClass::Object) => { - self.is_subtype_of(db, KnownClass::Type.to_instance(db)) - } - ( - Type::SubclassOf(SubclassOfType { - base: ClassBase::Class(self_class), - }), - Type::SubclassOf(SubclassOfType { - base: ClassBase::Class(target_class), - }), - ) => self_class.is_subclass_of(db, target_class), - // C ⊆ type - // type[C] ⊆ type - // Though note that this works regardless of which metaclass C has, not just for type. - ( - Type::ClassLiteral(ClassLiteralType { class: self_class }) - | Type::SubclassOf(SubclassOfType { - base: ClassBase::Class(self_class), - }), - Type::Instance(InstanceType { - class: target_class, - }), - ) if self_class - .metaclass(db) - .into_class_literal() - .map(|meta| meta.class.is_subclass_of(db, target_class)) - .unwrap_or(false) => - { - true - } - (Type::Union(union), ty) => union + + (Type::Union(union), _) => union .elements(db) .iter() - .all(|&elem_ty| elem_ty.is_subtype_of(db, ty)), - (ty, Type::Union(union)) => union + .all(|&elem_ty| elem_ty.is_subtype_of(db, target)), + (_, Type::Union(union)) => union .elements(db) .iter() - .any(|&elem_ty| ty.is_subtype_of(db, elem_ty)), + .any(|&elem_ty| self.is_subtype_of(db, elem_ty)), (Type::Intersection(self_intersection), Type::Intersection(target_intersection)) => { // Check that all target positive values are covered in self positive values target_intersection @@ -739,26 +685,152 @@ impl<'db> Type<'db> { }) }) } - (Type::Intersection(intersection), ty) => intersection + (Type::Intersection(intersection), _) => intersection .positive(db) .iter() - .any(|&elem_ty| elem_ty.is_subtype_of(db, ty)), - (ty, Type::Intersection(intersection)) => { + .any(|&elem_ty| elem_ty.is_subtype_of(db, target)), + (_, Type::Intersection(intersection)) => { intersection .positive(db) .iter() - .all(|&pos_ty| ty.is_subtype_of(db, pos_ty)) + .all(|&pos_ty| self.is_subtype_of(db, pos_ty)) && intersection .negative(db) .iter() - .all(|&neg_ty| neg_ty.is_disjoint_from(db, ty)) + .all(|&neg_ty| neg_ty.is_disjoint_from(db, self)) } + + // All `StringLiteral` types are a subtype of `LiteralString`. + (Type::StringLiteral(_), Type::LiteralString) => true, + + // Except for the special `LiteralString` case above, + // most `Literal` types delegate to their instance fallbacks + // unless `self` is exactly equivalent to `target` (handled above) + (Type::StringLiteral(_) | Type::LiteralString, _) => { + KnownClass::Str.to_instance(db).is_subtype_of(db, target) + } + (Type::BooleanLiteral(_), _) => { + KnownClass::Bool.to_instance(db).is_subtype_of(db, target) + } + (Type::IntLiteral(_), _) => KnownClass::Int.to_instance(db).is_subtype_of(db, target), + (Type::BytesLiteral(_), _) => { + KnownClass::Bytes.to_instance(db).is_subtype_of(db, target) + } + (Type::ModuleLiteral(_), _) => KnownClass::ModuleType + .to_instance(db) + .is_subtype_of(db, target), + (Type::SliceLiteral(_), _) => { + KnownClass::Slice.to_instance(db).is_subtype_of(db, target) + } + + // A `FunctionLiteral` type is a single-valued type like the other literals handled above, + // so it also, for now, just delegates to its instance fallback. + // This will change in a way similar to the `LiteralString`/`StringLiteral()` case above + // when we add support for `typing.Callable`. + (Type::FunctionLiteral(_), _) => KnownClass::FunctionType + .to_instance(db) + .is_subtype_of(db, target), + + // A fully static heterogenous tuple type `A` is a subtype of a fully static heterogeneous tuple type `B` + // iff the two tuple types have the same number of elements and each element-type in `A` is a subtype + // of the element-type at the same index in `B`. (Now say that 5 times fast.) + // + // For example: `tuple[bool, bool]` is a subtype of `tuple[int, int]`, + // but `tuple[bool, bool, bool]` is not a subtype of `tuple[int, int]` + (Type::Tuple(self_tuple), Type::Tuple(target_tuple)) => { + let self_elements = self_tuple.elements(db); + let target_elements = target_tuple.elements(db); + self_elements.len() == target_elements.len() + && self_elements.iter().zip(target_elements).all( + |(self_element, target_element)| { + self_element.is_subtype_of(db, *target_element) + }, + ) + } + + // Other than the special tuple-to-tuple case handled, above, + // tuple subtyping delegates to `Instance(tuple)` in the same way as the literal types. + // + // All heterogenous tuple types are subtypes of `Instance()`: + // `Instance()` expresses "the set of all possible instances of the class `T`"; + // consequently, `Instance()` expresses "the set of all possible instances of the class `tuple`". + // This type can be spelled in type annotations as `tuple[object, ...]` (since `tuple` is covariant). + // + // Note that this is not the same type as the type spelled in type annotations as `tuple`; + // as that type is equivalent to `type[Any, ...]` (and therefore not a fully static type). + (Type::Tuple(_), _) => KnownClass::Tuple.to_instance(db).is_subtype_of(db, target), + + // `Type::ClassLiteral(T)` expresses "the set of size one with only the class `T` in it; + // `Type::SubclassOf(S)` expresses "the set that contains all subclasses of the class `S`". + // The first is only a subtype of the second if `T` is a subclass of `S`. + // + // For example, `Literal[int]` and `Literal[bool]` are both subtypes of `type[int]`. + ( + Type::ClassLiteral(ClassLiteralType { class: self_class }), + Type::SubclassOf(SubclassOfType { + base: ClassBase::Class(target_class), + }), + ) => self_class.is_subclass_of(db, target_class), + + // `Literal[str]` is a subtype of `type` because `str` is an instance of `type`. + // `Literal[enum.Enum]` is a subtype of `enum.EnumMeta` because `enum.Enum` + // is an instance of `enum.EnumMeta`. + ( + Type::ClassLiteral(self_class_ty), + Type::Instance(InstanceType { + class: target_class, + }), + ) => self_class_ty.is_instance_of(db, target_class), + + // Other than the cases enumerated above, + // class literals aren't subtypes of any other types. + (Type::ClassLiteral(_), _) => false, + + // `type[T]` delegates to `Literal[T]` + // when deciding if `type[T]` is a subtype of another type + // (but only if `T` is a fully static type) + ( + Type::SubclassOf(SubclassOfType { + base: ClassBase::Class(self_class), + }), + _, + ) => Type::class_literal(self_class).is_subtype_of(db, target), + + // As the `unreachable!()` message says, these should all be handled + // right at the top of this function. + ( + Type::SubclassOf(SubclassOfType { + base: ClassBase::Any | ClassBase::Unknown | ClassBase::Todo(_), + }), + _, + ) => unreachable!( + "Non-fully-static types should be handled at the top of this function!" + ), + + // For example: `Type::KnownInstance(KnownInstanceType::Type)` is a subtype of `Type::Instance(_SpecialForm)`, + // because the symbol `typing.Type` is an instance of `typing._SpecialForm` at runtime (Type::KnownInstance(left), right) => { left.instance_fallback(db).is_subtype_of(db, right) } + + // For example, `Instance(ABCMeta)` is a subtype of `type[object]`, + // because (since `ABCMeta` subclasses `type`) all instances of `ABCMeta` are instances of `type`, + // and all instances of `type` are members of the type `type[object]` + ( + Type::Instance(_), + Type::SubclassOf(SubclassOfType { + base: ClassBase::Class(target_class), + }), + ) if target_class.is_known(db, KnownClass::Object) => { + self.is_subtype_of(db, KnownClass::Type.to_instance(db)) + } + + // `bool` is a subtype of `int`, because all instances of `bool` are also instances of `int`. (Type::Instance(left), Type::Instance(right)) => left.is_instance_of(db, right.class), - // TODO - _ => false, + + // Other than the special cases enumerated above, + // `Instance` types are never subtypes of any other variants + (Type::Instance(_), _) => false, } } @@ -2983,6 +3055,18 @@ impl<'db> ClassLiteralType<'db> { fn member(self, db: &'db dyn Db, name: &str) -> Symbol<'db> { self.class.class_member(db, name) } + + /// Return `true` if the singleton class object represented by this type + /// is an instance of the class `other`. + /// + /// A class is an instance of its metaclass; consequently, + /// a class will only ever be an instance of another class + /// if its metaclass is a subclass of that other class. + fn is_instance_of(self, db: &'db dyn Db, other: Class<'db>) -> bool { + self.class.metaclass(db).into_class_literal().is_some_and( + |ClassLiteralType { class: metaclass }| metaclass.is_subclass_of(db, other), + ) + } } impl<'db> From> for Type<'db> { @@ -3186,7 +3270,7 @@ pub(crate) mod tests { use super::*; use crate::db::tests::{setup_db, TestDb, TestDbBuilder}; use crate::stdlib::typing_symbol; - use crate::PythonVersion; + use crate::{resolve_module, PythonVersion}; use ruff_db::files::system_path_to_file; use ruff_db::parsed::parsed_module; use ruff_db::system::DbWithTestSystem; @@ -3226,6 +3310,8 @@ pub(crate) mod tests { Tuple(Vec), SubclassOfAny, SubclassOfBuiltinClass(&'static str), + StdlibModule(CoreStdlibModule), + SliceLiteral(i32, i32, i32), } impl Ty { @@ -3276,6 +3362,15 @@ pub(crate) mod tests { .expect_class_literal() .class, ), + Ty::StdlibModule(module) => { + Type::ModuleLiteral(resolve_module(db, &module.name()).unwrap().file()) + } + Ty::SliceLiteral(start, stop, step) => Type::SliceLiteral(SliceLiteralType::new( + db, + Some(start), + Some(stop), + Some(step), + )), } } } @@ -3402,6 +3497,13 @@ pub(crate) mod tests { #[test_case(Ty::TypingLiteral, Ty::BuiltinInstance("object"))] #[test_case(Ty::AbcClassLiteral("ABC"), Ty::AbcInstance("ABCMeta"))] #[test_case(Ty::AbcInstance("ABCMeta"), Ty::SubclassOfBuiltinClass("object"))] + #[test_case(Ty::Tuple(vec![Ty::BuiltinInstance("int")]), Ty::BuiltinInstance("tuple"))] + #[test_case(Ty::BuiltinClassLiteral("str"), Ty::BuiltinInstance("type"))] + #[test_case( + Ty::StdlibModule(CoreStdlibModule::Typing), + Ty::KnownClassInstance(KnownClass::ModuleType) + )] + #[test_case(Ty::SliceLiteral(1, 2, 3), Ty::BuiltinInstance("slice"))] fn is_subtype_of(from: Ty, to: Ty) { let db = setup_db(); assert!(from.into_type(&db).is_subtype_of(&db, to.into_type(&db))); From 6cbbf4a9e1d42432b7a3c1eb90c4db147bdfb1c3 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 13 Dec 2024 11:24:33 +0000 Subject: [PATCH 2/8] make the comments make sense Co-authored-by: Carl Meyer --- crates/red_knot_python_semantic/src/types.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 6f79bae9ad577..5d12fa4eb70ee 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -615,10 +615,10 @@ impl<'db> Type<'db> { /// /// [subtype of]: https://typing.readthedocs.io/en/latest/spec/concepts.html#subtype-supertype-and-type-equivalence pub(crate) fn is_subtype_of(self, db: &'db dyn Db, target: Type<'db>) -> bool { - // Two equivalent types are always equal to each other. + // Two equivalent types are always subtypes of each other. // // "Equivalent to" here means that the two types are both fully static - // and can be determined to point to exactly the same set of possible runtime objects. + // and point to exactly the same set of possible runtime objects. // For example, `int` is a subtype of `int` because `int` and `int` are equivalent to each other. // Equally, `type[object]` is a subtype of `type`, // because the former type expresses "all subclasses of `object`" @@ -632,8 +632,8 @@ impl<'db> Type<'db> { // // Type `A` can only be a subtype of type `B` if the set of possible runtime objects // that `A` represents is a subset of the set of possible runtime objects that `B` represents. - // But the set of objects that a fully static type represents is (either partially or wholly) unknown, - // so the question is simply unanswerable for fully static types. + // But the set of objects described by a non-fully-static type is (either partially or wholly) unknown, + // so the question is simply unanswerable for non-fully-static types. if !self.is_fully_static(db) || !target.is_fully_static(db) { return false; } From 14fa1f293eedd13a62f65ecb74f1c46c729e66f2 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 13 Dec 2024 11:46:59 +0000 Subject: [PATCH 3/8] fix `type[]` subtyping --- crates/red_knot_python_semantic/src/types.rs | 67 ++++++++++---------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 5d12fa4eb70ee..de429a4ea32d2 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -760,13 +760,35 @@ impl<'db> Type<'db> { // as that type is equivalent to `type[Any, ...]` (and therefore not a fully static type). (Type::Tuple(_), _) => KnownClass::Tuple.to_instance(db).is_subtype_of(db, target), - // `Type::ClassLiteral(T)` expresses "the set of size one with only the class `T` in it; - // `Type::SubclassOf(S)` expresses "the set that contains all subclasses of the class `S`". - // The first is only a subtype of the second if `T` is a subclass of `S`. - // - // For example, `Literal[int]` and `Literal[bool]` are both subtypes of `type[int]`. + // `Type::ClassLiteral` always delegates to `Type::SubclassOf`: + (Type::ClassLiteral(ClassLiteralType { class }), _) => { + Type::subclass_of(class).is_subtype_of(db, target) + } + + // As the `unreachable!()` message says, non-fully-static `SubclassOf` types such as + // `type[Any]`,` type[Unknown]` and `type[Todo]` should all be handled right at the top of this function. + ( + Type::SubclassOf(SubclassOfType { + base: ClassBase::Any | ClassBase::Unknown | ClassBase::Todo(_), + }), + _, + ) + | ( + _, + Type::SubclassOf(SubclassOfType { + base: ClassBase::Any | ClassBase::Unknown | ClassBase::Todo(_), + }), + ) => unreachable!( + "Non-fully-static types should be handled at the top of this function!" + ), + + // For example, `type[bool]` describes all possible runtime subclasses of the class `bool`, + // and `type[int]` describes all possible runtime subclasses of the class `int`. + // The first set is a subset of the second set, because `bool` is itself a subclass of `int`. ( - Type::ClassLiteral(ClassLiteralType { class: self_class }), + Type::SubclassOf(SubclassOfType { + base: ClassBase::Class(self_class), + }), Type::SubclassOf(SubclassOfType { base: ClassBase::Class(target_class), }), @@ -775,37 +797,17 @@ impl<'db> Type<'db> { // `Literal[str]` is a subtype of `type` because `str` is an instance of `type`. // `Literal[enum.Enum]` is a subtype of `enum.EnumMeta` because `enum.Enum` // is an instance of `enum.EnumMeta`. - ( - Type::ClassLiteral(self_class_ty), - Type::Instance(InstanceType { - class: target_class, - }), - ) => self_class_ty.is_instance_of(db, target_class), - - // Other than the cases enumerated above, - // class literals aren't subtypes of any other types. - (Type::ClassLiteral(_), _) => false, - - // `type[T]` delegates to `Literal[T]` - // when deciding if `type[T]` is a subtype of another type - // (but only if `T` is a fully static type) ( Type::SubclassOf(SubclassOfType { base: ClassBase::Class(self_class), }), - _, - ) => Type::class_literal(self_class).is_subtype_of(db, target), - - // As the `unreachable!()` message says, these should all be handled - // right at the top of this function. - ( - Type::SubclassOf(SubclassOfType { - base: ClassBase::Any | ClassBase::Unknown | ClassBase::Todo(_), + Type::Instance(InstanceType { + class: target_class, }), - _, - ) => unreachable!( - "Non-fully-static types should be handled at the top of this function!" - ), + ) => ClassLiteralType { class: self_class }.is_instance_of(db, target_class), + + // Other than the cases enumerated above, `type[]` just delegates to `Instance("type")` + (Type::SubclassOf(_), _) => KnownClass::Type.to_instance(db).is_subtype_of(db, target), // For example: `Type::KnownInstance(KnownInstanceType::Type)` is a subtype of `Type::Instance(_SpecialForm)`, // because the symbol `typing.Type` is an instance of `typing._SpecialForm` at runtime @@ -3537,6 +3539,7 @@ pub(crate) mod tests { #[test_case(Ty::BuiltinInstance("type"), Ty::SubclassOfBuiltinClass("str"))] #[test_case(Ty::BuiltinClassLiteral("str"), Ty::SubclassOfAny)] #[test_case(Ty::AbcInstance("ABCMeta"), Ty::SubclassOfBuiltinClass("type"))] + #[test_case(Ty::SubclassOfBuiltinClass("str"), Ty::BuiltinClassLiteral("str"))] fn is_not_subtype_of(from: Ty, to: Ty) { let db = setup_db(); assert!(!from.into_type(&db).is_subtype_of(&db, to.into_type(&db))); From 968a2529fdd700a24a1b06306d0fddb26a17bbc0 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 13 Dec 2024 11:59:43 +0000 Subject: [PATCH 4/8] readability improvements --- crates/red_knot_python_semantic/src/types.rs | 38 +++++++++---------- .../src/types/infer.rs | 6 +-- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index de429a4ea32d2..3e6021b646621 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -681,7 +681,7 @@ impl<'db> Type<'db> { target_neg_elem.is_subtype_of(db, self_neg_elem) // Is target negative value is disjoint from a self positive value? }) || self_intersection.positive(db).iter().any(|&self_pos_elem| { - target_neg_elem.is_disjoint_from(db, self_pos_elem) + self_pos_elem.is_disjoint_from(db, target_neg_elem) }) }) } @@ -697,7 +697,7 @@ impl<'db> Type<'db> { && intersection .negative(db) .iter() - .all(|&neg_ty| neg_ty.is_disjoint_from(db, self)) + .all(|&neg_ty| self.is_disjoint_from(db, neg_ty)) } // All `StringLiteral` types are a subtype of `LiteralString`. @@ -804,7 +804,7 @@ impl<'db> Type<'db> { Type::Instance(InstanceType { class: target_class, }), - ) => ClassLiteralType { class: self_class }.is_instance_of(db, target_class), + ) => self_class.is_instance_of(db, target_class), // Other than the cases enumerated above, `type[]` just delegates to `Instance("type")` (Type::SubclassOf(_), _) => KnownClass::Type.to_instance(db).is_subtype_of(db, target), @@ -828,7 +828,9 @@ impl<'db> Type<'db> { } // `bool` is a subtype of `int`, because all instances of `bool` are also instances of `int`. - (Type::Instance(left), Type::Instance(right)) => left.is_instance_of(db, right.class), + (Type::Instance(self_instance), Type::Instance(target_instance)) => { + self_instance.is_subtype_of(db, target_instance) + } // Other than the special cases enumerated above, // `Instance` types are never subtypes of any other variants @@ -2853,6 +2855,17 @@ impl<'db> Class<'db> { self.iter_mro(db).contains(&ClassBase::Class(other)) } + /// Return `true` if this class object is an instance of the class `other`. + /// + /// A class is an instance of its metaclass; consequently, + /// a class will only ever be an instance of another class + /// if its metaclass is a subclass of that other class. + fn is_instance_of(self, db: &'db dyn Db, other: Class<'db>) -> bool { + self.metaclass(db).into_class_literal().is_some_and( + |ClassLiteralType { class: metaclass }| metaclass.is_subclass_of(db, other), + ) + } + /// Return the explicit `metaclass` of this class, if one is defined. /// /// ## Note @@ -3057,18 +3070,6 @@ impl<'db> ClassLiteralType<'db> { fn member(self, db: &'db dyn Db, name: &str) -> Symbol<'db> { self.class.class_member(db, name) } - - /// Return `true` if the singleton class object represented by this type - /// is an instance of the class `other`. - /// - /// A class is an instance of its metaclass; consequently, - /// a class will only ever be an instance of another class - /// if its metaclass is a subclass of that other class. - fn is_instance_of(self, db: &'db dyn Db, other: Class<'db>) -> bool { - self.class.metaclass(db).into_class_literal().is_some_and( - |ClassLiteralType { class: metaclass }| metaclass.is_subclass_of(db, other), - ) - } } impl<'db> From> for Type<'db> { @@ -3096,9 +3097,8 @@ pub struct InstanceType<'db> { } impl<'db> InstanceType<'db> { - /// Return `true` if members of this type are instances of the class `class` at runtime. - pub fn is_instance_of(self, db: &'db dyn Db, class: Class<'db>) -> bool { - self.class.is_subclass_of(db, class) + fn is_subtype_of(self, db: &'db dyn Db, other: InstanceType<'db>) -> bool { + self.class.is_subclass_of(db, other.class) } } diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 03eedaa8e5d30..008dffcb89b15 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -3375,8 +3375,8 @@ impl<'db> TypeInferenceBuilder<'db> { op, ), - (left_ty @ Type::Instance(left), right_ty @ Type::Instance(right), op) => { - if left != right && right.is_instance_of(self.db, left.class) { + (Type::Instance(left), Type::Instance(right), op) => { + if left != right && right.is_subtype_of(self.db, left) { let reflected_dunder = op.reflected_dunder(); let rhs_reflected = right.class.class_member(self.db, reflected_dunder); if !rhs_reflected.is_unbound() @@ -5320,7 +5320,7 @@ fn perform_rich_comparison<'db>( }; // The reflected dunder has priority if the right-hand side is a strict subclass of the left-hand side. - if left != right && right.is_instance_of(db, left.class) { + if left != right && right.is_subtype_of(db, left) { call_dunder(op.reflect(), right, left).or_else(|| call_dunder(op, left, right)) } else { call_dunder(op, left, right).or_else(|| call_dunder(op.reflect(), right, left)) From d497734d4d45d358c9cf7c424c83bdc7a89ff2cf Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 13 Dec 2024 12:11:19 +0000 Subject: [PATCH 5/8] further improvements to comments --- crates/red_knot_python_semantic/src/types.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 3e6021b646621..523727f182f1a 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -794,8 +794,11 @@ impl<'db> Type<'db> { }), ) => self_class.is_subclass_of(db, target_class), - // `Literal[str]` is a subtype of `type` because `str` is an instance of `type`. - // `Literal[enum.Enum]` is a subtype of `enum.EnumMeta` because `enum.Enum` + // `type[str]` (== `SubclassOf("str")` in red-knot) describes all possible runtime subclasses + // of the class object `str`. It is a subtype of `type` (== `Instance("type")`) because `str` + // is an instance of `type`, and so all possible subclasses of `str` will also be instances of `type`. + // + // Similarly `type[enum.Enum]` is a subtype of `enum.EnumMeta` because `enum.Enum` // is an instance of `enum.EnumMeta`. ( Type::SubclassOf(SubclassOfType { @@ -810,14 +813,16 @@ impl<'db> Type<'db> { (Type::SubclassOf(_), _) => KnownClass::Type.to_instance(db).is_subtype_of(db, target), // For example: `Type::KnownInstance(KnownInstanceType::Type)` is a subtype of `Type::Instance(_SpecialForm)`, - // because the symbol `typing.Type` is an instance of `typing._SpecialForm` at runtime + // because `Type::KnownInstance(KnownInstanceType::Type)` is a set with exactly one runtime value in it + // (the symbol `typing.Type`), and that symbol is known to be an instance of `typing._SpecialForm` at runtime. (Type::KnownInstance(left), right) => { left.instance_fallback(db).is_subtype_of(db, right) } - // For example, `Instance(ABCMeta)` is a subtype of `type[object]`, - // because (since `ABCMeta` subclasses `type`) all instances of `ABCMeta` are instances of `type`, - // and all instances of `type` are members of the type `type[object]` + // For example, `abc.ABCMeta` (== `Instance("abc.ABCMeta")`) is a subtype of `type[object]` + // (== `SubclassOf("object")`) because (since `abc.ABCMeta` subclasses `type`) all instances of `ABCMeta` + // are instances of `type`, and `type[object]` represents the set of all subclasses of `object`, + // which is exactly equal to the set of all instances of `type`. ( Type::Instance(_), Type::SubclassOf(SubclassOfType { @@ -827,7 +832,8 @@ impl<'db> Type<'db> { self.is_subtype_of(db, KnownClass::Type.to_instance(db)) } - // `bool` is a subtype of `int`, because all instances of `bool` are also instances of `int`. + // `bool` is a subtype of `int`, because `bool` subclasses `int`, + // which means that all instances of `bool` are also instances of `int` (Type::Instance(self_instance), Type::Instance(target_instance)) => { self_instance.is_subtype_of(db, target_instance) } From cfa122a35b3279fc86ee10db9f56e91a4fac0ab0 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 13 Dec 2024 14:34:48 +0000 Subject: [PATCH 6/8] excise use of "point to" --- crates/red_knot_python_semantic/src/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 523727f182f1a..7043187eb6bd0 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -618,7 +618,7 @@ impl<'db> Type<'db> { // Two equivalent types are always subtypes of each other. // // "Equivalent to" here means that the two types are both fully static - // and point to exactly the same set of possible runtime objects. + // and describe exactly the same set of possible runtime objects. // For example, `int` is a subtype of `int` because `int` and `int` are equivalent to each other. // Equally, `type[object]` is a subtype of `type`, // because the former type expresses "all subclasses of `object`" From c433e88dbf90c2dd4b6b0f2c3dac3c42386204ec Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 13 Dec 2024 19:14:31 +0000 Subject: [PATCH 7/8] Fix nontransitivity pointed out by property tests --- crates/red_knot_python_semantic/src/types.rs | 93 ++++++++++++++------ 1 file changed, 66 insertions(+), 27 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 7043187eb6bd0..2bdf55df08e96 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -655,10 +655,12 @@ impl<'db> Type<'db> { .elements(db) .iter() .all(|&elem_ty| elem_ty.is_subtype_of(db, target)), + (_, Type::Union(union)) => union .elements(db) .iter() .any(|&elem_ty| self.is_subtype_of(db, elem_ty)), + (Type::Intersection(self_intersection), Type::Intersection(target_intersection)) => { // Check that all target positive values are covered in self positive values target_intersection @@ -685,10 +687,12 @@ impl<'db> Type<'db> { }) }) } + (Type::Intersection(intersection), _) => intersection .positive(db) .iter() .any(|&elem_ty| elem_ty.is_subtype_of(db, target)), + (_, Type::Intersection(intersection)) => { intersection .positive(db) @@ -1006,6 +1010,8 @@ impl<'db> Type<'db> { } } + // any single-valued type is disjoint from another single-valued type + // iff the two types are nonequal ( left @ (Type::BooleanLiteral(..) | Type::IntLiteral(..) @@ -1014,7 +1020,8 @@ impl<'db> Type<'db> { | Type::SliceLiteral(..) | Type::FunctionLiteral(..) | Type::ModuleLiteral(..) - | Type::ClassLiteral(..)), + | Type::ClassLiteral(..) + | Type::KnownInstance(..)), right @ (Type::BooleanLiteral(..) | Type::IntLiteral(..) | Type::StringLiteral(..) @@ -1022,9 +1029,38 @@ impl<'db> Type<'db> { | Type::SliceLiteral(..) | Type::FunctionLiteral(..) | Type::ModuleLiteral(..) - | Type::ClassLiteral(..)), + | Type::ClassLiteral(..) + | Type::KnownInstance(..)), ) => left != right, + // One tuple type can be a subtype of another tuple type, + // but we know for sure that any given tuple type is disjoint from all single-valued types + ( + Type::Tuple(..), + Type::ClassLiteral(..) + | Type::ModuleLiteral(..) + | Type::BooleanLiteral(..) + | Type::BytesLiteral(..) + | Type::FunctionLiteral(..) + | Type::IntLiteral(..) + | Type::SliceLiteral(..) + | Type::StringLiteral(..) + | Type::LiteralString, + ) => true, + + ( + Type::ClassLiteral(..) + | Type::ModuleLiteral(..) + | Type::BooleanLiteral(..) + | Type::BytesLiteral(..) + | Type::FunctionLiteral(..) + | Type::IntLiteral(..) + | Type::SliceLiteral(..) + | Type::StringLiteral(..) + | Type::LiteralString, + Type::Tuple(..), + ) => true, + ( Type::SubclassOf(SubclassOfType { base: ClassBase::Class(class_a), @@ -1037,10 +1073,13 @@ impl<'db> Type<'db> { base: ClassBase::Class(class_a), }), ) => !class_b.is_subclass_of(db, class_a), + (Type::SubclassOf(_), Type::SubclassOf(_)) => false, + (Type::SubclassOf(_), Type::Instance(_)) | (Type::Instance(_), Type::SubclassOf(_)) => { false } + ( Type::SubclassOf(_), Type::BooleanLiteral(..) @@ -1061,6 +1100,7 @@ impl<'db> Type<'db> { | Type::ModuleLiteral(..), Type::SubclassOf(_), ) => true, + (Type::SubclassOf(_), _) | (_, Type::SubclassOf(_)) => { // TODO: Once we have support for final classes, we can determine disjointness in some cases // here. However, note that it might be better to turn `Type::SubclassOf('FinalClass')` into @@ -1068,13 +1108,14 @@ impl<'db> Type<'db> { // final classes inside `Type::SubclassOf` everywhere. false } - (Type::KnownInstance(left), Type::KnownInstance(right)) => left != right, + (Type::KnownInstance(left), right) => { left.instance_fallback(db).is_disjoint_from(db, right) } (left, Type::KnownInstance(right)) => { left.is_disjoint_from(db, right.instance_fallback(db)) } + ( Type::Instance(InstanceType { class: class_none }), Type::Instance(InstanceType { class: class_other }), @@ -1086,6 +1127,7 @@ impl<'db> Type<'db> { class_other.known(db), Some(KnownClass::NoneType | KnownClass::Object) ), + (Type::Instance(InstanceType { class: class_none }), _) | (_, Type::Instance(InstanceType { class: class_none })) if class_none.is_known(db, KnownClass::NoneType) => @@ -1112,7 +1154,6 @@ impl<'db> Type<'db> { | (Type::Instance(InstanceType { class }), Type::StringLiteral(..)) => { !matches!(class.known(db), Some(KnownClass::Str | KnownClass::Object)) } - (Type::StringLiteral(..), _) | (_, Type::StringLiteral(..)) => true, (Type::LiteralString, Type::LiteralString) => false, (Type::LiteralString, Type::Instance(InstanceType { class })) @@ -1126,14 +1167,12 @@ impl<'db> Type<'db> { class.known(db), Some(KnownClass::Bytes | KnownClass::Object) ), - (Type::BytesLiteral(..), _) | (_, Type::BytesLiteral(..)) => true, (Type::SliceLiteral(..), Type::Instance(InstanceType { class })) | (Type::Instance(InstanceType { class }), Type::SliceLiteral(..)) => !matches!( class.known(db), Some(KnownClass::Slice | KnownClass::Object) ), - (Type::SliceLiteral(..), _) | (_, Type::SliceLiteral(..)) => true, (Type::ClassLiteral(..), Type::Instance(InstanceType { class })) | (Type::Instance(InstanceType { class }), Type::ClassLiteral(..)) => { @@ -1161,30 +1200,29 @@ impl<'db> Type<'db> { false } - (Type::Tuple(tuple), other) | (other, Type::Tuple(tuple)) => { - if let Type::Tuple(other_tuple) = other { - if tuple.len(db) == other_tuple.len(db) { - tuple - .elements(db) - .iter() - .zip(other_tuple.elements(db)) - .any(|(e1, e2)| e1.is_disjoint_from(db, *e2)) - } else { - true - } + (Type::Tuple(tuple), Type::Tuple(other_tuple)) => { + if tuple.len(db) == other_tuple.len(db) { + tuple + .elements(db) + .iter() + .zip(other_tuple.elements(db)) + .any(|(e1, e2)| e1.is_disjoint_from(db, *e2)) } else { - // We can not be sure if the tuple is disjoint from 'other' because: - // - 'other' might be the homogeneous arbitrary-length tuple type - // tuple[T, ...] (which we don't have support for yet); if all of - // our element types are not disjoint with T, this is not disjoint - // - 'other' might be a user subtype of tuple, which, if generic - // over the same or compatible *Ts, would overlap with tuple. - // - // TODO: add checks for the above cases once we support them - - false + true } } + + (Type::Tuple(..), Type::Instance(..)) | (Type::Instance(..), Type::Tuple(..)) => { + // We can not be sure if the tuple is disjoint from the instance because: + // - 'other' might be the homogeneous arbitrary-length tuple type + // tuple[T, ...] (which we don't have support for yet); if all of + // our element types are not disjoint with T, this is not disjoint + // - 'other' might be a user subtype of tuple, which, if generic + // over the same or compatible *Ts, would overlap with tuple. + // + // TODO: add checks for the above cases once we support them + false + } } } @@ -3677,6 +3715,7 @@ pub(crate) mod tests { #[test_case(Ty::Tuple(vec![Ty::IntLiteral(1)]), Ty::Tuple(vec![Ty::IntLiteral(2)]))] #[test_case(Ty::Tuple(vec![Ty::IntLiteral(1), Ty::IntLiteral(2)]), Ty::Tuple(vec![Ty::IntLiteral(1)]))] #[test_case(Ty::Tuple(vec![Ty::IntLiteral(1), Ty::IntLiteral(2)]), Ty::Tuple(vec![Ty::IntLiteral(1), Ty::IntLiteral(3)]))] + #[test_case(Ty::Tuple(vec![]), Ty::BuiltinClassLiteral("object"))] fn is_disjoint_from(a: Ty, b: Ty) { let db = setup_db(); let a = a.into_type(&db); From b87bc2d6b1131ac07acd13cb94f2f82ac71e217b Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 13 Dec 2024 19:26:39 +0000 Subject: [PATCH 8/8] reduce Salsa lookups in `Tuple` branch --- crates/red_knot_python_semantic/src/types.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 2bdf55df08e96..4d9ea40ab3bef 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -1201,19 +1201,17 @@ impl<'db> Type<'db> { } (Type::Tuple(tuple), Type::Tuple(other_tuple)) => { - if tuple.len(db) == other_tuple.len(db) { - tuple - .elements(db) + let self_elements = tuple.elements(db); + let other_elements = other_tuple.elements(db); + self_elements.len() != other_elements.len() + || self_elements .iter() - .zip(other_tuple.elements(db)) + .zip(other_elements) .any(|(e1, e2)| e1.is_disjoint_from(db, *e2)) - } else { - true - } } (Type::Tuple(..), Type::Instance(..)) | (Type::Instance(..), Type::Tuple(..)) => { - // We can not be sure if the tuple is disjoint from the instance because: + // We cannot be sure if the tuple is disjoint from the instance because: // - 'other' might be the homogeneous arbitrary-length tuple type // tuple[T, ...] (which we don't have support for yet); if all of // our element types are not disjoint with T, this is not disjoint