mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #120569 - Zalathar:fn-sig, r=oli-obk

Browse Source 
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
This commit is contained in:
Matthias Krüger 2024-02-05 06:37:15 +01:00 committed by GitHub
commit 7294c15f41
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 272 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
compiler/rustc_mir_transform/src/coverage/mod.rs
View File

@ -394,7 +394,9 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
struct ExtractedHirInfo {
function_source_hash: u64,
is_async_fn: bool,
fn_sig_span: Span,
/// The span of the function's signature, extended to the start of `body_span`.
/// Must have the same context and filename as the body span.
fn_sig_span_extended: Option<Span>,
body_span: Span,
}
@ -407,13 +409,25 @@ fn extract_hir_info<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> ExtractedHir
hir::map::associated_body(hir_node).expect("HIR node is a function with body");
let hir_body = tcx.hir().body(fn_body_id);
let is_async_fn = hir_node.fn_sig().is_some_and(|fn_sig| fn_sig.header.is_async());
let body_span = get_body_span(tcx, hir_body, def_id);
let maybe_fn_sig = hir_node.fn_sig();
let is_async_fn = maybe_fn_sig.is_some_and(|fn_sig| fn_sig.header.is_async());
let mut body_span = hir_body.value.span;
use rustc_hir::{Closure, Expr, ExprKind, Node};
// Unexpand a closure's body span back to the context of its declaration.
// This helps with closure bodies that consist of just a single bang-macro,
// and also with closure bodies produced by async desugaring.
if let Node::Expr(&Expr { kind: ExprKind::Closure(&Closure { fn_decl_span, .. }), .. }) =
hir_node
{
body_span = body_span.find_ancestor_in_same_ctxt(fn_decl_span).unwrap_or(body_span);
}
// The actual signature span is only used if it has the same context and
// filename as the body, and precedes the body.
let maybe_fn_sig_span = hir_node.fn_sig().map(|fn_sig| fn_sig.span);
let fn_sig_span = maybe_fn_sig_span
let fn_sig_span_extended = maybe_fn_sig
.map(|fn_sig| fn_sig.span)
.filter(|&fn_sig_span| {
let source_map = tcx.sess.source_map();
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
&& file_idx(fn_sig_span) == file_idx(body_span)
})
// If so, extend it to the start of the body span.
.map(|fn_sig_span| fn_sig_span.with_hi(body_span.lo()))
// Otherwise, create a dummy signature span at the start of the body.
.unwrap_or_else(|| body_span.shrink_to_lo());
.map(|fn_sig_span| fn_sig_span.with_hi(body_span.lo()));
let function_source_hash = hash_mir_source(tcx, hir_body);
ExtractedHirInfo { function_source_hash, is_async_fn, fn_sig_span, body_span }
}
fn get_body_span<'tcx>(
tcx: TyCtxt<'tcx>,
hir_body: &rustc_hir::Body<'tcx>,
def_id: LocalDefId,
) -> Span {
let mut body_span = hir_body.value.span;
if tcx.is_closure_or_coroutine(def_id.to_def_id()) {
// If the current function is a closure, and its "body" span was created
// by macro expansion or compiler desugaring, try to walk backwards to
// the pre-expansion call site or body.
body_span = body_span.source_callsite();
}
body_span
ExtractedHirInfo { function_source_hash, is_async_fn, fn_sig_span_extended, body_span }
}
fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 {

29
compiler/rustc_mir_transform/src/coverage/spans.rs
View File

@ -3,7 +3,7 @@
use rustc_middle::mir;
use rustc_span::{BytePos, Span, DUMMY_SP};
use super::graph::{BasicCoverageBlock, CoverageGraph};
use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph, START_BCB};
use crate::coverage::ExtractedHirInfo;
mod from_mir;
@ -46,13 +46,26 @@ pub(super) fn generate_coverage_spans(
) -> Option<CoverageSpans> {
let mut mappings = vec![];
let sorted_spans =
from_mir::mir_to_initial_sorted_coverage_spans(mir_body, hir_info, basic_coverage_blocks);
let coverage_spans = SpansRefiner::refine_sorted_spans(basic_coverage_blocks, sorted_spans);
mappings.extend(coverage_spans.into_iter().map(|CoverageSpan { bcb, span, .. }| {
// Each span produced by the generator represents an ordinary code region.
BcbMapping { kind: BcbMappingKind::Code(bcb), span }
}));
if hir_info.is_async_fn {
// An async function desugars into a function that returns a future,
// with the user code wrapped in a closure. Any spans in the desugared
// outer function will be unhelpful, so just keep the signature span
// and ignore all of the spans in the MIR body.
if let Some(span) = hir_info.fn_sig_span_extended {
mappings.push(BcbMapping { kind: BcbMappingKind::Code(START_BCB), span });
}
} else {
let sorted_spans = from_mir::mir_to_initial_sorted_coverage_spans(
mir_body,
hir_info,
basic_coverage_blocks,
);
let coverage_spans = SpansRefiner::refine_sorted_spans(basic_coverage_blocks, sorted_spans);
mappings.extend(coverage_spans.into_iter().map(|CoverageSpan { bcb, span, .. }| {
// Each span produced by the generator represents an ordinary code region.
BcbMapping { kind: BcbMappingKind::Code(bcb), span }
}));
}
if mappings.is_empty() {
return None;

28
compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs
View File

@ -23,25 +23,21 @@ pub(super) fn mir_to_initial_sorted_coverage_spans(
hir_info: &ExtractedHirInfo,
basic_coverage_blocks: &CoverageGraph,
) -> Vec<CoverageSpan> {
let &ExtractedHirInfo { is_async_fn, fn_sig_span, body_span, .. } = hir_info;
let &ExtractedHirInfo { body_span, .. } = hir_info;
let mut initial_spans = vec![SpanFromMir::for_fn_sig(fn_sig_span)];
let mut initial_spans = vec![];
if is_async_fn {
// An async function desugars into a function that returns a future,
// with the user code wrapped in a closure. Any spans in the desugared
// outer function will be unhelpful, so just keep the signature span
// and ignore all of the spans in the MIR body.
} else {
for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() {
initial_spans.extend(bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data));
}
for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() {
initial_spans.extend(bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data));
}
// If no spans were extracted from the body, discard the signature span.
// FIXME: This preserves existing behavior; consider getting rid of it.
if initial_spans.len() == 1 {
initial_spans.clear();
}
// Only add the signature span if we found at least one span in the body.
if !initial_spans.is_empty() {
// If there is no usable signature span, add a fake one (before refinement)
// to avoid an ugly gap between the body start and the first real span.
// FIXME: Find a more principled way to solve this problem.
let fn_sig_span = hir_info.fn_sig_span_extended.unwrap_or_else(|| body_span.shrink_to_lo());
initial_spans.push(SpanFromMir::for_fn_sig(fn_sig_span));
}
initial_spans.sort_by(|a, b| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb));

25
tests/coverage/closure_macro.cov-map
View File

@ -1,20 +1,20 @@
Function name: closure_macro::load_configuration_files
Raw bytes (9): 0x[01, 01, 00, 01, 01, 1e, 01, 02, 02]
Raw bytes (9): 0x[01, 01, 00, 01, 01, 1d, 01, 02, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 30, 1) to (start + 2, 2)
- Code(Counter(0)) at (prev + 29, 1) to (start + 2, 2)
Function name: closure_macro::main
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]
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]
Number of files: 1
- file 0 => global file 1
Number of expressions: 2
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
- expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub)
Number of file 0 mappings: 7
- Code(Counter(0)) at (prev + 34, 1) to (start + 1, 33)
- Code(Counter(0)) at (prev + 33, 1) to (start + 1, 33)
- Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 15)
= (c0 - c1)
- Code(Counter(1)) at (prev + 0, 18) to (start + 0, 19)
@ -27,10 +27,19 @@ Number of file 0 mappings: 7
= (c1 + (c0 - c1))
Function name: closure_macro::main::{closure#0}
Raw bytes (9): 0x[01, 01, 00, 01, 01, 24, 12, 00, 54]
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]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 36, 18) to (start + 0, 84)
Number of expressions: 3
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
- expression 1 operands: lhs = Counter(1), rhs = Expression(2, Add)
- expression 2 operands: lhs = Counter(2), rhs = Zero
Number of file 0 mappings: 5
- Code(Counter(0)) at (prev + 16, 28) to (start + 3, 33)
- Code(Counter(1)) at (prev + 4, 17) to (start + 1, 39)
- Code(Expression(0, Sub)) at (prev + 3, 17) to (start + 0, 22)
= (c0 - c1)
- Code(Zero) at (prev + 0, 23) to (start + 0, 30)
- Code(Expression(1, Add)) at (prev + 2, 9) to (start + 0, 10)
= (c1 + (c2 + Zero))

