Skip to content

Commit b36cac0

Browse files
rust-timerrust-timer
authored
Unrolled build for rust-lang#120569
Rollup merge of rust-lang#120569 - Zalathar:fn-sig, r=oli-obk coverage: Improve handling of function/closure spans This is a combination of some loosely-related changes that touch the same code: 1. Make unexpansion of closure bodies more precise, by unexpanding back to the context of the closure declaration, instead of unexpanding all the way back to the top-level context. This preserves the way we handle async desugaring and closures containing a single bang-macro, while also giving better results for closures defined in macros. 2. Skip the normal span-refinement code when dealing with the trivial outer part of an async function. 3. Be more explicit about the fact that `fn_sig_span` has been extended to the start of the function body, and is not necessarily present. --- `@rustbot` label +A-code-coverage
2 parents 0984bec + a246b6b commit b36cac0

15 files changed

+274
-84
lines changed

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

+21-26
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,9 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
394394
struct ExtractedHirInfo {
395395
function_source_hash: u64,
396396
is_async_fn: bool,
397-
fn_sig_span: Span,
397+
/// The span of the function's signature, extended to the start of `body_span`.
398+
/// Must have the same context and filename as the body span.
399+
fn_sig_span_extended: Option<Span>,
398400
body_span: Span,
399401
}
400402

@@ -407,13 +409,25 @@ fn extract_hir_info<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> ExtractedHir
407409
hir::map::associated_body(hir_node).expect("HIR node is a function with body");
408410
let hir_body = tcx.hir().body(fn_body_id);
409411

410-
let is_async_fn = hir_node.fn_sig().is_some_and(|fn_sig| fn_sig.header.is_async());
411-
let body_span = get_body_span(tcx, hir_body, def_id);
412+
let maybe_fn_sig = hir_node.fn_sig();
413+
let is_async_fn = maybe_fn_sig.is_some_and(|fn_sig| fn_sig.header.is_async());
414+
415+
let mut body_span = hir_body.value.span;
416+
417+
use rustc_hir::{Closure, Expr, ExprKind, Node};
418+
// Unexpand a closure's body span back to the context of its declaration.
419+
// This helps with closure bodies that consist of just a single bang-macro,
420+
// and also with closure bodies produced by async desugaring.
421+
if let Node::Expr(&Expr { kind: ExprKind::Closure(&Closure { fn_decl_span, .. }), .. }) =
422+
hir_node
423+
{
424+
body_span = body_span.find_ancestor_in_same_ctxt(fn_decl_span).unwrap_or(body_span);
425+
}
412426

413427
// The actual signature span is only used if it has the same context and
414428
// filename as the body, and precedes the body.
415-
let maybe_fn_sig_span = hir_node.fn_sig().map(|fn_sig| fn_sig.span);
416-
let fn_sig_span = maybe_fn_sig_span
429+
let fn_sig_span_extended = maybe_fn_sig
430+
.map(|fn_sig| fn_sig.span)
417431
.filter(|&fn_sig_span| {
418432
let source_map = tcx.sess.source_map();
419433
let file_idx = |span: Span| source_map.lookup_source_file_idx(span.lo());
@@ -423,30 +437,11 @@ fn extract_hir_info<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> ExtractedHir
423437
&& file_idx(fn_sig_span) == file_idx(body_span)
424438
})
425439
// If so, extend it to the start of the body span.
426-
.map(|fn_sig_span| fn_sig_span.with_hi(body_span.lo()))
427-
// Otherwise, create a dummy signature span at the start of the body.
428-
.unwrap_or_else(|| body_span.shrink_to_lo());
440+
.map(|fn_sig_span| fn_sig_span.with_hi(body_span.lo()));
429441

430442
let function_source_hash = hash_mir_source(tcx, hir_body);
431443

