Skip to content

Add coverage to continue statements #84295

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

Merged
merged 2 commits into from
Apr 20, 2021
Merged

Add coverage to continue statements #84295

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
448e52d
Add coverage to continue statements
richkadel Apr 18, 2021
d1d7fb1
Only generate dummy assign when instrumenting coverage
richkadel Apr 18, 2021
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
15 changes: 15 additions & 0 deletions compiler/rustc_mir_build/src/build/scope.rs
View file
Open in desktop
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`
Copy link
Member

Choose a reason for hiding this comment

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

nit: the function itself doesn't know about 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
75 changes: 75 additions & 0 deletions src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.continue.txt
View file
Open in desktop
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| }
Comment on lines +10 to +15
Copy link
Member

Choose a reason for hiding this comment

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

small detail, but can you think of why the beginning/end of the block have counters for the _ / x = 1 case, but not the continue?

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|}

4 changes: 2 additions & 2 deletions src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt
View file
Open in desktop
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
View file
Open in desktop
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;
}