Skip to content

Commit 01b67f4

Browse files
committed
coverage: Simplify sorting of coverage spans extracted from MIR
Switching to `Ordering::then_with` makes control-flow less complicated, and there is no need to use `partial_cmp` here.
1 parent a4cb31b commit 01b67f4

File tree

3 files changed

+19
-36
lines changed

3 files changed

+19
-36
lines changed

compiler/rustc_data_structures/src/graph/dominators/mod.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ pub fn dominators<G: ControlFlowGraph>(graph: &G) -> Dominators<G::Node> {
5151
// Traverse the graph, collecting a number of things:
5252
//
5353
// * Preorder mapping (to it, and back to the actual ordering)
54-
// * Postorder mapping (used exclusively for rank_partial_cmp on the final product)
54+
// * Postorder mapping (used exclusively for `cmp_in_dominator_order` on the final product)
5555
// * Parents for each vertex in the preorder tree
5656
//
5757
// These are all done here rather than through one of the 'standard'
@@ -342,8 +342,8 @@ impl<Node: Idx> Dominators<Node> {
342342
/// relationship, the dominator will always precede the dominated. (The relative ordering
343343
/// of two unrelated nodes will also be consistent, but otherwise the order has no
344344
/// meaning.) This method cannot be used to determine if either Node dominates the other.
345-
pub fn rank_partial_cmp(&self, lhs: Node, rhs: Node) -> Option<Ordering> {
346-
self.post_order_rank[rhs].partial_cmp(&self.post_order_rank[lhs])
345+
pub fn cmp_in_dominator_order(&self, lhs: Node, rhs: Node) -> Ordering {
346+
self.post_order_rank[rhs].cmp(&self.post_order_rank[lhs])
347347
}
348348

349349
/// Returns true if `a` dominates `b`.

compiler/rustc_mir_transform/src/coverage/graph.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,8 @@ impl CoverageGraph {
199199
}
200200

201201
#[inline(always)]
202-
pub fn rank_partial_cmp(
203-
&self,
204-
a: BasicCoverageBlock,
205-
b: BasicCoverageBlock,
206-
) -> Option<Ordering> {
207-
self.dominators.as_ref().unwrap().rank_partial_cmp(a, b)
202+
pub fn cmp_in_dominator_order(&self, a: BasicCoverageBlock, b: BasicCoverageBlock) -> Ordering {
203+
self.dominators.as_ref().unwrap().cmp_in_dominator_order(a, b)
208204
}
209205
}
210206

compiler/rustc_mir_transform/src/coverage/spans.rs

+14-27
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use rustc_span::source_map::original_sp;
1212
use rustc_span::{BytePos, ExpnKind, MacroKind, Span, Symbol};
1313

1414
use std::cell::OnceCell;
15-
use std::cmp::Ordering;
1615

1716
#[derive(Debug, Copy, Clone)]
1817
pub(super) enum CoverageStatement {
@@ -334,32 +333,20 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
334333
initial_spans.push(CoverageSpan::for_fn_sig(self.fn_sig_span));
335334

336335
initial_spans.sort_by(|a, b| {
337-
if a.span.lo() == b.span.lo() {
338-
if a.span.hi() == b.span.hi() {
339-
if a.is_in_same_bcb(b) {
340-
Some(Ordering::Equal)
341-
} else {
342-
// Sort equal spans by dominator relationship (so dominators always come
343-
// before the dominated equal spans). When later comparing two spans in
344-
// order, the first will either dominate the second, or they will have no
345-
// dominator relationship.
346-
self.basic_coverage_blocks.rank_partial_cmp(a.bcb, b.bcb)
347-
}
348-
} else {
349-
// Sort hi() in reverse order so shorter spans are attempted after longer spans.
350-
// This guarantees that, if a `prev` span overlaps, and is not equal to, a
351-
// `curr` span, the prev span either extends further left of the curr span, or
352-
// they start at the same position and the prev span extends further right of
353-
// the end of the curr span.
354-
b.span.hi().partial_cmp(&a.span.hi())
355-
}
356-
} else {
357-
a.span.lo().partial_cmp(&b.span.lo())
358-
}
359-
.unwrap()
360-
// If two spans are otherwise identical, put closure spans first,
361-
// as this seems to be what the refinement step expects.
362-
.then_with(|| Ord::cmp(&a.is_closure, &b.is_closure).reverse())
336+
// First sort by span start.
337+
Ord::cmp(&a.span.lo(), &b.span.lo())
338+
// If span starts are the same, sort by span end in reverse order.
339+
// This ensures that if spans A and B are adjacent in the list,
340+
// and they overlap but are not equal, then either:
341+
// - Span A extends further left, or
342+
// - Both have the same start and span A extends further right
343+
.then_with(|| Ord::cmp(&a.span.hi(), &b.span.hi()).reverse())
344+
// If both spans are equal, sort the BCBs in dominator order,
345+
// so that dominating BCBs come before other BCBs they dominate.
346+
.then_with(|| self.basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb))
347+
// If two spans are otherwise identical, put closure spans first,
348+
// as this seems to be what the refinement step expects.
349+
.then_with(|| Ord::cmp(&a.is_closure, &b.is_closure).reverse())
363350
});
364351

365352
initial_spans

0 commit comments

Comments
 (0)