Skip to content

Commit 45842cc

Browse files
authored
[ty] Fix non-determinism in ConstraintSet.specialize_constrained (#21744)
This fixes a non-determinism that we were seeing in the constraint set tests in #21715. In this test, we create the following constraint set, and then try to create a specialization from it: ``` (T@constrained_by_gradual_list = list[Base]) ∨ (Bottom[list[Any]] ≤ T@constrained_by_gradual_list ≤ Top[list[Any]]) ``` That is, `T` is either specifically `list[Base]`, or it's any `list`. Our current heuristics say that, absent other restrictions, we should specialize `T` to the more specific type (`list[Base]`). In the correct test output, we end up creating a BDD that looks like this: ``` (T@constrained_by_gradual_list = list[Base]) ┡━₁ always └─₀ (Bottom[list[Any]] ≤ T@constrained_by_gradual_list ≤ Top[list[Any]]) ┡━₁ always └─₀ never ``` In the incorrect output, the BDD looks like this: ``` (Bottom[list[Any]] ≤ T@constrained_by_gradual_list ≤ Top[list[Any]]) ┡━₁ always └─₀ never ``` The difference is the ordering of the two individual constraints. Both constraints appear in the first BDD, but the second BDD only contains `T is any list`. If we were to force the second BDD to contain both constraints, it would look like this: ``` (Bottom[list[Any]] ≤ T@constrained_by_gradual_list ≤ Top[list[Any]]) ┡━₁ always └─₀ (T@constrained_by_gradual_list = list[Base]) ┡━₁ always └─₀ never ``` This is the standard shape for an OR of two constraints. However! Those two constraints are not independent of each other! If `T` is specifically `list[Base]`, then it's definitely also "any `list`". From that, we can infer the contrapositive: that if `T` is not any list, then it cannot be `list[Base]` specifically. When we encounter impossible situations like that, we prune that path in the BDD, and treat it as `false`. That rewrites the second BDD to the following: ``` (Bottom[list[Any]] ≤ T@constrained_by_gradual_list ≤ Top[list[Any]]) ┡━₁ always └─₀ (T@constrained_by_gradual_list = list[Base]) ┡━₁ never <-- IMPOSSIBLE, rewritten to never └─₀ never ``` We then would see that that BDD node is redundant, since both of its outgoing edges point at the `never` node. Our BDDs are _reduced_, which means we have to remove that redundant node, resulting in the BDD we saw above: ``` (Bottom[list[Any]] ≤ T@constrained_by_gradual_list ≤ Top[list[Any]]) ┡━₁ always └─₀ never <-- redundant node removed ``` The end result is that we were "forgetting" about the `T = list[Base]` constraint, but only for some BDD variable orderings. To fix this, I'm leaning in to the fact that our BDDs really do need to "remember" all of the constraints that they were created with. Some combinations might not be possible, but we now have the sequent map, which is quite good at detecting and pruning those. So now our BDDs are _quasi-reduced_, which just means that redundant nodes are allowed. (At first I was worried that allowing redundant nodes would be an unsound "fix the glitch". But it turns out they're real! [This](https://ieeexplore.ieee.org/abstract/document/130209) is the paper that introduces them, though it's very difficult to read. Knuth mentions them in §7.1.4 of [TAOCP](https://course.khoury.northeastern.edu/csu690/ssl/bdd-knuth.pdf), and [this paper](https://par.nsf.gov/servlets/purl/10128966) has a nice short summary of them in §2.) While we're here, I've added a bunch of `debug` and `trace` level log messages to the constraint set implementation. I was getting tired of having to add these by hands over and over. To enable them, just set `TY_LOG` in your environment, e.g. ```sh env TY_LOG=ty_python_semantic::types::constraints::SequentMap=trace ty check ... ``` [Note, this has an `internal` label because are still not using `specialize_constrained` in anything user-facing yet.]
1 parent cd079bd commit 45842cc

File tree

2 files changed

+300
-53
lines changed

2 files changed

+300
-53
lines changed

crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ def unbounded[T]():
4141
# revealed: None
4242
reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(bool, T, bool) & ConstraintSet.range(Never, T, str)))
4343

44-
# revealed: ty_extensions.Specialization[T@unbounded = int]
44+
# TODO: revealed: ty_extensions.Specialization[T@unbounded = int]
45+
# revealed: ty_extensions.Specialization[T@unbounded = bool]
4546
reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(Never, T, int) | ConstraintSet.range(Never, T, bool)))
4647
# revealed: ty_extensions.Specialization[T@unbounded = Never]
4748
reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(Never, T, int) | ConstraintSet.range(Never, T, str)))
@@ -175,7 +176,7 @@ def constrained_by_gradual[T: (Base, Any)]():
175176
# revealed: ty_extensions.Specialization[T@constrained_by_gradual = Base]
176177
reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.always()))
177178
# TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = Any]
178-
# revealed: ty_extensions.Specialization[T@constrained_by_gradual = object]
179+
# revealed: ty_extensions.Specialization[T@constrained_by_gradual = Base]
179180
reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.range(Never, T, object)))
180181
# revealed: None
181182
reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.never()))
@@ -251,6 +252,30 @@ def constrained_by_gradual_list[T: (list[Base], list[Any])]():
251252
# revealed: ty_extensions.Specialization[T@constrained_by_gradual_list = list[Sub]]
252253
reveal_type(generic_context(constrained_by_gradual_list).specialize_constrained(ConstraintSet.range(list[Sub], T, list[Sub])))
253254

255+
# Same tests as above, but with the typevar constraints in a different order, to make sure the
256+
# results do not depend on our BDD variable ordering.
257+
def constrained_by_gradual_list_reverse[T: (list[Any], list[Base])]():
258+
# revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base]]
259+
reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.always()))
260+
# revealed: None
261+
reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.never()))
262+
263+
# revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base]]
264+
reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[Base])))
265+
# TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = list[Any]]
266+
# revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Unrelated]]
267+
reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[Unrelated])))
268+
269+
# TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = list[Any]]
270+
# revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Super]]
271+
reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[Super])))
272+
# TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = list[Any]]
273+
# revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Super]]
274+
reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(list[Super], T, list[Super])))
275+
# TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = list[Any]]
276+
# revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Sub]]
277+
reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(list[Sub], T, list[Sub])))
278+
254279
def constrained_by_two_gradual_lists[T: (list[Any], list[Any])]():
255280
# TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = list[Any]]
256281
# revealed: ty_extensions.Specialization[T@constrained_by_two_gradual_lists = Top[list[Any]]]

0 commit comments

Comments
 (0)