Skip to content
Merged
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -424,9 +424,26 @@ impl ReachabilityConstraintsBuilder {
}
}

/// Returns whether `a` or `b` has a "larger" atom. TDDs are ordered such that interior nodes
/// can only have edges to "larger" nodes. Terminals are considered to have a larger atom than
/// any internal node, since they are leaf nodes.
/// Implements the ordering that determines which level a TDD node appears at.
///
/// Each interior node checks the value of a single variable (for us, a `Predicate`).
/// TDDs are ordered such that every path from the root of the graph to the leaves must
/// check each variable at most once, and must check each variable in the same order.
///
/// We can choose any ordering that we want, as long as it's consistent — with the
/// caveat that terminal nodes must always be last in the ordering, since they are the
/// leaf nodes of the graph.
///
/// We currently compare interior nodes by looking at the Salsa IDs of each variable's
/// `Predicate`, since this is already available and easy to compare. We also _reverse_
/// the comparison of those Salsa IDs. The Salsa IDs are assigned roughly sequentially
/// while traversing the source code. Reversing the comparison means `Predicate`s that
/// appear later in the source will tend to be placed "higher" (closer to the root) in
/// the TDD graph. We have found empirically that this leads to smaller TDD graphs [1],
/// since there are often repeated combinations of `Predicate`s from earlier in the
/// file.
///
/// [1]: https://github.com/astral-sh/ruff/pull/20098
fn cmp_atoms(
&self,
a: ScopedReachabilityConstraintId,
Expand All @@ -439,7 +456,12 @@ impl ReachabilityConstraintsBuilder {
} else if b.is_terminal() {
Ordering::Less
} else {
self.interiors[a].atom.cmp(&self.interiors[b].atom)
// See https://github.com/astral-sh/ruff/pull/20098 for an explanation of why this
// ordering is reversed.
self.interiors[a]
.atom
.cmp(&self.interiors[b].atom)
.reverse()
}
}

Expand Down
Loading