432-
ExtractedHirInfo { function_source_hash, is_async_fn, fn_sig_span, body_span }
433-
}
434-
435-
fn get_body_span<'tcx>(
436-
tcx: TyCtxt<'tcx>,
437-
hir_body: &rustc_hir::Body<'tcx>,
438-
def_id: LocalDefId,
439-
) -> Span {
440-
let mut body_span = hir_body.value.span;
441-
442-
if tcx.is_closure_or_coroutine(def_id.to_def_id()) {
443-
// If the current function is a closure, and its "body" span was created
444-
// by macro expansion or compiler desugaring, try to walk backwards to
445-
// the pre-expansion call site or body.
446-
body_span = body_span.source_callsite();
447-
}
448-
449-
body_span
444+
ExtractedHirInfo { function_source_hash, is_async_fn, fn_sig_span_extended, body_span }
450445
}
451446

452447
fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 {

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

+21-8
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use rustc_index::bit_set::BitSet;
33
use rustc_middle::mir;
44
use rustc_span::{BytePos, Span, DUMMY_SP};
55

6-
use super::graph::{BasicCoverageBlock, CoverageGraph};
6+
use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph, START_BCB};
77
use crate::coverage::ExtractedHirInfo;
88

99
mod from_mir;
@@ -46,13 +46,26 @@ pub(super) fn generate_coverage_spans(
4646
) -> Option<CoverageSpans> {
4747
let mut mappings = vec![];
4848

49-
let sorted_spans =
50-
from_mir::mir_to_initial_sorted_coverage_spans(mir_body, hir_info, basic_coverage_blocks);
51-
let coverage_spans = SpansRefiner::refine_sorted_spans(basic_coverage_blocks, sorted_spans);
52-
mappings.extend(coverage_spans.into_iter().map(|CoverageSpan { bcb, span, .. }| {
53-
// Each span produced by the generator represents an ordinary code region.
54-
BcbMapping { kind: BcbMappingKind::Code(bcb), span }
55-
}));
49+
if hir_info.is_async_fn {
50+
// An async function desugars into a function that returns a future,
51+
// with the user code wrapped in a closure. Any spans in the desugared
52+
// outer function will be unhelpful, so just keep the signature span
53+
// and ignore all of the spans in the MIR body.
54+
if let Some(span) = hir_info.fn_sig_span_extended {
55+
mappings.push(BcbMapping { kind: BcbMappingKind::Code(START_BCB), span });
56+
}
57+
} else {
58+
let sorted_spans = from_mir::mir_to_initial_sorted_coverage_spans(
59+
mir_body,
60+
hir_info,
61+
basic_coverage_blocks,
62+
);
63+
let coverage_spans = SpansRefiner::refine_sorted_spans(basic_coverage_blocks, sorted_spans);
64+
mappings.extend(coverage_spans.into_iter().map(|CoverageSpan { bcb, span, .. }| {
65+
// Each span produced by the generator represents an ordinary code region.
66+
BcbMapping { kind: BcbMappingKind::Code(bcb), span }
67+
}));
68+
}
5669

5770
if mappings.is_empty() {
5871
return None;

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

+14-18
Original file line numberDiff line numberDiff line change
@@ -23,25 +23,21 @@ pub(super) fn mir_to_initial_sorted_coverage_spans(
2323
hir_info: &ExtractedHirInfo,
2424
basic_coverage_blocks: &CoverageGraph,
2525
) -> Vec<CoverageSpan> {
26-
let &ExtractedHirInfo { is_async_fn, fn_sig_span, body_span, .. } = hir_info;
27-
28-
let mut initial_spans = vec![SpanFromMir::for_fn_sig(fn_sig_span)];
29-
30-
if is_async_fn {
31-
// An async function desugars into a function that returns a future,
32-
// with the user code wrapped in a closure. Any spans in the desugared
33-
// outer function will be unhelpful, so just keep the signature span
34-
// and ignore all of the spans in the MIR body.
35-
} else {
36-
for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() {
37-
initial_spans.extend(bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data));
38-
}
26+
let &ExtractedHirInfo { body_span, .. } = hir_info;
3927

40-
// If no spans were extracted from the body, discard the signature span.
41-
// FIXME: This preserves existing behavior; consider getting rid of it.
42-
if initial_spans.len() == 1 {
43-
initial_spans.clear();
44-
}
28+
let mut initial_spans = vec![];
29+
30+
for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() {
31+
initial_spans.extend(bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data));
32+
}
33+
34+
// Only add the signature span if we found at least one span in the body.
35+
if !initial_spans.is_empty() {
36+
// If there is no usable signature span, add a fake one (before refinement)
37+
// to avoid an ugly gap between the body start and the first real span.
38+
// FIXME: Find a more principled way to solve this problem.
39+
let fn_sig_span = hir_info.fn_sig_span_extended.unwrap_or_else(|| body_span.shrink_to_lo());
40+
initial_spans.push(SpanFromMir::for_fn_sig(fn_sig_span));
4541
}
4642

4743
initial_spans.sort_by(|a, b| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb));

‎tests/coverage/closure_macro.cov-map

+17-8
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
11
Function name: closure_macro::load_configuration_files
2-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 1e, 01, 02, 02]
2+
Raw bytes (9): 0x[01, 01, 00, 01, 01, 1d, 01, 02, 02]
33
Number of files: 1
44
- file 0 => global file 1
55
Number of expressions: 0
66
Number of file 0 mappings: 1
7-
- Code(Counter(0)) at (prev + 30, 1) to (start + 2, 2)
7+
- Code(Counter(0)) at (prev + 29, 1) to (start + 2, 2)
88

