Skip to content

Commit ac6219e

Browse files
carljmcarljm
authored
[red-knot] fix collapsing literal and its negation to object (#17605)
## Summary Another follow-up to the unions-of-large-literals optimization. Restore the behavior that e.g. `Literal[""] | ~Literal[""]` collapses to `object`. ## Test Plan Added mdtests.
1 parent e93fa70 commit ac6219e

File tree

2 files changed

+125
-44
lines changed

2 files changed

+125
-44
lines changed

‎crates/red_knot_python_semantic/resources/mdtest/call/union.md

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -165,26 +165,39 @@ def _(flag: bool):
165165
## Unions with literals and negations
166166

167167
```py
168-
from typing import Literal, Union
168+
from typing import Literal
169169
from knot_extensions import Not, AlwaysFalsy, static_assert, is_subtype_of, is_assignable_to
170170

171-
static_assert(is_subtype_of(Literal["a", ""], Union[Literal["a", ""], Not[AlwaysFalsy]]))
172-
static_assert(is_subtype_of(Not[AlwaysFalsy], Union[Literal["", "a"], Not[AlwaysFalsy]]))
173-
static_assert(is_subtype_of(Literal["a", ""], Union[Not[AlwaysFalsy], Literal["a", ""]]))
174-
static_assert(is_subtype_of(Not[AlwaysFalsy], Union[Not[AlwaysFalsy], Literal["a", ""]]))
171+
static_assert(is_subtype_of(Literal["a", ""], Literal["a", ""] | Not[AlwaysFalsy]))
172+
static_assert(is_subtype_of(Not[AlwaysFalsy], Literal["", "a"] | Not[AlwaysFalsy]))
173+
static_assert(is_subtype_of(Literal["a", ""], Not[AlwaysFalsy] | Literal["a", ""]))
174+
static_assert(is_subtype_of(Not[AlwaysFalsy], Not[AlwaysFalsy] | Literal["a", ""]))
175175

176-
static_assert(is_subtype_of(Literal["a", ""], Union[Literal["a", ""], Not[Literal[""]]]))
177-
static_assert(is_subtype_of(Not[Literal[""]], Union[Literal["a", ""], Not[Literal[""]]]))
178-
static_assert(is_subtype_of(Literal["a", ""], Union[Not[Literal[""]], Literal["a", ""]]))
179-
static_assert(is_subtype_of(Not[Literal[""]], Union[Not[Literal[""]], Literal["a", ""]]))
176+
static_assert(is_subtype_of(Literal["a", ""], Literal["a", ""] | Not[Literal[""]]))
177+
static_assert(is_subtype_of(Not[Literal[""]], Literal["a", ""] | Not[Literal[""]]))
178+
static_assert(is_subtype_of(Literal["a", ""], Not[Literal[""]] | Literal["a", ""]))
179+
static_assert(is_subtype_of(Not[Literal[""]], Not[Literal[""]] | Literal["a", ""]))
180180

181181
def _(
182-
x: Union[Literal["a", ""], Not[AlwaysFalsy]],
183-
y: Union[Literal["a", ""], Not[Literal[""]]],
182+
a: Literal["a", ""] | Not[AlwaysFalsy],
183+
b: Literal["a", ""] | Not[Literal[""]],
184+
c: Literal[""] | Not[Literal[""]],
185+
d: Not[Literal[""]] | Literal[""],
186+
e: Literal["a"] | Not[Literal["a"]],
187+
f: Literal[b"b"] | Not[Literal[b"b"]],
188+
g: Not[Literal[b"b"]] | Literal[b"b"],
189+
h: Literal[42] | Not[Literal[42]],
190+
i: Not[Literal[42]] | Literal[42],
184191
):
185-
reveal_type(x) # revealed: Literal[""] | ~AlwaysFalsy
186-
# TODO should be `object`
187-
reveal_type(y) # revealed: Literal[""] | ~Literal[""]
192+
reveal_type(a) # revealed: Literal[""] | ~AlwaysFalsy
193+
reveal_type(b) # revealed: object
194+
reveal_type(c) # revealed: object
195+
reveal_type(d) # revealed: object
196+
reveal_type(e) # revealed: object
197+
reveal_type(f) # revealed: object
198+
reveal_type(g) # revealed: object
199+
reveal_type(h) # revealed: object
200+
reveal_type(i) # revealed: object
188201
```
189202

190203
## Cannot use an argument as both a value and a type form

‎crates/red_knot_python_semantic/src/types/builder.rs

Lines changed: 98 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -97,37 +97,70 @@ impl<'db> UnionElement<'db> {
9797
fn try_reduce(&mut self, db: &'db dyn Db, other_type: Type<'db>) -> ReduceResult<'db> {
9898
match self {
9999
UnionElement::IntLiterals(literals) => {
100-
ReduceResult::KeepIf(if other_type.splits_literals(db, LiteralKind::Int) {
100+
if other_type.splits_literals(db, LiteralKind::Int) {
101+
let mut collapse = false;
102+
let negated = other_type.negate(db);
101103
literals.retain(|literal| {
102-
!Type::IntLiteral(*literal).is_subtype_of(db, other_type)
104+
let ty = Type::IntLiteral(*literal);
105+
if negated.is_subtype_of(db, ty) {
106+
collapse = true;
107+
}
108+
!ty.is_subtype_of(db, other_type)
103109
});
104-
!literals.is_empty()
110+
if collapse {
111+
ReduceResult::CollapseToObject
112+
} else {
113+
ReduceResult::KeepIf(!literals.is_empty())
114+
}
105115
} else {
106-
// SAFETY: All `UnionElement` literal kinds must always be non-empty
107-
!Type::IntLiteral(literals[0]).is_subtype_of(db, other_type)
108-
})
116+
ReduceResult::KeepIf(
117+
!Type::IntLiteral(literals[0]).is_subtype_of(db, other_type),
118+
)
119+
}
109120
}
110121
UnionElement::StringLiterals(literals) => {
111-
ReduceResult::KeepIf(if other_type.splits_literals(db, LiteralKind::String) {
122+
if other_type.splits_literals(db, LiteralKind::String) {
123+
let mut collapse = false;
124+
let negated = other_type.negate(db);
112125
literals.retain(|literal| {
113-
!Type::StringLiteral(*literal).is_subtype_of(db, other_type)
126+
let ty = Type::StringLiteral(*literal);
127+
if negated.is_subtype_of(db, ty) {
128+
collapse = true;
129+
}
130+
!ty.is_subtype_of(db, other_type)
114131
});
115-
!literals.is_empty()
132+
if collapse {
133+
ReduceResult::CollapseToObject
134+
} else {
135+
ReduceResult::KeepIf(!literals.is_empty())
136+
}
116137
} else {
117-
// SAFETY: All `UnionElement` literal kinds must always be non-empty
118-
!Type::StringLiteral(literals[0]).is_subtype_of(db, other_type)
119-
})
138+
ReduceResult::KeepIf(
139+
!Type::StringLiteral(literals[0]).is_subtype_of(db, other_type),
140+
)
141+
}
120142
}
121143
UnionElement::BytesLiterals(literals) => {
122-
ReduceResult::KeepIf(if other_type.splits_literals(db, LiteralKind::Bytes) {
144+
if other_type.splits_literals(db, LiteralKind::Bytes) {
145+
let mut collapse = false;
146+
let negated = other_type.negate(db);
123147
literals.retain(|literal| {
124-
!Type::BytesLiteral(*literal).is_subtype_of(db, other_type)
148+
let ty = Type::BytesLiteral(*literal);
149+
if negated.is_subtype_of(db, ty) {
150+
collapse = true;
151+
}
152+
!ty.is_subtype_of(db, other_type)
125153
});
126-
!literals.is_empty()
154+
if collapse {
155+
ReduceResult::CollapseToObject
156+
} else {
157+
ReduceResult::KeepIf(!literals.is_empty())
158+
}
127159
} else {
128-
// SAFETY: All `UnionElement` literal kinds must always be non-empty
129-
!Type::BytesLiteral(literals[0]).is_subtype_of(db, other_type)
130-
})
160+
ReduceResult::KeepIf(
161+
!Type::BytesLiteral(literals[0]).is_subtype_of(db, other_type),
162+
)
163+
}
131164
}
132165
UnionElement::Type(existing) => ReduceResult::Type(*existing),
133166
}
@@ -138,6 +171,8 @@ enum ReduceResult<'db> {
138171
/// Reduction of this `UnionElement` is complete; keep it in the union if the nested
139172
/// boolean is true, eliminate it from the union if false.
140173
KeepIf(bool),
174+
/// Collapse this entire union to `object`.
175+
CollapseToObject,
141176
/// The given `Type` can stand-in for the entire `UnionElement` for further union
142177
/// simplification checks.
143178
Type(Type<'db>),
@@ -195,6 +230,7 @@ impl<'db> UnionBuilder<'db> {
195230
// containing it.
196231
Type::StringLiteral(literal) => {
197232
let mut found = false;
233+
let ty_negated = ty.negate(self.db);
198234
for element in &mut self.elements {
199235
match element {
200236
UnionElement::StringLiterals(literals) => {
@@ -207,8 +243,16 @@ impl<'db> UnionBuilder<'db> {
207243
found = true;
208244
break;
209245
}
210-
UnionElement::Type(existing) if ty.is_subtype_of(self.db, *existing) => {
211-
return;
246+
UnionElement::Type(existing) => {
247+
if ty.is_subtype_of(self.db, *existing) {
248+
return;
249+
}
250+
if ty_negated.is_subtype_of(self.db, *existing) {
251+
// The type that includes both this new element, and its negation
252+
// (or a supertype of its negation), must be simply `object`.
253+
self.collapse_to_object();
254+
return;
255+
}
212256
}
213257
_ => {}
214258
}
@@ -223,6 +267,7 @@ impl<'db> UnionBuilder<'db> {
223267
// Same for bytes literals as for string literals, above.
224268
Type::BytesLiteral(literal) => {
225269
let mut found = false;
270+
let ty_negated = ty.negate(self.db);
226271
for element in &mut self.elements {
227272
match element {
228273
UnionElement::BytesLiterals(literals) => {
@@ -235,8 +280,16 @@ impl<'db> UnionBuilder<'db> {
235280
found = true;
236281
break;
237282
}
238-
UnionElement::Type(existing) if ty.is_subtype_of(self.db, *existing) => {
239-
return;
283+
UnionElement::Type(existing) => {
284+
if ty.is_subtype_of(self.db, *existing) {
285+
return;
286+
}
287+
if ty_negated.is_subtype_of(self.db, *existing) {
288+
// The type that includes both this new element, and its negation
289+
// (or a supertype of its negation), must be simply `object`.
290+
self.collapse_to_object();
291+
return;
292+
}
240293
}
241294
_ => {}
242295
}
@@ -251,6 +304,7 @@ impl<'db> UnionBuilder<'db> {
251304
// And same for int literals as well.
252305
Type::IntLiteral(literal) => {
253306
let mut found = false;
307+
let ty_negated = ty.negate(self.db);
254308
for element in &mut self.elements {
255309
match element {
256310
UnionElement::IntLiterals(literals) => {
@@ -263,8 +317,16 @@ impl<'db> UnionBuilder<'db> {
263317
found = true;
264318
break;
265319
}
266-
UnionElement::Type(existing) if ty.is_subtype_of(self.db, *existing) => {
267-
return;
320+
UnionElement::Type(existing) => {
321+
if ty.is_subtype_of(self.db, *existing) {
322+
return;
323+
}
324+
if ty_negated.is_subtype_of(self.db, *existing) {
325+
// The type that includes both this new element, and its negation
326+
// (or a supertype of its negation), must be simply `object`.
327+
self.collapse_to_object();
328+
return;
329+
}
268330
}
269331
_ => {}
270332
}
@@ -298,6 +360,10 @@ impl<'db> UnionBuilder<'db> {
298360
continue;
299361
}
300362
ReduceResult::Type(ty) => ty,
363+
ReduceResult::CollapseToObject => {
364+
self.collapse_to_object();
365+
return;
366+
}
301367
};
302368
if Some(element_type) == bool_pair {
303369
to_add = KnownClass::Bool.to_instance(self.db);
@@ -317,12 +383,14 @@ impl<'db> UnionBuilder<'db> {
317383
} else if element_type.is_subtype_of(self.db, ty) {
318384
to_remove.push(index);
319385
} else if ty_negated.is_subtype_of(self.db, element_type) {
320-
// We add `ty` to the union. We just checked that `~ty` is a subtype of an existing `element`.
321-
// This also means that `~ty | ty` is a subtype of `element | ty`, because both elements in the
322-
// first union are subtypes of the corresponding elements in the second union. But `~ty | ty` is
323-
// just `object`. Since `object` is a subtype of `element | ty`, we can only conclude that
324-
// `element | ty` must be `object` (object has no other supertypes). This means we can simplify
325-
// the whole union to just `object`, since all other potential elements would also be subtypes of
386+
// We add `ty` to the union. We just checked that `~ty` is a subtype of an
387+
// existing `element`. This also means that `~ty | ty` is a subtype of
388+
// `element | ty`, because both elements in the first union are subtypes of
389+
// the corresponding elements in the second union. But `~ty | ty` is just
390+
// `object`. Since `object` is a subtype of `element | ty`, we can only
391+
// conclude that `element | ty` must be `object` (object has no other
392+
// supertypes). This means we can simplify the whole union to just
393+
// `object`, since all other potential elements would also be subtypes of
326394
// `object`.
327395
self.collapse_to_object();
328396
return;

0 commit comments

Comments
 (0)