Skip to content

Commit

Permalink
Unrolled build for rust-lang#121135
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#121135 - Zalathar:no-whole-body-span, r=wesleywiser

coverage: Discard spans that fill the entire function body

While debugging some other coverage changes, I discovered a frustrating inconsistency that occurs in functions containing closures, if they end with an implicit `()` return instead of an explicit trailing-expression.

This turns out to have been caused by the corresponding node in MIR having a span that covers the entire function body. When preparing coverage spans, any span that fills the whole body tends to cause more harm than good, so this PR detects and discards those spans.

(This isn't the first time whole-body spans have caused problems; we also eliminated some of them in rust-lang#118525.)
  • Loading branch information
rust-timer authored Feb 17, 2024
2 parents dfdbe30 + cd9021e commit e123197
Show file tree
Hide file tree
Showing 11 changed files with 117 additions and 28 deletions.
13 changes: 9 additions & 4 deletions compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,18 +132,23 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>(
bcb_data.basic_blocks.iter().flat_map(move |&bb| {
let data = &mir_body[bb];

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

let statement_spans = data.statements.iter().filter_map(move |statement| {
let expn_span = filtered_statement_span(statement)?;
let (span, visible_macro) =
unexpand_into_body_span_with_visible_macro(expn_span, body_span)?;
let (span, visible_macro) = unexpand(expn_span)?;

Some(SpanFromMir::new(span, visible_macro, bcb, is_closure_like(statement)))
});

let terminator_span = Some(data.terminator()).into_iter().filter_map(move |terminator| {
let expn_span = filtered_terminator_span(terminator)?;
let (span, visible_macro) =
unexpand_into_body_span_with_visible_macro(expn_span, body_span)?;
let (span, visible_macro) = unexpand(expn_span)?;

Some(SpanFromMir::new(span, visible_macro, bcb, false))
});
Expand Down
34 changes: 34 additions & 0 deletions tests/coverage/closure_unit_return.cov-map
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
Function name: closure_unit_return::explicit_unit
Raw bytes (14): 0x[01, 01, 00, 02, 01, 07, 01, 01, 10, 01, 05, 05, 02, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 2
- Code(Counter(0)) at (prev + 7, 1) to (start + 1, 16)
- Code(Counter(0)) at (prev + 5, 5) to (start + 2, 2)

Function name: closure_unit_return::explicit_unit::{closure#0} (unused)
Raw bytes (9): 0x[01, 01, 00, 01, 00, 08, 16, 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 + 8, 22) to (start + 2, 6)

Function name: closure_unit_return::implicit_unit
Raw bytes (14): 0x[01, 01, 00, 02, 01, 10, 01, 01, 10, 01, 05, 05, 02, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 2
- Code(Counter(0)) at (prev + 16, 1) to (start + 1, 16)
- Code(Counter(0)) at (prev + 5, 5) to (start + 2, 2)

Function name: closure_unit_return::implicit_unit::{closure#0} (unused)
Raw bytes (9): 0x[01, 01, 00, 01, 00, 11, 16, 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, 22) to (start + 2, 6)

30 changes: 30 additions & 0 deletions tests/coverage/closure_unit_return.coverage
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
LL| |#![feature(coverage_attribute)]
LL| |// edition: 2021
LL| |
LL| |// Regression test for an inconsistency between functions that return the value
LL| |// of their trailing expression, and functions that implicitly return `()`.
LL| |
LL| 1|fn explicit_unit() {
LL| 1| let closure = || {
LL| 0| ();
LL| 0| };
LL| |
LL| 1| drop(closure);
LL| 1| () // explicit return of trailing value
LL| 1|}
LL| |
LL| 1|fn implicit_unit() {
LL| 1| let closure = || {
LL| 0| ();
LL| 0| };
LL| |
LL| 1| drop(closure);
LL| 1| // implicit return of `()`
LL| 1|}
LL| |
LL| |#[coverage(off)]
LL| |fn main() {
LL| | explicit_unit();
LL| | implicit_unit();
LL| |}

29 changes: 29 additions & 0 deletions tests/coverage/closure_unit_return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#![feature(coverage_attribute)]
// edition: 2021

// Regression test for an inconsistency between functions that return the value
// of their trailing expression, and functions that implicitly return `()`.

fn explicit_unit() {
let closure = || {
();
};

drop(closure);
() // explicit return of trailing value
}

fn implicit_unit() {
let closure = || {
();
};

drop(closure);
// implicit return of `()`
}

#[coverage(off)]
fn main() {
explicit_unit();
implicit_unit();
}
8 changes: 4 additions & 4 deletions tests/coverage/coverage_attr_closure.cov-map
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ 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]
Raw bytes (19): 0x[01, 01, 00, 03, 01, 0f, 01, 01, 1a, 01, 05, 09, 00, 1b, 01, 04, 01, 00, 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)
- Code(Counter(0)) at (prev + 15, 1) to (start + 1, 26)
- Code(Counter(0)) at (prev + 5, 9) to (start + 0, 27)
- Code(Counter(0)) at (prev + 4, 1) to (start + 0, 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]
Expand Down
8 changes: 4 additions & 4 deletions tests/coverage/coverage_attr_closure.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
LL| |#[coverage(on)]
LL| 1|fn contains_closures_on() {
LL| 1| let _local_closure_on = #[coverage(on)]
LL| 1| |input: &str| {
LL| 0| |input: &str| {
LL| 0| println!("{input}");
LL| 1| };
LL| 0| };
LL| 1| let _local_closure_off = #[coverage(off)]
LL| 1| |input: &str| {
LL| | |input: &str| {
LL| | println!("{input}");
LL| 1| };
LL| | };
LL| 1|}
LL| |
LL| |#[coverage(off)]
Expand Down
6 changes: 3 additions & 3 deletions tests/coverage/inline-dead.cov-map
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ Number of file 0 mappings: 4
= (Zero + (c0 - Zero))

Function name: inline_dead::main
Raw bytes (14): 0x[01, 01, 00, 02, 01, 04, 01, 03, 0d, 01, 05, 06, 02, 02]
Raw bytes (14): 0x[01, 01, 00, 02, 01, 04, 01, 03, 0a, 01, 06, 05, 01, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 2
- Code(Counter(0)) at (prev + 4, 1) to (start + 3, 13)
- Code(Counter(0)) at (prev + 5, 6) to (start + 2, 2)
- Code(Counter(0)) at (prev + 4, 1) to (start + 3, 10)
- Code(Counter(0)) at (prev + 6, 5) to (start + 1, 2)

Function name: inline_dead::main::{closure#0}
Raw bytes (23): 0x[01, 01, 02, 00, 06, 01, 00, 03, 01, 07, 17, 01, 16, 00, 01, 17, 00, 18, 03, 01, 05, 00, 06]
Expand Down
4 changes: 2 additions & 2 deletions tests/coverage/macro_name_span.cov-map
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
Function name: macro_name_span::affected_function
Raw bytes (9): 0x[01, 01, 00, 01, 01, 16, 1c, 02, 06]
Raw bytes (9): 0x[01, 01, 00, 01, 01, 16, 1c, 01, 40]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 22, 28) to (start + 2, 6)
- Code(Counter(0)) at (prev + 22, 28) to (start + 1, 64)

Function name: macro_name_span::main
Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 01, 02, 02]
Expand Down
2 changes: 1 addition & 1 deletion tests/coverage/macro_name_span.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@
LL| |macro_name_span_helper::macro_that_defines_a_function! {
LL| 1| fn affected_function() {
LL| 1| macro_with_an_unreasonably_and_egregiously_long_name!();
LL| 1| }
LL| | }
LL| |}

8 changes: 0 additions & 8 deletions tests/coverage/unicode.cov-map
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,6 @@ Number of file 0 mappings: 9
- Code(Expression(5, Add)) at (prev + 2, 5) to (start + 1, 2)
= (c4 + ((((c0 + c1) - c1) - c2) + c3))

Function name: unicode::サビ
Raw bytes (9): 0x[01, 01, 00, 01, 01, 1e, 14, 00, 18]
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, 20) to (start + 0, 24)

Function name: unicode::他 (unused)
Raw bytes (9): 0x[01, 01, 00, 01, 00, 1e, 19, 00, 25]
Number of files: 1
Expand Down
3 changes: 1 addition & 2 deletions tests/coverage/unicode.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@
LL| |
LL| |macro_rules! macro_that_defines_a_function {
LL| | (fn $名:ident () $体:tt) => {
LL| 1| fn $名 () $体 fn 他 () {}
^0
LL| 0| fn $名 () $体 fn 他 () {}
LL| | }
LL| |}
LL| |
Expand Down

0 comments on commit e123197

Please sign in to comment.