Skip to content

Commit 11d5614

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of rust-lang#130013 - jonathan-conder:await_coverage, r=Zalathar
coverage: Count await when the Future is immediately ready Currently `await` is only counted towards coverage if the containing function is suspended and resumed at least once. This is because it expands to code which contains a branch on the discriminant of `Poll`. By treating it like a branching macro (e.g. `assert!`), these implementation details will be hidden from the coverage results. I added a test to ensure the fix works in simple cases, but the heuristic of picking only the first await-related covspan might be unreliable. I plan on testing more thoroughly with a real codebase over the next couple of weeks. closes rust-lang#98712
2 parents 6841e35 + 25d1830 commit 11d5614

File tree

8 files changed

+160
-63
lines changed

8 files changed

+160
-63
lines changed

Diff for: ‎compiler/rustc_mir_transform/src/coverage/spans.rs

+21-13
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::collections::VecDeque;
33
use rustc_data_structures::captures::Captures;
44
use rustc_data_structures::fx::FxHashSet;
55
use rustc_middle::mir;
6-
use rustc_span::Span;
6+
use rustc_span::{DesugaringKind, ExpnKind, MacroKind, Span};
77
use tracing::{debug, debug_span, instrument};
88

99
use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph};
@@ -25,7 +25,7 @@ pub(super) fn extract_refined_covspans(
2525

2626
// First, perform the passes that need macro information.
2727
covspans.sort_by(|a, b| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb));
28-
remove_unwanted_macro_spans(&mut covspans);
28+
remove_unwanted_expansion_spans(&mut covspans);
2929
split_visible_macro_spans(&mut covspans);
3030

