Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coverage: Discard spans that fill the entire function body #121135

Merged
merged 2 commits into from
Feb 17, 2024
Merged

coverage: Discard spans that fill the entire function body #121135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75af3c5
coverage: Regression test for a span extraction inconsistency
Zalathar Feb 15, 2024
cd9021e
coverage: Discard spans that fill the entire function body
Zalathar Feb 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs
View file
Open in desktop
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 `()`.
Comment on lines +137 to +138
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some cases like fn foo() { }, I wonder if this heuristic will trigger incorrectly? Since all the changes in the tests are for the better, I'm not super worried about that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about adding a special-case to detect functions with a non-empty body span and no extracted spans, in order to explicitly add a span that covers the whole body. But currently it doesn't seem to be necessary, so I haven't bothered.

For fn foo() {}, I believe the way it currently works is that we generate a span for the signature, and there's another zero-width span right at the end of the body (representing the return), and the span refiner then merges those together into something that covers the whole body.

(None of this is ideal, but sadly that's the reality of trying to heuristically extract coverage spans from MIR. Right now I'm happy to be able to make any progress in simplifying the span refiner into something a bit more maintainable.)

.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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading