Skip to content

Commit

Permalink
Auto merge of #84295 - richkadel:continue-coverage, r=tmandry
Browse files Browse the repository at this point in the history
Add coverage to continue statements

`continue` statements were missing coverage. This was particularly
noticeable in a match pattern that contained only a `continue`
statement, leaving the branch appear uncounted. This PR addresses the
problem and adds tests to prove it.

r? `@tmandry`
cc: `@wesleywiser`
  • Loading branch information
bors committed Apr 20, 2021
2 parents e888a57 + d1d7fb1 commit b2c20b5
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 2 deletions.
15 changes: 15 additions & 0 deletions compiler/rustc_mir_build/src/build/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
} else {
assert!(value.is_none(), "`return` and `break` should have a destination");
if self.tcx.sess.instrument_coverage() {
// Unlike `break` and `return`, which push an `Assign` statement to MIR, from which
// a Coverage code region can be generated, `continue` needs no `Assign`; but
// without one, the `InstrumentCoverage` MIR pass cannot generate a code region for
// `continue`. Coverage will be missing unless we add a dummy `Assign` to MIR.
self.add_dummy_assignment(&span, block, source_info);
}
}

let region_scope = self.scopes.breakable_scopes[break_index].region_scope;
Expand All @@ -643,6 +650,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.cfg.start_new_block().unit()
}

// Add a dummy `Assign` statement to the CFG, with the span for the source code's `continue`
// statement.
fn add_dummy_assignment(&mut self, span: &Span, block: BasicBlock, source_info: SourceInfo) {
let local_decl = LocalDecl::new(self.tcx.mk_unit(), *span).internal();
let temp_place = Place::from(self.local_decls.push(local_decl));
self.cfg.push_assign_unit(block, source_info, temp_place, self.tcx);
}

crate fn exit_top_scope(
&mut self,
mut block: BasicBlock,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
1| |#![allow(unused_assignments, unused_variables)]
2| |
3| 1|fn main() {
4| 1| let is_true = std::env::args().len() == 1;
5| 1|
6| 1| let mut x = 0;
7| 11| for _ in 0..10 {
^10
8| 10| match is_true {
9| | true => {
10| 10| continue;
11| | }
12| 0| _ => {
13| 0| x = 1;
14| 0| }
15| 0| }
16| 0| x = 3;
17| | }
18| 11| for _ in 0..10 {
^10
19| 10| match is_true {
20| 0| false => {
21| 0| x = 1;
22| 0| }
23| | _ => {
24| 10| continue;
25| | }
26| | }
27| 0| x = 3;
28| | }
29| 11| for _ in 0..10 {
^10
30| 10| match is_true {
31| 10| true => {
32| 10| x = 1;
33| 10| }
34| | _ => {
35| 0| continue;
36| | }
37| | }
38| 10| x = 3;
39| | }
40| 11| for _ in 0..10 {
^10
41| 10| if is_true {
42| 10| continue;
43| 0| }
44| 0| x = 3;
45| | }
46| 11| for _ in 0..10 {
^10
47| 10| match is_true {
48| 0| false => {
49| 0| x = 1;
50| 0| }
51| 10| _ => {
52| 10| let _ = x;
53| 10| }
54| | }
55| 10| x = 3;
56| | }
57| 1| for _ in 0..10 {
58| 1| match is_true {
59| 0| false => {
60| 0| x = 1;
61| 0| }
62| | _ => {
63| 1| break;
64| | }
65| | }
66| 0| x = 3;
67| | }
68| | let _ = x;
69| 1|}

Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
18| 2| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
19| 2|}
------------------
| used_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
| used_crate::used_only_from_bin_crate_generic_function::<&str>:
| 17| 1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
| 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
| 19| 1|}
------------------
| used_crate::used_only_from_bin_crate_generic_function::<&str>:
| used_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
| 17| 1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
| 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
| 19| 1|}
Expand Down
69 changes: 69 additions & 0 deletions src/test/run-make-fulldeps/coverage/continue.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
#![allow(unused_assignments, unused_variables)]

fn main() {
let is_true = std::env::args().len() == 1;

let mut x = 0;
for _ in 0..10 {
match is_true {
true => {
continue;
}
_ => {
x = 1;
}
}
x = 3;
}
for _ in 0..10 {
match is_true {
false => {
x = 1;
}
_ => {
continue;
}
}
x = 3;
}
for _ in 0..10 {
match is_true {
true => {
x = 1;
}
_ => {
continue;
}
}
x = 3;
}
for _ in 0..10 {
if is_true {
continue;
}
x = 3;
}
for _ in 0..10 {
match is_true {
false => {
x = 1;
}
_ => {
let _ = x;
}
}
x = 3;
}
for _ in 0..10 {
match is_true {
false => {
x = 1;
}
_ => {
break;
}
}
x = 3;
}
let _ = x;
}

0 comments on commit b2c20b5

Please sign in to comment.