3131
// We no longer need the extra information in `SpanFromMir`, so convert to `Covspan`.
@@ -76,18 +76,24 @@ pub(super) fn extract_refined_covspans(
7676
/// invocation, which is unhelpful. Keeping only the first such span seems to
7777
/// give better mappings, so remove the others.
7878
///
79+
/// Similarly, `await` expands to a branch on the discriminant of `Poll`, which
80+
/// leads to incorrect coverage if the `Future` is immediately ready (#98712).
81+
///
7982
/// (The input spans should be sorted in BCB dominator order, so that the
8083
/// retained "first" span is likely to dominate the others.)
81-
fn remove_unwanted_macro_spans(covspans: &mut Vec<SpanFromMir>) {
82-
let mut seen_macro_spans = FxHashSet::default();
84+
fn remove_unwanted_expansion_spans(covspans: &mut Vec<SpanFromMir>) {
85+
let mut deduplicated_spans = FxHashSet::default();
86+
8387
covspans.retain(|covspan| {
84-
// Ignore (retain) non-macro-expansion spans.
85-
if covspan.visible_macro.is_none() {
86-
return true;
88+
match covspan.expn_kind {
89+
// Retain only the first await-related or macro-expanded covspan with this span.
90+
Some(ExpnKind::Desugaring(kind)) if kind == DesugaringKind::Await => {
91+
deduplicated_spans.insert(covspan.span)
92+
}
93+
Some(ExpnKind::Macro(MacroKind::Bang, _)) => deduplicated_spans.insert(covspan.span),
94+
// Ignore (retain) other spans.
95+
_ => true,
8796
}
88-
89-
// Retain only the first macro-expanded covspan with this span.
90-
seen_macro_spans.insert(covspan.span)
9197
});
9298
}
9399

@@ -99,7 +105,9 @@ fn split_visible_macro_spans(covspans: &mut Vec<SpanFromMir>) {
99105
let mut extra_spans = vec![];
100106

101107
covspans.retain(|covspan| {
102-
let Some(visible_macro) = covspan.visible_macro else { return true };
108+
let Some(ExpnKind::Macro(MacroKind::Bang, visible_macro)) = covspan.expn_kind else {
109+
return true;
110+
};
103111

104112
let split_len = visible_macro.as_str().len() as u32 + 1;
105113
let (before, after) = covspan.span.split_at(split_len);
@@ -111,8 +119,8 @@ fn split_visible_macro_spans(covspans: &mut Vec<SpanFromMir>) {
111119
return true;
112120
}
113121

114-
extra_spans.push(SpanFromMir::new(before, covspan.visible_macro, covspan.bcb));
115-
extra_spans.push(SpanFromMir::new(after, covspan.visible_macro, covspan.bcb));
122+
extra_spans.push(SpanFromMir::new(before, covspan.expn_kind.clone(), covspan.bcb));
123+
extra_spans.push(SpanFromMir::new(after, covspan.expn_kind.clone(), covspan.bcb));
116124
false // Discard the original covspan that we just split.
117125
});
118126

Diff for: ‎compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs

+11-11
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@ use rustc_middle::mir::coverage::CoverageKind;
33
use rustc_middle::mir::{
44
self, FakeReadCause, Statement, StatementKind, Terminator, TerminatorKind,
55
};
6-
use rustc_span::{Span, Symbol};
6+
use rustc_span::{ExpnKind, Span};
77

88
use crate::coverage::graph::{
99
BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph, START_BCB,
1010
};
1111
use crate::coverage::spans::Covspan;
12-
use crate::coverage::unexpand::unexpand_into_body_span_with_visible_macro;
12+
use crate::coverage::unexpand::unexpand_into_body_span_with_expn_kind;
1313
use crate::coverage::ExtractedHirInfo;
1414

1515
pub(crate) struct ExtractedCovspans {
@@ -60,17 +60,17 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>(
6060
let data = &mir_body[bb];
6161

6262
let unexpand = move |expn_span| {
63-
unexpand_into_body_span_with_visible_macro(expn_span, body_span)
63+
unexpand_into_body_span_with_expn_kind(expn_span, body_span)
6464
// Discard any spans that fill the entire body, because they tend
6565
// to represent compiler-inserted code, e.g. implicitly returning `()`.
6666
.filter(|(span, _)| !span.source_equal(body_span))
6767
};
6868

6969
let mut extract_statement_span = |statement| {
7070
let expn_span = filtered_statement_span(statement)?;
71-
let (span, visible_macro) = unexpand(expn_span)?;
71+
let (span, expn_kind) = unexpand(expn_span)?;
7272

73-
initial_covspans.push(SpanFromMir::new(span, visible_macro, bcb));
73+
initial_covspans.push(SpanFromMir::new(span, expn_kind, bcb));
7474
Some(())
7575
};
7676
for statement in data.statements.iter() {
@@ -79,9 +79,9 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>(
7979

8080
let mut extract_terminator_span = |terminator| {
8181
let expn_span = filtered_terminator_span(terminator)?;
82-
let (span, visible_macro) = unexpand(expn_span)?;
82+
let (span, expn_kind) = unexpand(expn_span)?;
8383

84-
initial_covspans.push(SpanFromMir::new(span, visible_macro, bcb));
84+
initial_covspans.push(SpanFromMir::new(span, expn_kind, bcb));
8585
Some(())
8686
};
8787
extract_terminator_span(data.terminator());
@@ -214,7 +214,7 @@ pub(crate) struct SpanFromMir {
214214
/// With the exception of `fn_sig_span`, this should always be contained
215215
/// within `body_span`.
216216
pub(crate) span: Span,
217-
pub(crate) visible_macro: Option<Symbol>,
217+
pub(crate) expn_kind: Option<ExpnKind>,
218218
pub(crate) bcb: BasicCoverageBlock,
219219
}
220220

@@ -223,12 +223,12 @@ impl SpanFromMir {
223223
Self::new(fn_sig_span, None, START_BCB)
224224
}
225225

226-
pub(crate) fn new(span: Span, visible_macro: Option<Symbol>, bcb: BasicCoverageBlock) -> Self {
227-
Self { span, visible_macro, bcb }
226+
pub(crate) fn new(span: Span, expn_kind: Option<ExpnKind>, bcb: BasicCoverageBlock) -> Self {
227+
Self { span, expn_kind, bcb }
228228
}
229229

230230
pub(crate) fn into_covspan(self) -> Covspan {
231-
let Self { span, visible_macro: _, bcb } = self;
231+
let Self { span, expn_kind: _, bcb } = self;
232232
Covspan { span, bcb }
233233
}
234234
}

Diff for: ‎compiler/rustc_mir_transform/src/coverage/unexpand.rs

+5-10
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use rustc_span::{ExpnKind, MacroKind, Span, Symbol};
1+
use rustc_span::{ExpnKind, Span};
22

33
/// Walks through the expansion ancestors of `original_span` to find a span that
44
/// is contained in `body_span` and has the same [syntax context] as `body_span`.
@@ -13,20 +13,15 @@ pub(crate) fn unexpand_into_body_span(original_span: Span, body_span: Span) -> O
1313
///
1414
/// If the returned span represents a bang-macro invocation (e.g. `foo!(..)`),
1515
/// the returned symbol will be the name of that macro (e.g. `foo`).
16-
pub(crate) fn unexpand_into_body_span_with_visible_macro(
16+
pub(crate) fn unexpand_into_body_span_with_expn_kind(
1717
original_span: Span,
1818
body_span: Span,
19-
) -> Option<(Span, Option<Symbol>)> {
19+
) -> Option<(Span, Option<ExpnKind>)> {
2020
let (span, prev) = unexpand_into_body_span_with_prev(original_span, body_span)?;
2121

22-
let visible_macro = prev
23-
.map(|prev| match prev.ctxt().outer_expn_data().kind {
24-
ExpnKind::Macro(MacroKind::Bang, name) => Some(name),
25-
_ => None,
26-
})
27-
.flatten();
22+
let expn_kind = prev.map(|prev| prev.ctxt().outer_expn_data().kind);
2823

29-
Some((span, visible_macro))
24+
Some((span, expn_kind))
3025
}
3126

3227
/// Walks through the expansion ancestors of `original_span` to find a span that

Diff for: ‎tests/coverage/async.cov-map

+21-27
Original file line numberDiff line numberDiff line change
@@ -92,20 +92,18 @@ Number of file 0 mappings: 1
9292
- Code(Counter(0)) at (prev + 25, 1) to (start + 0, 23)
9393

9494
Function name: async::g::{closure#0} (unused)
95-
Raw bytes (69): 0x[01, 01, 00, 0d, 00, 19, 17, 01, 0c, 00, 02, 09, 00, 0a, 00, 00, 0e, 00, 11, 00, 00, 12, 00, 17, 00, 00, 1b, 00, 1c, 00, 00, 20, 00, 22, 00, 01, 09, 00, 0a, 00, 00, 0e, 00, 11, 00, 00, 12, 00, 17, 00, 00, 1b, 00, 1c, 00, 00, 20, 00, 22, 00, 01, 0e, 00, 10, 00, 02, 01, 00, 02]
95+
Raw bytes (59): 0x[01, 01, 00, 0b, 00, 19, 17, 01, 0c, 00, 02, 09, 00, 0a, 00, 00, 0e, 00, 17, 00, 00, 1b, 00, 1c, 00, 00, 20, 00, 22, 00, 01, 09, 00, 0a, 00, 00, 0e, 00, 17, 00, 00, 1b, 00, 1c, 00, 00, 20, 00, 22, 00, 01, 0e, 00, 10, 00, 02, 01, 00, 02]
9696
Number of files: 1
9797
- file 0 => global file 1
9898
Number of expressions: 0
99-
Number of file 0 mappings: 13
99+
Number of file 0 mappings: 11
100100
- Code(Zero) at (prev + 25, 23) to (start + 1, 12)
101101
- Code(Zero) at (prev + 2, 9) to (start + 0, 10)
102-
- Code(Zero) at (prev + 0, 14) to (start + 0, 17)
103-
- Code(Zero) at (prev + 0, 18) to (start + 0, 23)
102+
- Code(Zero) at (prev + 0, 14) to (start + 0, 23)
104103
- Code(Zero) at (prev + 0, 27) to (start + 0, 28)
105104
- Code(Zero) at (prev + 0, 32) to (start + 0, 34)
106105
- Code(Zero) at (prev + 1, 9) to (start + 0, 10)
107-
- Code(Zero) at (prev + 0, 14) to (start + 0, 17)
108-
- Code(Zero) at (prev + 0, 18) to (start + 0, 23)
106+
- Code(Zero) at (prev + 0, 14) to (start + 0, 23)
109107
- Code(Zero) at (prev + 0, 27) to (start + 0, 28)
110108
- Code(Zero) at (prev + 0, 32) to (start + 0, 34)
111109
- Code(Zero) at (prev + 1, 14) to (start + 0, 16)
@@ -120,15 +118,14 @@ Number of file 0 mappings: 1
120118
- Code(Counter(0)) at (prev + 33, 1) to (start + 0, 22)
121119

122120
Function name: async::h::{closure#0} (unused)
123-
Raw bytes (44): 0x[01, 01, 00, 08, 00, 21, 16, 03, 0c, 00, 04, 09, 00, 0a, 00, 00, 0e, 00, 13, 00, 00, 14, 00, 19, 00, 00, 1a, 00, 1b, 00, 00, 20, 00, 22, 00, 01, 0e, 00, 10, 00, 02, 01, 00, 02]
121+
Raw bytes (39): 0x[01, 01, 00, 07, 00, 21, 16, 03, 0c, 00, 04, 09, 00, 0a, 00, 00, 0e, 00, 19, 00, 00, 1a, 00, 1b, 00, 00, 20, 00, 22, 00, 01, 0e, 00, 10, 00, 02, 01, 00, 02]
124122
Number of files: 1
125123
- file 0 => global file 1
126124
Number of expressions: 0
127-
Number of file 0 mappings: 8
125+
Number of file 0 mappings: 7
128126
- Code(Zero) at (prev + 33, 22) to (start + 3, 12)
129127
- Code(Zero) at (prev + 4, 9) to (start + 0, 10)
130-
- Code(Zero) at (prev + 0, 14) to (start + 0, 19)
131-
- Code(Zero) at (prev + 0, 20) to (start + 0, 25)
128+
- Code(Zero) at (prev + 0, 14) to (start + 0, 25)
132129
- Code(Zero) at (prev + 0, 26) to (start + 0, 27)
133130
- Code(Zero) at (prev + 0, 32) to (start + 0, 34)
134131
- Code(Zero) at (prev + 1, 14) to (start + 0, 16)
@@ -143,28 +140,25 @@ Number of file 0 mappings: 1
143140
- Code(Counter(0)) at (prev + 42, 1) to (start + 0, 19)
144141

145142
Function name: async::i::{closure#0}
146-
Raw bytes (78): 0x[01, 01, 02, 07, 21, 19, 1d, 0e, 01, 2a, 13, 04, 0c, 0d, 05, 09, 00, 0a, 01, 00, 0e, 00, 12, 05, 00, 13, 00, 18, 09, 00, 1c, 00, 21, 0d, 00, 27, 00, 2a, 15, 00, 2b, 00, 30, 1d, 01, 09, 00, 0a, 11, 00, 0e, 00, 11, 25, 00, 12, 00, 17, 29, 00, 1b, 00, 20, 1d, 00, 24, 00, 26, 21, 01, 0e, 00, 10, 03, 02, 01, 00, 02]
143+
Raw bytes (63): 0x[01, 01, 02, 07, 19, 11, 15, 0b, 01, 2a, 13, 04, 0c, 09, 05, 09, 00, 0a, 01, 00, 0e, 00, 18, 05, 00, 1c, 00, 21, 09, 00, 27, 00, 30, 15, 01, 09, 00, 0a, 0d, 00, 0e, 00, 17, 1d, 00, 1b, 00, 20, 15, 00, 24, 00, 26, 19, 01, 0e, 00, 10, 03, 02, 01, 00, 02]
147144
Number of files: 1
148145
- file 0 => global file 1
149146
Number of expressions: 2
150-
- expression 0 operands: lhs = Expression(1, Add), rhs = Counter(8)
151-
- expression 1 operands: lhs = Counter(6), rhs = Counter(7)
152-
Number of file 0 mappings: 14
147+
- expression 0 operands: lhs = Expression(1, Add), rhs = Counter(6)
148+
- expression 1 operands: lhs = Counter(4), rhs = Counter(5)
149+
Number of file 0 mappings: 11
153150
- Code(Counter(0)) at (prev + 42, 19) to (start + 4, 12)
154-
- Code(Counter(3)) at (prev + 5, 9) to (start + 0, 10)
155-
- Code(Counter(0)) at (prev + 0, 14) to (start + 0, 18)
156-
- Code(Counter(1)) at (prev + 0, 19) to (start + 0, 24)
157-
- Code(Counter(2)) at (prev + 0, 28) to (start + 0, 33)
158-
- Code(Counter(3)) at (prev + 0, 39) to (start + 0, 42)
159-
- Code(Counter(5)) at (prev + 0, 43) to (start + 0, 48)
160-
- Code(Counter(7)) at (prev + 1, 9) to (start + 0, 10)
161-
- Code(Counter(4)) at (prev + 0, 14) to (start + 0, 17)
162-
- Code(Counter(9)) at (prev + 0, 18) to (start + 0, 23)
163-
- Code(Counter(10)) at (prev + 0, 27) to (start + 0, 32)
164-
- Code(Counter(7)) at (prev + 0, 36) to (start + 0, 38)
165-
- Code(Counter(8)) at (prev + 1, 14) to (start + 0, 16)
151+
- Code(Counter(2)) at (prev + 5, 9) to (start + 0, 10)
152+
- Code(Counter(0)) at (prev + 0, 14) to (start + 0, 24)
153+
- Code(Counter(1)) at (prev + 0, 28) to (start + 0, 33)
154+
- Code(Counter(2)) at (prev + 0, 39) to (start + 0, 48)
155+
- Code(Counter(5)) at (prev + 1, 9) to (start + 0, 10)
156+
- Code(Counter(3)) at (prev + 0, 14) to (start + 0, 23)
157+
- Code(Counter(7)) at (prev + 0, 27) to (start + 0, 32)
158+
- Code(Counter(5)) at (prev + 0, 36) to (start + 0, 38)
159+
- Code(Counter(6)) at (prev + 1, 14) to (start + 0, 16)
166160
- Code(Expression(0, Add)) at (prev + 2, 1) to (start + 0, 2)
167-
= ((c6 + c7) + c8)
161+
= ((c4 + c5) + c6)
168162

169163
Function name: async::j
170164
Raw bytes (58): 0x[01, 01, 02, 07, 0d, 05, 09, 0a, 01, 35, 01, 00, 0d, 01, 0b, 0b, 00, 0c, 05, 01, 09, 00, 0a, 01, 00, 0e, 00, 1b, 05, 00, 1f, 00, 27, 09, 01, 09, 00, 0a, 11, 00, 0e, 00, 1a, 09, 00, 1e, 00, 20, 0d, 01, 0e, 00, 10, 03, 02, 01, 00, 02]

Diff for: ‎tests/coverage/async.coverage

+2-2
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@
4545
LL| 1| // executed asynchronously.
4646
LL| 1| match x {
4747
LL| 1| y if c(x).await == y + 1 => { d().await; }
48-
^0 ^0 ^0 ^0
48+
^0 ^0
4949
LL| 1| y if f().await == y + 1 => (),
50-
^0 ^0 ^0
50+
^0 ^0
5151
LL| 1| _ => (),
5252
LL| | }
5353
LL| 1|}

Diff for: ‎tests/coverage/await_ready.cov-map

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
Function name: await_ready::await_ready
2+
Raw bytes (9): 0x[01, 01, 00, 01, 01, 0a, 01, 00, 1e]
3+
Number of files: 1
4+
- file 0 => global file 1
5+
Number of expressions: 0
6+
Number of file 0 mappings: 1
7+
- Code(Counter(0)) at (prev + 10, 1) to (start + 0, 30)
8+
9+
Function name: await_ready::await_ready::{closure#0}
10+
Raw bytes (14): 0x[01, 01, 00, 02, 01, 0a, 1e, 03, 0f, 05, 04, 01, 00, 02]
11+
Number of files: 1
12+
- file 0 => global file 1
13+
Number of expressions: 0
14+
Number of file 0 mappings: 2
15+
- Code(Counter(0)) at (prev + 10, 30) to (start + 3, 15)
16+
- Code(Counter(1)) at (prev + 4, 1) to (start + 0, 2)
17+
18+
Function name: await_ready::main
19+
Raw bytes (9): 0x[01, 01, 00, 01, 01, 10, 01, 03, 02]
20+
Number of files: 1
21+
- file 0 => global file 1
22+
Number of expressions: 0
23+
Number of file 0 mappings: 1
24+
- Code(Counter(0)) at (prev + 16, 1) to (start + 3, 2)
25+

Diff for: ‎tests/coverage/await_ready.coverage

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
LL| |#![feature(coverage_attribute)]
2+
LL| |#![feature(custom_inner_attributes)] // for #![rustfmt::skip]
3+
LL| |#![feature(noop_waker)]
4+
LL| |#![rustfmt::skip]
5+
LL| |//@ edition: 2021
6+
LL| |
7+
LL| |#[coverage(off)]
8+
LL| |async fn ready() -> u8 { 1 }
9+
LL| |
10+
LL| 1|async fn await_ready() -> u8 {
11+
LL| 1| // await should be covered even if the function never yields
12+
LL| 1| ready()
13+
LL| 1| .await
14+
LL| 1|}
15+
LL| |
16+
LL| 1|fn main() {
17+
LL| 1| let mut future = Box::pin(await_ready());
18+
LL| 1| executor::block_on(future.as_mut());
19+
LL| 1|}
20+
LL| |
21+
LL| |mod executor {
22+
LL| | use core::future::Future;
23+
LL| | use core::pin::pin;
24+
LL| | use core::task::{Context, Poll, Waker};
25+
LL| |
26+
LL| | #[coverage(off)]
27+
LL| | pub fn block_on<F: Future>(mut future: F) -> F::Output {
28+
LL| | let mut future = pin!(future);
29+
LL| | let mut context = Context::from_waker(Waker::noop());
30+
LL| |
31+
LL| | loop {
32+
LL| | if let Poll::Ready(val) = future.as_mut().poll(&mut context) {
33+
LL| | break val;
34+
LL| | }
35+
LL| | }
36+
LL| | }
37+
LL| |}
38+

Diff for: ‎tests/coverage/await_ready.rs

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
#![feature(coverage_attribute)]
2+
#![feature(custom_inner_attributes)] // for #![rustfmt::skip]
3+
#![feature(noop_waker)]
4+
#![rustfmt::skip]
5+
//@ edition: 2021
6+
7+
#[coverage(off)]
8+
async fn ready() -> u8 { 1 }
9+
10+
async fn await_ready() -> u8 {
11+
// await should be covered even if the function never yields
12+
ready()
13+
.await
14+
}
15+
16+
fn main() {
17+
let mut future = Box::pin(await_ready());
18+
executor::block_on(future.as_mut());
19+
}
20+
21+
mod executor {
22+
use core::future::Future;
23+
use core::pin::pin;
24+
use core::task::{Context, Poll, Waker};
25+
26+
#[coverage(off)]
27+
pub fn block_on<F: Future>(mut future: F) -> F::Output {
28+
let mut future = pin!(future);
29+
let mut context = Context::from_waker(Waker::noop());
30+
31+
loop {
32+
if let Poll::Ready(val) = future.as_mut().poll(&mut context) {
33+
break val;
34+
}
35+
}
36+
}
37+
}

0 commit comments

Comments
 (0)