99
Function name: closure_macro::main
10-
Raw bytes (43): 0x[01, 01, 02, 01, 05, 05, 02, 07, 01, 22, 01, 01, 21, 02, 02, 09, 00, 0f, 05, 00, 12, 00, 13, 02, 00, 12, 00, 13, 05, 00, 54, 00, 55, 02, 02, 09, 02, 0b, 07, 03, 01, 00, 02]
10+
Raw bytes (43): 0x[01, 01, 02, 01, 05, 05, 02, 07, 01, 21, 01, 01, 21, 02, 02, 09, 00, 0f, 05, 00, 12, 00, 13, 02, 00, 12, 00, 13, 05, 00, 54, 00, 55, 02, 02, 09, 02, 0b, 07, 03, 01, 00, 02]
1111
Number of files: 1
1212
- file 0 => global file 1
1313
Number of expressions: 2
1414
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
1515
- expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub)
1616
Number of file 0 mappings: 7
17-
- Code(Counter(0)) at (prev + 34, 1) to (start + 1, 33)
17+
- Code(Counter(0)) at (prev + 33, 1) to (start + 1, 33)
1818
- Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 15)
1919
= (c0 - c1)
2020
- Code(Counter(1)) at (prev + 0, 18) to (start + 0, 19)
@@ -27,10 +27,19 @@ Number of file 0 mappings: 7
2727
= (c1 + (c0 - c1))
2828

2929
Function name: closure_macro::main::{closure#0}
30-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 24, 12, 00, 54]
30+
Raw bytes (35): 0x[01, 01, 03, 01, 05, 05, 0b, 09, 00, 05, 01, 10, 1c, 03, 21, 05, 04, 11, 01, 27, 02, 03, 11, 00, 16, 00, 00, 17, 00, 1e, 07, 02, 09, 00, 0a]
3131
Number of files: 1
3232
- file 0 => global file 1
33-
Number of expressions: 0
34-
Number of file 0 mappings: 1
35-
- Code(Counter(0)) at (prev + 36, 18) to (start + 0, 84)
33+
Number of expressions: 3
34+
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
35+
- expression 1 operands: lhs = Counter(1), rhs = Expression(2, Add)
36+
- expression 2 operands: lhs = Counter(2), rhs = Zero
37+
Number of file 0 mappings: 5
38+
- Code(Counter(0)) at (prev + 16, 28) to (start + 3, 33)
39+
- Code(Counter(1)) at (prev + 4, 17) to (start + 1, 39)
40+
- Code(Expression(0, Sub)) at (prev + 3, 17) to (start + 0, 22)
41+
= (c0 - c1)
42+
- Code(Zero) at (prev + 0, 23) to (start + 0, 30)
43+
- Code(Expression(1, Add)) at (prev + 2, 9) to (start + 0, 10)
44+
= (c1 + (c2 + Zero))
3645

‎tests/coverage/closure_macro.coverage

