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 instruments closure bodies in macros (not the macro body) #84897

Merged
merged 1 commit into from
May 7, 2021
Merged

Coverage instruments closure bodies in macros (not the macro body) #84897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 24 additions & 3 deletions compiler/rustc_mir/src/transform/coverage/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use rustc_middle::mir::{
use rustc_middle::ty::TyCtxt;
use rustc_span::def_id::DefId;
use rustc_span::source_map::SourceMap;
use rustc_span::{CharPos, Pos, SourceFile, Span, Symbol};
use rustc_span::{CharPos, ExpnKind, Pos, SourceFile, Span, Symbol};

/// A simple error message wrapper for `coverage::Error`s.
#[derive(Debug)]
Expand Down Expand Up @@ -113,8 +113,29 @@ struct Instrumentor<'a, 'tcx> {
impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
fn new(pass_name: &'a str, tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self {
let source_map = tcx.sess.source_map();
let (some_fn_sig, hir_body) = fn_sig_and_body(tcx, mir_body.source.def_id());
let body_span = hir_body.value.span;
let def_id = mir_body.source.def_id();
let (some_fn_sig, hir_body) = fn_sig_and_body(tcx, def_id);

let mut body_span = hir_body.value.span;

if tcx.is_closure(def_id) {
// If the MIR function is a closure, and if the closure body span
// starts from a macro, but it's content is not in that macro, try
// to find a non-macro callsite, and instrument the spans there
// instead.
loop {
let expn_data = body_span.ctxt().outer_expn_data();
if expn_data.is_root() {
break;
}
if let ExpnKind::Macro(..) = expn_data.kind {
body_span = expn_data.call_site;
} else {
break;
}
}
}

let source_file = source_map.lookup_source_file(body_span.lo());
let fn_sig_span = match some_fn_sig.filter(|fn_sig| {
fn_sig.span.ctxt() == body_span.ctxt()
Expand Down
106 changes: 84 additions & 22 deletions src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
37| 0| countdown = 10;
38| 0| }
39| 0| "alt string 2".to_owned()
40| 1| };
40| 0| };
41| 1| println!(
42| 1| "The string or alt: {}"
43| 1| ,
Expand Down Expand Up @@ -125,36 +125,98 @@
125| 0| countdown = 10;
126| 0| }
127| 0| "closure should be unused".to_owned()
128| 1| };
129| 1|
128| 0| };
129| |
130| 1| let mut countdown = 10;
131| 1| let _short_unused_closure = | _unused_arg: u8 | countdown += 1;
^0
132| 1|
133| 1| // Macros can sometimes confuse the coverage results. Compare this next assignment, with an
134| 1| // unused closure that invokes the `println!()` macro, with the closure assignment above, that
135| 1| // does not use a macro. The closure above correctly shows `0` executions.
136| 1| let _short_unused_closure = | _unused_arg: u8 | println!("not called");
137| 1| // The closure assignment above is executed, with a line count of `1`, but the `println!()`
138| 1| // could not have been called, and yet, there is no indication that it wasn't...
139| 1|
140| 1| // ...but adding block braces gives the expected result, showing the block was not executed.
132| |
133| |
134| 1| let short_used_covered_closure_macro = | used_arg: u8 | println!("called");
135| 1| let short_used_not_covered_closure_macro = | used_arg: u8 | println!("not called");
^0
136| 1| let _short_unused_closure_macro = | _unused_arg: u8 | println!("not called");
^0
137| |
138| |
139| |
140| |
141| 1| let _short_unused_closure_block = | _unused_arg: u8 | { println!("not called") };
^0
142| 1|
142| |
143| 1| let _shortish_unused_closure = | _unused_arg: u8 | {
144| 0| println!("not called")
145| 1| };
146| 1|
145| 0| };
146| |
147| 1| let _as_short_unused_closure = |
148| | _unused_arg: u8
149| 1| | { println!("not called") };
^0
150| 1|
149| 0| | { println!("not called") };
150| |
151| 1| let _almost_as_short_unused_closure = |
152| | _unused_arg: u8
153| 1| | { println!("not called") }
^0
154| 1| ;
155| 1|}
153| 0| | { println!("not called") }
154| | ;
155| |
156| |
157| |
158| |
159| |
160| 1| let _short_unused_closure_line_break_no_block = | _unused_arg: u8 |
161| 0|println!("not called")
162| | ;
163| |
164| 1| let _short_unused_closure_line_break_no_block2 =
165| | | _unused_arg: u8 |
166| 0| println!(
167| 0| "not called"
168| 0| )
169| | ;
170| |
171| 1| let short_used_not_covered_closure_line_break_no_block_embedded_branch =
172| 1| | _unused_arg: u8 |
173| 0| println!(
174| 0| "not called: {}",
175| 0| if is_true { "check" } else { "me" }
176| 0| )
177| | ;
178| |
179| 1| let short_used_not_covered_closure_line_break_block_embedded_branch =
180| 1| | _unused_arg: u8 |
181| 0| {
182| 0| println!(
183| 0| "not called: {}",
184| 0| if is_true { "check" } else { "me" }
185| | )
186| 0| }
187| | ;
188| |
189| 1| let short_used_covered_closure_line_break_no_block_embedded_branch =
190| 1| | _unused_arg: u8 |
191| 1| println!(
192| 1| "not called: {}",
193| 1| if is_true { "check" } else { "me" }
^0
194| 1| )
195| | ;
196| |
197| 1| let short_used_covered_closure_line_break_block_embedded_branch =
198| 1| | _unused_arg: u8 |
199| 1| {
200| 1| println!(
201| 1| "not called: {}",
202| 1| if is_true { "check" } else { "me" }
^0
203| | )
204| 1| }
205| | ;
206| |
207| 1| if is_false {
208| 0| short_used_not_covered_closure_macro(0);
209| 0| short_used_not_covered_closure_line_break_no_block_embedded_branch(0);
210| 0| short_used_not_covered_closure_line_break_block_embedded_branch(0);
211| 1| }
212| 1| short_used_covered_closure_macro(0);
213| 1| short_used_covered_closure_line_break_no_block_embedded_branch(0);
214| 1| short_used_covered_closure_line_break_block_embedded_branch(0);
215| 1|}

