Skip to content

Commit 84d064a

Browse files
authored
[red-knot] fix building unions with literals and AlwaysTruthy/AlwaysFalsy (#17451)
In #17403 I added a comment asserting that all same-kind literal types share all the same super-types. This is true, with two notable exceptions: the types `AlwaysTruthy` and `AlwaysFalsy`. These two types are super-types of some literal types within a given kind and not others: `Literal[0]`, `Literal[""]`, and `Literal[b""]` inhabit `AlwaysFalsy`, while other literals inhabit `AlwaysTruthy`. This PR updates the literal-unions optimization to handle these types correctly. Fixes #17447 Verified locally that `QUICKCHECK_TESTS=100000 cargo test -p red_knot_python_semantic -- --ignored types::property_tests::stable` now passes again.
1 parent e4e405d commit 84d064a

File tree

2 files changed

+118
-24
lines changed

2 files changed

+118
-24
lines changed

crates/red_knot_python_semantic/resources/mdtest/union_types.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,3 +166,46 @@ def _(
166166
reveal_type(i1) # revealed: P & Q
167167
reveal_type(i2) # revealed: P & Q
168168
```
169+
170+
## Unions of literals with `AlwaysTruthy` and `AlwaysFalsy`
171+
172+
```py
173+
from typing import Literal
174+
from knot_extensions import AlwaysTruthy, AlwaysFalsy
175+
176+
type strings = Literal["foo", ""]
177+
type ints = Literal[0, 1]
178+
type bytes = Literal[b"foo", b""]
179+
180+
def _(
181+
strings_or_truthy: strings | AlwaysTruthy,
182+
truthy_or_strings: AlwaysTruthy | strings,
183+
strings_or_falsy: strings | AlwaysFalsy,
184+
falsy_or_strings: AlwaysFalsy | strings,
185+
ints_or_truthy: ints | AlwaysTruthy,
186+
truthy_or_ints: AlwaysTruthy | ints,
187+
ints_or_falsy: ints | AlwaysFalsy,
188+
falsy_or_ints: AlwaysFalsy | ints,
189+
bytes_or_truthy: bytes | AlwaysTruthy,
190+
truthy_or_bytes: AlwaysTruthy | bytes,
191+
bytes_or_falsy: bytes | AlwaysFalsy,
192+
falsy_or_bytes: AlwaysFalsy | bytes,
193+
):
194+
reveal_type(strings_or_truthy) # revealed: Literal[""] | AlwaysTruthy
195+
reveal_type(truthy_or_strings) # revealed: AlwaysTruthy | Literal[""]
196+
197+
reveal_type(strings_or_falsy) # revealed: Literal["foo"] | AlwaysFalsy
198+
reveal_type(falsy_or_strings) # revealed: AlwaysFalsy | Literal["foo"]
199+
200+
reveal_type(ints_or_truthy) # revealed: Literal[0] | AlwaysTruthy
201+
reveal_type(truthy_or_ints) # revealed: AlwaysTruthy | Literal[0]
202+
203+
reveal_type(ints_or_falsy) # revealed: Literal[1] | AlwaysFalsy
204+
reveal_type(falsy_or_ints) # revealed: AlwaysFalsy | Literal[1]
205+
206+
reveal_type(bytes_or_truthy) # revealed: Literal[b""] | AlwaysTruthy
207+
reveal_type(truthy_or_bytes) # revealed: AlwaysTruthy | Literal[b""]
208+
209+
reveal_type(bytes_or_falsy) # revealed: Literal[b"foo"] | AlwaysFalsy
210+
reveal_type(falsy_or_bytes) # revealed: AlwaysFalsy | Literal[b"foo"]
211+
```

crates/red_knot_python_semantic/src/types/builder.rs

Lines changed: 75 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,67 @@ enum UnionElement<'db> {
5151
Type(Type<'db>),
5252
}
5353

54+
impl<'db> UnionElement<'db> {
55+
/// Try reducing this `UnionElement` given the presence in the same union of `other_type`.
56+
///
57+
/// If this `UnionElement` is a group of literals, filter the literals present if needed and
58+
/// return `ReduceResult::KeepIf` with a boolean value indicating whether the remaining group
59+
/// of literals should be kept in the union
60+
///
61+
/// If this `UnionElement` is some other type, return `ReduceResult::Type` so `UnionBuilder`
62+
/// can perform more complex checks on it.
63+
fn try_reduce(&mut self, db: &'db dyn Db, other_type: Type<'db>) -> ReduceResult<'db> {
64+
// `AlwaysTruthy` and `AlwaysFalsy` are the only types which can be a supertype of only
65+
// _some_ literals of the same kind, so we need to walk the full set in this case.
66+
let needs_filter = matches!(other_type, Type::AlwaysTruthy | Type::AlwaysFalsy);
67+
match self {
68+
UnionElement::IntLiterals(literals) => {
69+
ReduceResult::KeepIf(if needs_filter {
70+
literals.retain(|literal| {
71+
!Type::IntLiteral(*literal).is_subtype_of(db, other_type)
72+
});
73+
!literals.is_empty()
74+
} else {
75+
// SAFETY: All `UnionElement` literal kinds must always be non-empty
76+
!Type::IntLiteral(literals[0]).is_subtype_of(db, other_type)
77+
})
78+
}
79+
UnionElement::StringLiterals(literals) => {
80+
ReduceResult::KeepIf(if needs_filter {
81+
literals.retain(|literal| {
82+
!Type::StringLiteral(*literal).is_subtype_of(db, other_type)
83+
});
84+
!literals.is_empty()
85+
} else {
86+
// SAFETY: All `UnionElement` literal kinds must always be non-empty
87+
!Type::StringLiteral(literals[0]).is_subtype_of(db, other_type)
88+
})
89+
}
90+
UnionElement::BytesLiterals(literals) => {
91+
ReduceResult::KeepIf(if needs_filter {
92+
literals.retain(|literal| {
93+
!Type::BytesLiteral(*literal).is_subtype_of(db, other_type)
94+
});
95+
!literals.is_empty()
96+
} else {
97+
// SAFETY: All `UnionElement` literal kinds must always be non-empty
98+
!Type::BytesLiteral(literals[0]).is_subtype_of(db, other_type)
99+
})
100+
}
101+
UnionElement::Type(existing) => ReduceResult::Type(*existing),
102+
}
103+
}
104+
}
105+
106+
enum ReduceResult<'db> {
107+
/// Reduction of this `UnionElement` is complete; keep it in the union if the nested
108+
/// boolean is true, eliminate it from the union if false.
109+
KeepIf(bool),
110+
/// The given `Type` can stand-in for the entire `UnionElement` for further union
111+
/// simplification checks.
112+
Type(Type<'db>),
113+
}
114+
54115
// TODO increase this once we extend `UnionElement` throughout all union/intersection
55116
// representations, so that we can make large unions of literals fast in all operations.
56117
const MAX_UNION_LITERALS: usize = 200;
@@ -197,27 +258,17 @@ impl<'db> UnionBuilder<'db> {
197258
let mut to_remove = SmallVec::<[usize; 2]>::new();
198259
let ty_negated = ty.negate(self.db);
199260

200-
for (index, element) in self
201-
.elements
202-
.iter()
203-
.map(|element| {
204-
// For literals, the first element in the set can stand in for all the rest,
205-
// since they all have the same super-types. SAFETY: a `UnionElement` of
206-
// literal kind must always have at least one element in it.
207-
match element {
208-
UnionElement::IntLiterals(literals) => Type::IntLiteral(literals[0]),
209-
UnionElement::StringLiterals(literals) => {
210-
Type::StringLiteral(literals[0])
261+
for (index, element) in self.elements.iter_mut().enumerate() {
262+
let element_type = match element.try_reduce(self.db, ty) {
263+
ReduceResult::KeepIf(keep) => {
264+
if !keep {
265+
to_remove.push(index);
211266
}
212-
UnionElement::BytesLiterals(literals) => {
213-
Type::BytesLiteral(literals[0])
214-
}
215-
UnionElement::Type(ty) => *ty,
267+
continue;
216268
}
217-
})
218-
.enumerate()
219-
{
220-
if Some(element) == bool_pair {
269+
ReduceResult::Type(ty) => ty,
270+
};
271+
if Some(element_type) == bool_pair {
221272
to_add = KnownClass::Bool.to_instance(self.db);
222273
to_remove.push(index);
223274
// The type we are adding is a BooleanLiteral, which doesn't have any
@@ -227,14 +278,14 @@ impl<'db> UnionBuilder<'db> {
227278
break;
228279
}
229280

230-
if ty.is_same_gradual_form(element)
231-
|| ty.is_subtype_of(self.db, element)
232-
|| element.is_object(self.db)
281+
if ty.is_same_gradual_form(element_type)
282+
|| ty.is_subtype_of(self.db, element_type)
283+
|| element_type.is_object(self.db)
233284
{
234285
return;
235-
} else if element.is_subtype_of(self.db, ty) {
286+
} else if element_type.is_subtype_of(self.db, ty) {
236287
to_remove.push(index);
237-
} else if ty_negated.is_subtype_of(self.db, element) {
288+
} else if ty_negated.is_subtype_of(self.db, element_type) {
238289
// We add `ty` to the union. We just checked that `~ty` is a subtype of an existing `element`.
239290
// This also means that `~ty | ty` is a subtype of `element | ty`, because both elements in the
240291
// first union are subtypes of the corresponding elements in the second union. But `~ty | ty` is

0 commit comments

Comments
 (0)