Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[red-knot] Distribute intersections on negation #13962

Merged
merged 5 commits into from
Oct 29, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 50 additions & 6 deletions crates/red_knot_python_semantic/src/types/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,12 @@ impl<'db> IntersectionBuilder<'db> {
}
self
} else if let Type::Intersection(intersection) = ty {
// (A | B) & !(C & !D) -> (A | B) & (!C | D)
// -> ((A | B) & !C) | ((A | B) & D)
//
// (A | B) & ~(C & ~D)
// -> (A | B) & (~C | D)
// -> ((A | B) & ~C) | ((A | B) & D)
// i.e. if we have an intersection of positive constraints C
// and negative constraints D, then our new intersection
// is (existing & !C) | (existing & D)
// is (existing & ~C) | (existing & D)

let positive_side = intersection
.positive(self.db)
Expand Down Expand Up @@ -643,17 +643,61 @@ mod tests {
assert_eq!(sh_t.pos_vec(&db), &[st, ht]);
assert_eq!(sh_t.neg_vec(&db), &[]);

// !sh_t => not Sized or not Hashable
// ~sh_t => ~Sized | ~Hashable
let not_s_h_t = IntersectionBuilder::new(&db)
.add_negative(Type::Intersection(sh_t))
.build()
.expect_union();

// should have (not Sized) or (not Hashable)
// should have as elements: (~Sized),(~Hashable)
let not_st = st.negate(&db);
let not_ht = ht.negate(&db);
assert_eq!(not_s_h_t.elements(&db), &[not_st, not_ht]);
}

#[test]
fn mixed_intersection_negation_distributes_over_union() {
let db = setup_db();
let it = KnownClass::Int.to_instance(&db);
let st = KnownClass::Sized.to_instance(&db);
let ht = KnownClass::Hashable.to_instance(&db);
// sh_t: Sized & ~Hashable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// sh_t: Sized & ~Hashable
// s_not_h_t: Sized & ~Hashable

let s_not_h_t = IntersectionBuilder::new(&db)
.add_positive(st)
.add_negative(ht)
.build()
.expect_intersection();
assert_eq!(s_not_h_t.pos_vec(&db), &[st]);
assert_eq!(s_not_h_t.neg_vec(&db), &[ht]);

// let's build Int & ~(Sized & ~Hashable)
let tt = IntersectionBuilder::new(&db)
.add_positive(it)
.add_negative(Type::Intersection(s_not_h_t))
.build();

// Int & ~(Sized & ~Hashable)
// -> Int & (~Sized | Hashable)
// -> (Int & ~Sized) | (Int & Hashable)
let tt = tt.expect_union();

assert_eq!(
tt.elements(&db),
&[
// Int & ~Sized
IntersectionBuilder::new(&db)
.add_positive(it)
.add_negative(st)
.build(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a part of me wants something like Type::intersection(elts: Vec<Type<'db>>) for testing but I got pretty used to writing out the intersection builder at this point 😆

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, you aren't the first to say this after working on IntersectionBuilder, cc @sharkdp 😆 It's not a problem at all to add test helpers to make tests less verbose, someone just needs to feel the pain sufficiently to actually do it!

// Int & Hashable
IntersectionBuilder::new(&db)
.add_positive(it)
.add_positive(ht)
.build(),
]
);
}

#[test]
fn build_intersection_self_negation() {
let db = setup_db();
Expand Down
Loading