14 changes: 7 additions & 7 deletions src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure_macro.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
14| |
15| |macro_rules! on_error {
16| | ($value:expr, $error_message:expr) => {
17| 0| $value.or_else(|e| {
18| 0| let message = format!($error_message, e);
19| 0| if message.len() > 0 {
20| 0| println!("{}", message);
21| 0| Ok(String::from("ok"))
17| | $value.or_else(|e| { // FIXME(85000): no coverage in closure macros
18| | let message = format!($error_message, e);
19| | if message.len() > 0 {
20| | println!("{}", message);
21| | Ok(String::from("ok"))
22| | } else {
23| 0| bail!("error");
23| | bail!("error");
24| | }
25| 0| })
25| | })
26| | };
27| |}
28| |
Expand Down
14 changes: 7 additions & 7 deletions src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure_macro_async.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
14| |
15| |macro_rules! on_error {
16| | ($value:expr, $error_message:expr) => {
17| 0| $value.or_else(|e| {
18| 0| let message = format!($error_message, e);
19| 0| if message.len() > 0 {
20| 0| println!("{}", message);
21| 0| Ok(String::from("ok"))
17| | $value.or_else(|e| { // FIXME(85000): no coverage in closure macros
18| | let message = format!($error_message, e);
19| | if message.len() > 0 {
20| | println!("{}", message);
21| | Ok(String::from("ok"))
22| | } else {
23| 0| bail!("error");
23| | bail!("error");
24| | }
25| 0| })
25| | })
26| | };
27| |}
28| |
Expand Down
76 changes: 68 additions & 8 deletions src/test/run-make-fulldeps/coverage/closure.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,14 @@ fn main() {
let mut countdown = 10;
let _short_unused_closure = | _unused_arg: u8 | countdown += 1;

// Macros can sometimes confuse the coverage results. Compare this next assignment, with an
// unused closure that invokes the `println!()` macro, with the closure assignment above, that
// does not use a macro. The closure above correctly shows `0` executions.
let _short_unused_closure = | _unused_arg: u8 | println!("not called");
// The closure assignment above is executed, with a line count of `1`, but the `println!()`
// could not have been called, and yet, there is no indication that it wasn't...

// ...but adding block braces gives the expected result, showing the block was not executed.

let short_used_covered_closure_macro = | used_arg: u8 | println!("called");
let short_used_not_covered_closure_macro = | used_arg: u8 | println!("not called");
let _short_unused_closure_macro = | _unused_arg: u8 | println!("not called");




let _short_unused_closure_block = | _unused_arg: u8 | { println!("not called") };

let _shortish_unused_closure = | _unused_arg: u8 | {
Expand All @@ -152,4 +152,64 @@ fn main() {
_unused_arg: u8
| { println!("not called") }
;





let _short_unused_closure_line_break_no_block = | _unused_arg: u8 |
println!("not called")
;

let _short_unused_closure_line_break_no_block2 =
| _unused_arg: u8 |
println!(
"not called"
)
;

let short_used_not_covered_closure_line_break_no_block_embedded_branch =
| _unused_arg: u8 |
println!(
"not called: {}",
if is_true { "check" } else { "me" }
)
;

let short_used_not_covered_closure_line_break_block_embedded_branch =
| _unused_arg: u8 |
{
println!(
"not called: {}",
if is_true { "check" } else { "me" }
)
}
;

let short_used_covered_closure_line_break_no_block_embedded_branch =
| _unused_arg: u8 |
println!(
"not called: {}",
if is_true { "check" } else { "me" }
)
;

let short_used_covered_closure_line_break_block_embedded_branch =
| _unused_arg: u8 |
{
println!(
"not called: {}",
if is_true { "check" } else { "me" }
)
}
;

if is_false {
short_used_not_covered_closure_macro(0);
short_used_not_covered_closure_line_break_no_block_embedded_branch(0);
short_used_not_covered_closure_line_break_block_embedded_branch(0);
}
short_used_covered_closure_macro(0);
short_used_covered_closure_line_break_no_block_embedded_branch(0);
short_used_covered_closure_line_break_block_embedded_branch(0);
}
2 changes: 1 addition & 1 deletion src/test/run-make-fulldeps/coverage/closure_macro.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ macro_rules! bail {

macro_rules! on_error {
($value:expr, $error_message:expr) => {
$value.or_else(|e| {
$value.or_else(|e| { // FIXME(85000): no coverage in closure macros
let message = format!($error_message, e);
if message.len() > 0 {
println!("{}", message);
Expand Down
2 changes: 1 addition & 1 deletion src/test/run-make-fulldeps/coverage/closure_macro_async.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ macro_rules! bail {

macro_rules! on_error {
($value:expr, $error_message:expr) => {
$value.or_else(|e| {
$value.or_else(|e| { // FIXME(85000): no coverage in closure macros
let message = format!($error_message, e);
if message.len() > 0 {
println!("{}", message);
Expand Down