17
tests/coverage/closure_macro.coverage
View File

@ -1,4 +1,3 @@
LL| |#![feature(coverage_attribute)]
LL| |// edition: 2018
LL| |
LL| |macro_rules! bail {
@ -14,16 +13,16 @@
LL| |
LL| |macro_rules! on_error {
LL| | ($value:expr, $error_message:expr) => {
LL| | $value.or_else(|e| {
LL| | // FIXME(85000): no coverage in closure macros
LL| | let message = format!($error_message, e);
LL| | if message.len() > 0 {
LL| | println!("{}", message);
LL| | Ok(String::from("ok"))
LL| 0| $value.or_else(|e| {
LL| 0| // This closure, which is declared in a macro, should be instrumented.
LL| 0| let message = format!($error_message, e);
LL| 0| if message.len() > 0 {
LL| 0| println!("{}", message);
LL| 0| Ok(String::from("ok"))
LL| | } else {
LL| | bail!("error");
LL| 0| bail!("error");
LL| | }
LL| | })
LL| 0| })
LL| | };
LL| |}
LL| |

3
tests/coverage/closure_macro.rs
View File

@ -1,4 +1,3 @@
#![feature(coverage_attribute)]
// edition: 2018
macro_rules! bail {
@ -15,7 +14,7 @@ macro_rules! bail {
macro_rules! on_error {
($value:expr, $error_message:expr) => {
$value.or_else(|e| {
// FIXME(85000): no coverage in closure macros
// This closure, which is declared in a macro, should be instrumented.
let message = format!($error_message, e);
if message.len() > 0 {
println!("{}", message);

17
tests/coverage/closure_macro_async.cov-map
View File

@ -35,10 +35,19 @@ Number of file 0 mappings: 7
= (c1 + (c0 - c1))
Function name: closure_macro_async::test::{closure#0}::{closure#0}
Raw bytes (9): 0x[01, 01, 00, 01, 01, 25, 12, 00, 54]
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]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 37, 18) to (start + 0, 84)
Number of expressions: 3
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
- expression 1 operands: lhs = Counter(1), rhs = Expression(2, Add)
- expression 2 operands: lhs = Counter(2), rhs = Zero
Number of file 0 mappings: 5
- Code(Counter(0)) at (prev + 18, 28) to (start + 3, 33)
- Code(Counter(1)) at (prev + 4, 17) to (start + 1, 39)
- Code(Expression(0, Sub)) at (prev + 3, 17) to (start + 0, 22)
= (c0 - c1)
- Code(Zero) at (prev + 0, 23) to (start + 0, 30)
- Code(Expression(1, Add)) at (prev + 2, 9) to (start + 0, 10)
= (c1 + (c2 + Zero))

16
tests/coverage/closure_macro_async.coverage
View File

@ -15,16 +15,16 @@
LL| |
LL| |macro_rules! on_error {
LL| | ($value:expr, $error_message:expr) => {
LL| | $value.or_else(|e| {
LL| | // FIXME(85000): no coverage in closure macros
LL| | let message = format!($error_message, e);
LL| | if message.len() > 0 {
LL| | println!("{}", message);
LL| | Ok(String::from("ok"))
LL| 0| $value.or_else(|e| {
LL| 0| // This closure, which is declared in a macro, should be instrumented.
LL| 0| let message = format!($error_message, e);
LL| 0| if message.len() > 0 {
LL| 0| println!("{}", message);
LL| 0| Ok(String::from("ok"))
LL| | } else {
LL| | bail!("error");
LL| 0| bail!("error");
LL| | }
LL| | })
LL| 0| })
LL| | };
LL| |}
LL| |

2
tests/coverage/closure_macro_async.rs
View File

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

34
tests/coverage/coverage_attr_closure.cov-map Normal file
View File

@ -0,0 +1,34 @@
Function name: coverage_attr_closure::GLOBAL_CLOSURE_ON::{closure#0}
Raw bytes (9): 0x[01, 01, 00, 01, 01, 06, 0f, 02, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 6, 15) to (start + 2, 2)
Function name: coverage_attr_closure::contains_closures_off::{closure#0} (unused)
Raw bytes (9): 0x[01, 01, 00, 01, 00, 1d, 13, 02, 06]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Zero) at (prev + 29, 19) to (start + 2, 6)
Function name: coverage_attr_closure::contains_closures_on
Raw bytes (19): 0x[01, 01, 00, 03, 01, 0f, 01, 02, 05, 01, 04, 06, 02, 05, 01, 04, 06, 01, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 3
- Code(Counter(0)) at (prev + 15, 1) to (start + 2, 5)
- Code(Counter(0)) at (prev + 4, 6) to (start + 2, 5)
- Code(Counter(0)) at (prev + 4, 6) to (start + 1, 2)
Function name: coverage_attr_closure::contains_closures_on::{closure#0} (unused)
Raw bytes (9): 0x[01, 01, 00, 01, 00, 11, 13, 02, 06]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Zero) at (prev + 17, 19) to (start + 2, 6)

43
tests/coverage/coverage_attr_closure.coverage Normal file
View File

@ -0,0 +1,43 @@
LL| |#![feature(coverage_attribute, stmt_expr_attributes)]
LL| |#![allow(dead_code)]
LL| |// edition: 2021
LL| |
LL| |static GLOBAL_CLOSURE_ON: fn(&str) = #[coverage(on)]
LL| 0||input: &str| {
LL| 0| println!("{input}");
LL| 0|};
LL| |static GLOBAL_CLOSURE_OFF: fn(&str) = #[coverage(off)]
LL| ||input: &str| {
LL| | println!("{input}");
LL| |};
LL| |
LL| |#[coverage(on)]
LL| 1|fn contains_closures_on() {
LL| 1| let _local_closure_on = #[coverage(on)]
LL| 1| |input: &str| {
LL| 0| println!("{input}");
LL| 1| };
LL| 1| let _local_closure_off = #[coverage(off)]
LL| 1| |input: &str| {
LL| | println!("{input}");
LL| 1| };
LL| 1|}
LL| |
LL| |#[coverage(off)]
LL| |fn contains_closures_off() {
LL| | let _local_closure_on = #[coverage(on)]
LL| 0| |input: &str| {
LL| 0| println!("{input}");
LL| 0| };
LL| | let _local_closure_off = #[coverage(off)]
LL| | |input: &str| {
LL| | println!("{input}");
LL| | };
LL| |}
LL| |
LL| |#[coverage(off)]
LL| |fn main() {
LL| | contains_closures_on();
LL| | contains_closures_off();
LL| |}

42
tests/coverage/coverage_attr_closure.rs Normal file
View File

@ -0,0 +1,42 @@
#![feature(coverage_attribute, stmt_expr_attributes)]
#![allow(dead_code)]
// edition: 2021
static GLOBAL_CLOSURE_ON: fn(&str) = #[coverage(on)]
|input: &str| {
println!("{input}");
};
static GLOBAL_CLOSURE_OFF: fn(&str) = #[coverage(off)]
|input: &str| {
println!("{input}");
};
#[coverage(on)]
fn contains_closures_on() {
let _local_closure_on = #[coverage(on)]
|input: &str| {
println!("{input}");
};
let _local_closure_off = #[coverage(off)]
|input: &str| {
println!("{input}");
};
}
#[coverage(off)]
fn contains_closures_off() {
let _local_closure_on = #[coverage(on)]
|input: &str| {
println!("{input}");
};
let _local_closure_off = #[coverage(off)]
|input: &str| {
println!("{input}");
};
}
#[coverage(off)]
fn main() {
contains_closures_on();
contains_closures_off();
}

16
tests/coverage/macro_in_closure.cov-map Normal file
View File

@ -0,0 +1,16 @@
Function name: macro_in_closure::NO_BLOCK::{closure#0}
Raw bytes (9): 0x[01, 01, 00, 01, 01, 07, 1c, 00, 2d]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 7, 28) to (start + 0, 45)
Function name: macro_in_closure::WITH_BLOCK::{closure#0}
Raw bytes (9): 0x[01, 01, 00, 01, 01, 09, 1e, 02, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 9, 30) to (start + 2, 2)

18
tests/coverage/macro_in_closure.coverage Normal file
View File

@ -0,0 +1,18 @@
LL| |#![feature(coverage_attribute)]
LL| |// edition: 2021
LL| |
LL| |// If a closure body consists entirely of a single bang-macro invocation, the
LL| |// body span ends up inside the macro-expansion, so we need to un-expand it
LL| |// back to the declaration site.
LL| 1|static NO_BLOCK: fn() = || println!("hello");
LL| |
LL| 1|static WITH_BLOCK: fn() = || {
LL| 1| println!("hello");
LL| 1|};
LL| |
LL| |#[coverage(off)]
LL| |fn main() {
LL| | NO_BLOCK();
LL| | WITH_BLOCK();
LL| |}

17
tests/coverage/macro_in_closure.rs Normal file
View File

@ -0,0 +1,17 @@
#![feature(coverage_attribute)]
// edition: 2021
// If a closure body consists entirely of a single bang-macro invocation, the
// body span ends up inside the macro-expansion, so we need to un-expand it
// back to the declaration site.
static NO_BLOCK: fn() = || println!("hello");
static WITH_BLOCK: fn() = || {
println!("hello");
};
#[coverage(off)]
fn main() {
NO_BLOCK();
WITH_BLOCK();
}