[red-knot] Decompose bool to Literal[True, False] in unions and intersections
#15738
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -811,6 +811,31 @@ impl<'db> Type<'db> { | |
| } | ||
| } | ||
|
|
||
| /// Normalize the type `bool` -> `Literal[True, False]`. | ||
| /// | ||
| /// Using this method in various type-relational methods | ||
| /// ensures that the following invariants hold true: | ||
| /// | ||
| /// - bool ≡ Literal[True, False] | ||
| /// - bool | T ≡ Literal[True, False] | T | ||
| /// - bool <: Literal[True, False] | ||
| /// - bool | T <: Literal[True, False] | T | ||
| /// - Literal[True, False] <: bool | ||
| /// - Literal[True, False] | T <: bool | T | ||
| #[must_use] | ||
| pub fn with_normalized_bools(self, db: &'db dyn Db) -> Self { | ||
| const LITERAL_BOOLS: [Type; 2] = [Type::BooleanLiteral(false), Type::BooleanLiteral(true)]; | ||
|
|
||
| match self { | ||
| Type::Instance(InstanceType { class }) if class.is_known(db, KnownClass::Bool) => { | ||
| Type::Union(UnionType::new(db, Box::from(LITERAL_BOOLS))) | ||
| } | ||
| // TODO: decompose `LiteralString` into `Literal[""] | TruthyLiteralString`? | ||
| // We'd need to rename this method... --Alex | ||
| _ => self, | ||
| } | ||
| } | ||
|
|
||
| /// Return a normalized version of `self` in which all unions and intersections are sorted | ||
| /// according to a canonical order, no matter how "deeply" a union/intersection may be nested. | ||
| #[must_use] | ||
|
|
@@ -859,6 +884,12 @@ impl<'db> Type<'db> { | |
| return true; | ||
| } | ||
|
|
||
| let normalized_self = self.with_normalized_bools(db); | ||
| let normalized_target = target.with_normalized_bools(db); | ||
| if normalized_self != self || normalized_target != target { | ||
| return normalized_self.is_subtype_of(db, normalized_target); | ||
| } | ||
|
|
||
| // 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 | ||
|
|
@@ -961,7 +992,7 @@ impl<'db> Type<'db> { | |
| KnownClass::Str.to_instance(db).is_subtype_of(db, target) | ||
| } | ||
| (Type::BooleanLiteral(_), _) => { | ||
| KnownClass::Int.to_instance(db).is_subtype_of(db, target) | ||
|
Comment on lines
-964
to
+995
There was a problem hiding this comment. I had a stack overflow for a looong time here before I realised I had to change this... was tearing my hair out 🙃 There was a problem hiding this comment. Can you say more why this is necessary? And maybe add a comment (unless I'm missing something obvious)? There was a problem hiding this comment. The first step in There was a problem hiding this comment. Thanks! I still don't understand why it's correct though? There are types which are supertypes of There was a problem hiding this comment.
It might not be -- I have been known to make mistakes 😜
Yeah, I think our set-theoretic types such as unions and intersections should be handled in the branches above? I just added more test cases demonstrating this. (I will add a comment to the code reflecting this conversation, but first I'm experimenting with alternative ways of structuring the code that might make this clearer in the first place!) There was a problem hiding this comment. (#15773 is the "alternative way of structuring the code", for anybody else reading this thread) |
||
| } | ||
| (Type::IntLiteral(_), _) => KnownClass::Int.to_instance(db).is_subtype_of(db, target), | ||
| (Type::BytesLiteral(_), _) => { | ||
|
|
@@ -1077,6 +1108,11 @@ impl<'db> Type<'db> { | |
| if self.is_gradual_equivalent_to(db, target) { | ||
| return true; | ||
| } | ||
| let normalized_self = self.with_normalized_bools(db); | ||
| let normalized_target = target.with_normalized_bools(db); | ||
| if normalized_self != self || normalized_target != target { | ||
| return normalized_self.is_assignable_to(db, normalized_target); | ||
| } | ||
| match (self, target) { | ||
| // Never can be assigned to any type. | ||
| (Type::Never, _) => true, | ||
|
|
@@ -1177,6 +1213,13 @@ impl<'db> Type<'db> { | |
| pub(crate) fn is_equivalent_to(self, db: &'db dyn Db, other: Type<'db>) -> bool { | ||
| // TODO equivalent but not identical types: TypedDicts, Protocols, type aliases, etc. | ||
|
|
||
| let normalized_self = self.with_normalized_bools(db); | ||
| let normalized_other = other.with_normalized_bools(db); | ||
|
|
||
| if normalized_self != self || normalized_other != other { | ||
| return normalized_self.is_equivalent_to(db, normalized_other); | ||
| } | ||
|
|
||
| match (self, other) { | ||
| (Type::Union(left), Type::Union(right)) => left.is_equivalent_to(db, right), | ||
| (Type::Intersection(left), Type::Intersection(right)) => { | ||
|
|
@@ -1218,6 +1261,13 @@ impl<'db> Type<'db> { | |
| /// | ||
| /// [Summary of type relations]: https://typing.readthedocs.io/en/latest/spec/concepts.html#summary-of-type-relations | ||
| pub(crate) fn is_gradual_equivalent_to(self, db: &'db dyn Db, other: Type<'db>) -> bool { | ||
| let normalized_self = self.with_normalized_bools(db); | ||
| let normalized_other = other.with_normalized_bools(db); | ||
|
|
||
| if normalized_self != self || normalized_other != other { | ||
| return normalized_self.is_gradual_equivalent_to(db, normalized_other); | ||
| } | ||
|
|
||
| if self == other { | ||
| return true; | ||
| } | ||
|
|
@@ -1250,6 +1300,12 @@ impl<'db> Type<'db> { | |
| /// Note: This function aims to have no false positives, but might return | ||
| /// wrong `false` answers in some cases. | ||
| pub(crate) fn is_disjoint_from(self, db: &'db dyn Db, other: Type<'db>) -> bool { | ||
| let normalized_self = self.with_normalized_bools(db); | ||
| let normalized_other = other.with_normalized_bools(db); | ||
| if normalized_self != self || normalized_other != other { | ||
| return normalized_self.is_disjoint_from(db, normalized_other); | ||
| } | ||
|
|
||
| match (self, other) { | ||
| (Type::Never, _) | (_, Type::Never) => true, | ||
|
|
||
|
|
@@ -4642,18 +4698,19 @@ pub struct TupleType<'db> { | |
| } | ||
|
|
||
| impl<'db> TupleType<'db> { | ||
| pub fn from_elements<I, T>(db: &'db dyn Db, types: I) -> Type<'db> | ||
| where | ||
| I: IntoIterator<Item = T>, | ||
| T: Into<Type<'db>>, | ||
| { | ||
| let mut elements = vec![]; | ||
|
|
||
| for ty in types { | ||
| let ty: Type<'db> = ty.into(); | ||
| if ty.is_never() { | ||
| return Type::Never; | ||
| } | ||
| elements.push(ty.with_normalized_bools(db)); | ||
| } | ||
|
|
||
| Type::Tuple(Self::new(db, elements.into_boxed_slice())) | ||
|
|
||
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.
I changed this because it doesn't look like we've implemented comparisons between unions inside tuples yet, and
boolis now internally represented as a union, which meant that manyreveal_typeassertions in this test became TODOs rather thanbools. Testing whether we knew how to compare aboolwith abooldidn't seem to me to be the point of the test.