+8-9
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
LL| |#![feature(coverage_attribute)]
21
LL| |// edition: 2018
32
LL| |
43
LL| |macro_rules! bail {
@@ -14,16 +13,16 @@
1413
LL| |
1514
LL| |macro_rules! on_error {
1615
LL| | ($value:expr, $error_message:expr) => {
17-
LL| | $value.or_else(|e| {
18-
LL| | // FIXME(85000): no coverage in closure macros
19-
LL| | let message = format!($error_message, e);
20-
LL| | if message.len() > 0 {
21-
LL| | println!("{}", message);
22-
LL| | Ok(String::from("ok"))
16+
LL| 0| $value.or_else(|e| {
17+
LL| 0| // This closure, which is declared in a macro, should be instrumented.
18+
LL| 0| let message = format!($error_message, e);
19+
LL| 0| if message.len() > 0 {
20+
LL| 0| println!("{}", message);
21+
LL| 0| Ok(String::from("ok"))
2322
LL| | } else {
24-
LL| | bail!("error");
23+
LL| 0| bail!("error");
2524
LL| | }
26-
LL| | })
25+
LL| 0| })
2726
LL| | };
2827
LL| |}
2928
LL| |

‎tests/coverage/closure_macro.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
#![feature(coverage_attribute)]
21
// edition: 2018
32

43
macro_rules! bail {
@@ -15,7 +14,7 @@ macro_rules! bail {
1514
macro_rules! on_error {
1615
($value:expr, $error_message:expr) => {
1716
$value.or_else(|e| {
18-
// FIXME(85000): no coverage in closure macros
17+
// This closure, which is declared in a macro, should be instrumented.
1918
let message = format!($error_message, e);
2019
if message.len() > 0 {
2120
println!("{}", message);

‎tests/coverage/closure_macro_async.cov-map

+13-4
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,19 @@ Number of file 0 mappings: 7
3535
= (c1 + (c0 - c1))
3636

3737
Function name: closure_macro_async::test::{closure#0}::{closure#0}
38-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 25, 12, 00, 54]
38+
Raw bytes (35): 0x[01, 01, 03, 01, 05, 05, 0b, 09, 00, 05, 01, 12, 1c, 03, 21, 05, 04, 11, 01, 27, 02, 03, 11, 00, 16, 00, 00, 17, 00, 1e, 07, 02, 09, 00, 0a]
3939
Number of files: 1
4040
- file 0 => global file 1
41-
Number of expressions: 0
42-
Number of file 0 mappings: 1
43-
- Code(Counter(0)) at (prev + 37, 18) to (start + 0, 84)
41+
Number of expressions: 3
42+
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
43+
- expression 1 operands: lhs = Counter(1), rhs = Expression(2, Add)
44+
- expression 2 operands: lhs = Counter(2), rhs = Zero
45+
Number of file 0 mappings: 5
46+
- Code(Counter(0)) at (prev + 18, 28) to (start + 3, 33)
47+
- Code(Counter(1)) at (prev + 4, 17) to (start + 1, 39)
48+
- Code(Expression(0, Sub)) at (prev + 3, 17) to (start + 0, 22)
49+
= (c0 - c1)
50+
- Code(Zero) at (prev + 0, 23) to (start + 0, 30)
51+
- Code(Expression(1, Add)) at (prev + 2, 9) to (start + 0, 10)
52+
= (c1 + (c2 + Zero))
4453

‎tests/coverage/closure_macro_async.coverage

+8-8
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,16 @@
1515
LL| |
1616
LL| |macro_rules! on_error {
1717
LL| | ($value:expr, $error_message:expr) => {
18-
LL| | $value.or_else(|e| {
19-
LL| | // FIXME(85000): no coverage in closure macros
20-
LL| | let message = format!($error_message, e);
21-
LL| | if message.len() > 0 {
22-
LL| | println!("{}", message);
23-
LL| | Ok(String::from("ok"))
18+
LL| 0| $value.or_else(|e| {
19+
LL| 0| // This closure, which is declared in a macro, should be instrumented.
20+
LL| 0| let message = format!($error_message, e);
21+
LL| 0| if message.len() > 0 {
22+
LL| 0| println!("{}", message);
23+
LL| 0| Ok(String::from("ok"))
2424
LL| | } else {
25-
LL| | bail!("error");
25+
LL| 0| bail!("error");
2626
LL| | }
27-
LL| | })
27+
LL| 0| })
2828
LL| | };
2929
LL| |}
3030
LL| |

‎tests/coverage/closure_macro_async.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ macro_rules! bail {
1616
macro_rules! on_error {
1717
($value:expr, $error_message:expr) => {
1818
$value.or_else(|e| {
19-
// FIXME(85000): no coverage in closure macros
19+
// This closure, which is declared in a macro, should be instrumented.
2020
let message = format!($error_message, e);
2121
if message.len() > 0 {
2222
println!("{}", message);

‎tests/coverage/coverage_attr_closure.cov-map

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
Function name: coverage_attr_closure::GLOBAL_CLOSURE_ON::{closure#0}
2+
Raw bytes (9): 0x[01, 01, 00, 01, 01, 06, 0f, 02, 02]
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 + 6, 15) to (start + 2, 2)
8+
9+
Function name: coverage_attr_closure::contains_closures_off::{closure#0} (unused)
10+
Raw bytes (9): 0x[01, 01, 00, 01, 00, 1d, 13, 02, 06]
11+
Number of files: 1
12+
- file 0 => global file 1
13+
Number of expressions: 0
14+
Number of file 0 mappings: 1
15+
- Code(Zero) at (prev + 29, 19) to (start + 2, 6)
16+
17+
Function name: coverage_attr_closure::contains_closures_on
18+
Raw bytes (19): 0x[01, 01, 00, 03, 01, 0f, 01, 02, 05, 01, 04, 06, 02, 05, 01, 04, 06, 01, 02]
19+
Number of files: 1
20+
- file 0 => global file 1
21+
Number of expressions: 0
22+
Number of file 0 mappings: 3
23+
- Code(Counter(0)) at (prev + 15, 1) to (start + 2, 5)
24+
- Code(Counter(0)) at (prev + 4, 6) to (start + 2, 5)
25+
- Code(Counter(0)) at (prev + 4, 6) to (start + 1, 2)
26+
27+
Function name: coverage_attr_closure::contains_closures_on::{closure#0} (unused)
28+
Raw bytes (9): 0x[01, 01, 00, 01, 00, 11, 13, 02, 06]
29+
Number of files: 1
30+
- file 0 => global file 1
31+
Number of expressions: 0
32+
Number of file 0 mappings: 1
33+
- Code(Zero) at (prev + 17, 19) to (start + 2, 6)
34+

‎tests/coverage/coverage_attr_closure.coverage

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
LL| |#![feature(coverage_attribute, stmt_expr_attributes)]
2+
LL| |#![allow(dead_code)]
3+
LL| |// edition: 2021
4+
LL| |
5+
LL| |static GLOBAL_CLOSURE_ON: fn(&str) = #[coverage(on)]
6+
LL| 0||input: &str| {
7+
LL| 0| println!("{input}");
8+
LL| 0|};
9+
LL| |static GLOBAL_CLOSURE_OFF: fn(&str) = #[coverage(off)]
10+
LL| ||input: &str| {
11+
LL| | println!("{input}");
12+
LL| |};
13+
LL| |
14+
LL| |#[coverage(on)]
15+
LL| 1|fn contains_closures_on() {
16+
LL| 1| let _local_closure_on = #[coverage(on)]
17+
LL| 1| |input: &str| {
18+
LL| 0| println!("{input}");
19+
LL| 1| };
20+
LL| 1| let _local_closure_off = #[coverage(off)]
21+
LL| 1| |input: &str| {
22+
LL| | println!("{input}");
23+
LL| 1| };
24+
LL| 1|}
25+
LL| |
26+
LL| |#[coverage(off)]
27+
LL| |fn contains_closures_off() {
28+
LL| | let _local_closure_on = #[coverage(on)]
29+
LL| 0| |input: &str| {
30+
LL| 0| println!("{input}");
31+
LL| 0| };
32+
LL| | let _local_closure_off = #[coverage(off)]
33+
LL| | |input: &str| {
34+
LL| | println!("{input}");
35+
LL| | };
36+
LL| |}
37+
LL| |
38+
LL| |#[coverage(off)]
39+
LL| |fn main() {
40+
LL| | contains_closures_on();
41+
LL| | contains_closures_off();
42+
LL| |}
43+

0 commit comments

Comments
 (0)