Skip to content

Commit 4690f97

Browse files
ZalatharZalathar
committed
coverage: Fix an unstable-sort inconsistency in coverage spans
This code was calling `sort_unstable_by`, but failed to impose a total order on the initial spans. That resulted in unpredictable handling of closure spans, producing inconsistencies in the coverage maps and in user-visible coverage reports. This patch fixes the problem by always sorting closure spans before otherwise-identical non-closure spans, and also switches to a stable sort in case the ordering is still not total.
1 parent 078eb11 commit 4690f97

File tree

4 files changed

+20
-17
lines changed

4 files changed

+20
-17
lines changed

‎compiler/rustc_mir_transform/src/coverage/spans.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
333333

334334
initial_spans.push(CoverageSpan::for_fn_sig(self.fn_sig_span));
335335

336-
initial_spans.sort_unstable_by(|a, b| {
336+
initial_spans.sort_by(|a, b| {
337337
if a.span.lo() == b.span.lo() {
338338
if a.span.hi() == b.span.hi() {
339339
if a.is_in_same_bcb(b) {
@@ -357,6 +357,9 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
357357
a.span.lo().partial_cmp(&b.span.lo())
358358
}
359359
.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())
360363
});
361364

362365
initial_spans

‎tests/coverage-map/status-quo/closure.cov-map

+7-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
Function name: closure::main
2-
Raw bytes (170): 0x[01, 01, 17, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 05, 05, 5a, 01, 05, 18, 01, 08, 01, 0f, 0d, 03, 16, 0e, 06, 0a, 07, 10, 05, 13, 0d, 0b, 1a, 0e, 08, 09, 0f, 10, 05, 0e, 09, 13, 16, 05, 0d, 18, 17, 19, 09, 01, 21, 1b, 04, 09, 00, 29, 1f, 01, 09, 00, 2d, 23, 01, 09, 00, 24, 27, 05, 09, 00, 24, 2b, 02, 09, 00, 21, 2f, 04, 09, 00, 21, 33, 04, 09, 00, 28, 37, 09, 09, 00, 32, 3b, 04, 09, 00, 33, 3f, 07, 09, 00, 4b, 43, 08, 09, 01, 09, 47, 0a, 09, 01, 09, 4b, 08, 09, 01, 09, 4f, 0a, 08, 00, 10, 05, 00, 11, 04, 06, 5a, 04, 06, 00, 07, 57, 01, 05, 03, 02]
2+
Raw bytes (170): 0x[01, 01, 17, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 00, 01, 05, 05, 5a, 01, 05, 18, 01, 08, 01, 0f, 0d, 03, 16, 0e, 06, 0a, 07, 10, 05, 13, 0d, 0b, 1a, 0e, 06, 0a, 0f, 10, 05, 0c, 16, 13, 16, 05, 0d, 18, 17, 19, 09, 01, 1e, 1b, 04, 09, 00, 29, 1f, 01, 09, 00, 2d, 23, 01, 09, 00, 24, 27, 05, 09, 00, 24, 2b, 02, 09, 00, 21, 2f, 04, 09, 00, 21, 33, 04, 09, 00, 28, 37, 09, 09, 00, 32, 3b, 04, 09, 00, 33, 3f, 07, 09, 00, 4b, 43, 08, 09, 00, 48, 47, 0a, 09, 00, 47, 4b, 08, 09, 00, 44, 4f, 0a, 08, 00, 10, 05, 00, 11, 04, 06, 5a, 04, 06, 00, 07, 57, 01, 05, 03, 02]
33
Number of files: 1
44
- file 0 => global file 1
55
Number of expressions: 23
@@ -32,13 +32,13 @@ Number of file 0 mappings: 24
3232
= (c0 + Zero)
3333
- Code(Expression(1, Add)) at (prev + 16, 5) to (start + 19, 13)
3434
= (c0 + Zero)
35-
- Code(Expression(2, Add)) at (prev + 26, 14) to (start + 8, 9)
35+
- Code(Expression(2, Add)) at (prev + 26, 14) to (start + 6, 10)
3636
= (c0 + Zero)
37-
- Code(Expression(3, Add)) at (prev + 16, 5) to (start + 14, 9)
37+
- Code(Expression(3, Add)) at (prev + 16, 5) to (start + 12, 22)
3838
= (c0 + Zero)
3939
- Code(Expression(4, Add)) at (prev + 22, 5) to (start + 13, 24)
4040
= (c0 + Zero)
41-
- Code(Expression(5, Add)) at (prev + 25, 9) to (start + 1, 33)
41+
- Code(Expression(5, Add)) at (prev + 25, 9) to (start + 1, 30)
4242
= (c0 + Zero)
4343
- Code(Expression(6, Add)) at (prev + 4, 9) to (start + 0, 41)
4444
= (c0 + Zero)
@@ -60,11 +60,11 @@ Number of file 0 mappings: 24
6060
= (c0 + Zero)
6161
- Code(Expression(15, Add)) at (prev + 7, 9) to (start + 0, 75)
6262
= (c0 + Zero)
63-
- Code(Expression(16, Add)) at (prev + 8, 9) to (start + 1, 9)
63+
- Code(Expression(16, Add)) at (prev + 8, 9) to (start + 0, 72)
6464
= (c0 + Zero)
65-
- Code(Expression(17, Add)) at (prev + 10, 9) to (start + 1, 9)
65+
- Code(Expression(17, Add)) at (prev + 10, 9) to (start + 0, 71)
6666
= (c0 + Zero)
67-
- Code(Expression(18, Add)) at (prev + 8, 9) to (start + 1, 9)
67+
- Code(Expression(18, Add)) at (prev + 8, 9) to (start + 0, 68)
6868
= (c0 + Zero)
6969
- Code(Expression(19, Add)) at (prev + 10, 8) to (start + 0, 16)
7070
= (c0 + Zero)

‎tests/coverage-map/status-quo/generator.cov-map

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ Number of file 0 mappings: 4
1414
= (c1 + (c0 - c1))
1515

1616
Function name: generator::main
17-
Raw bytes (71): 0x[01, 01, 0b, 01, 00, 05, 0b, 09, 0d, 11, 00, 11, 15, 2a, 19, 11, 15, 15, 19, 26, 00, 2a, 19, 11, 15, 09, 01, 0f, 01, 02, 19, 03, 07, 0b, 00, 2e, 11, 01, 2b, 00, 2d, 07, 01, 0e, 00, 35, 0f, 02, 0b, 00, 2e, 2a, 01, 22, 00, 27, 26, 00, 2c, 00, 2e, 1f, 01, 0e, 00, 35, 23, 02, 01, 00, 02]
17+
Raw bytes (71): 0x[01, 01, 0b, 01, 00, 05, 0b, 09, 0d, 11, 00, 11, 15, 2a, 19, 11, 15, 15, 19, 26, 00, 2a, 19, 11, 15, 09, 01, 0f, 01, 02, 16, 03, 07, 0b, 00, 2e, 11, 01, 2b, 00, 2d, 07, 01, 0e, 00, 35, 0f, 02, 0b, 00, 2e, 2a, 01, 22, 00, 27, 26, 00, 2c, 00, 2e, 1f, 01, 0e, 00, 35, 23, 02, 01, 00, 02]
1818
Number of files: 1
1919
- file 0 => global file 1
2020
Number of expressions: 11
@@ -30,7 +30,7 @@ Number of expressions: 11
3030
- expression 9 operands: lhs = Expression(10, Sub), rhs = Counter(6)
3131
- expression 10 operands: lhs = Counter(4), rhs = Counter(5)
3232
Number of file 0 mappings: 9
33-
- Code(Counter(0)) at (prev + 15, 1) to (start + 2, 25)
33+
- Code(Counter(0)) at (prev + 15, 1) to (start + 2, 22)
3434
- Code(Expression(0, Add)) at (prev + 7, 11) to (start + 0, 46)
3535
= (c0 + Zero)
3636
- Code(Counter(4)) at (prev + 1, 43) to (start + 0, 45)

‎tests/run-coverage/closure.coverage

+7-7
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@
7676
LL| 1| some_string = None;
7777
LL| 1| let
7878
LL| 1| a
79-
LL| 1| =
80-
LL| 1| ||
79+
LL| | =
80+
LL| | ||
8181
LL| 1| {
8282
LL| 1| let mut countdown = 0;
8383
LL| 1| if is_false {
@@ -98,8 +98,8 @@
9898
LL| 1|
9999
LL| 1| let
100100
LL| 1| quote_closure
101-
LL| 1| =
102-
LL| 1| |val|
101+
LL| | =
102+
LL| | |val|
103103
LL| 5| {
104104
LL| 5| let mut countdown = 0;
105105
LL| 5| if is_false {
@@ -186,7 +186,7 @@
186186
LL| | ;
187187
LL| |
188188
LL| 1| let short_used_not_covered_closure_line_break_block_embedded_branch =
189-
LL| 1| | _unused_arg: u8 |
189+
LL| | | _unused_arg: u8 |
190190
LL| 0| {
191191
LL| 0| println!(
192192
LL| 0| "not called: {}",
@@ -196,7 +196,7 @@
196196
LL| | ;
197197
LL| |
198198
LL| 1| let short_used_covered_closure_line_break_no_block_embedded_branch =
199-
LL| 1| | _unused_arg: u8 |
199+
LL| | | _unused_arg: u8 |
200200
LL| 1| println!(
201201
LL| 1| "not called: {}",
202202
LL| 1| if is_true { "check" } else { "me" }
@@ -205,7 +205,7 @@
205205
LL| | ;
206206
LL| |
207207
LL| 1| let short_used_covered_closure_line_break_block_embedded_branch =
208-
LL| 1| | _unused_arg: u8 |
208+
LL| | | _unused_arg: u8 |
209209
LL| 1| {
210210
LL| 1| println!(
211211
LL| 1| "not called: {}",

0 commit comments

Comments
 (0)