[ty] Consistent ordering of constraint set specializations, take 2#21983
Merged
[ty] Consistent ordering of constraint set specializations, take 2#21983
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
dcreager
force-pushed
the
dcreager/source-order-constraints
branch
from
466fd1c to
cba45ac
Compare
carljm
approved these changes
Dec 15, 2025
Contributor
carljm
left a comment
carljm
left a comment
There was a problem hiding this comment.
Looks good! It's too bad how much extra work we have to do (rebuilding entire BDD subtrees) in order to preserve this ordering information, but at the moment I'm not seeing a better way.
Member
Author
Hmm, that's a good thought. For expediency I'm just going to add a TODO, but I wonder if we can store the |
dcreager
added a commit
that referenced
this pull request
Dec 19, 2025
We're seeing a lot of nondeterminism in the ecosystem tests at the moment, which started (or at least got worse) once `Callable` inference landed. This PR attempts to remove this nondeterminism. We recently (#21983) added a `source_order` field to BDD nodes, which tracks when their constraint was added to the BDD. Since we build up constraints based on the order that they appear in the underlying source, that gives us a stable ordering even though we use an arbitrary salsa-derived ordering for the BDD variables. The issue (at least for some of the flakiness) is that we add "derived" constraints when walking a BDD tree, and those derived constraints inherit or borrow the `source_order` of the "real" constraint that implied them. That means we can get multiple constraints in our specialization that all have the same `source_order`. If we're not careful, those "tied" constraints can be ordered arbitrarily. The fix requires ~three~ ~four~ several steps: - When starting to construct a sequent map (the data structure that stores the derived constraints), we first sort all of the "real" constraints by their `source_order`. That ensures that we insert things into the sequent map in a stable order. - During sequent map construction, derived facts are discovered by a deterministic process applied to constraints in a (now) stable order. So derived facts are now also inserted in a stable order. - We update the fields of `SequentMap` to use `FxOrderSet` instead of `FxHashSet`, so that we retain that stable insertion order. - When walking BDD paths when constructing a specialization, we were already sorting the constraints by their `source_order`. However, we were not considering that we might get derived constraints, and therefore constraints with "ties". Because of that, we need to make sure to use a _stable_ sort, that retains the insertion order for those ties. All together, this...should...fix the nondeterminism. (Unfortunately, I haven't been able to effectively test this, since I haven't been able to coerce local tests to flop into the other order that we sometimes see in CI.)
This was referenced Feb 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In #21957, we tried to use
union_or_intersection_elements_orderingto provide a stable ordering of the union and intersection elements that are created when determining which type a typevar should specialize to. @AlexWaygood pointed out that this won't work, since that provides a consistent ordering within a single process run, but does not provide a stable ordering across runs.This is an attempt to produce a proper stable ordering for constraint sets, so that we end up with consistent diagnostic and test output.
We do this by maintaining a new
source_orderfield on each interior BDD node, which records when that node's constraint was added to the set. Several of the BDD operators (and,or, etc) now have_with_offsetvariants, which update eachsource_orderin the rhs to be larger than any of thesource_orders in the lhs. This is what causes that field to be in line with (a) when you add each constraint to the set, and (b) the order of the parameters you provide toand,or, etc. Then we sort by that new field before constructing the union/intersection types when creating a specialization.