From 448e52d97c9b76600fd1ec29c8159391329e48ab Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Sun, 18 Apr 2021 02:55:17 -0700 Subject: [PATCH] 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. --- compiler/rustc_mir_build/src/build/scope.rs | 10 +++ ....main.SimplifyCfg-promote-consts.after.mir | 4 + .../expected_show_coverage.continue.txt | 75 +++++++++++++++++++ .../expected_show_coverage.uses_crate.txt | 4 +- .../run-make-fulldeps/coverage/continue.rs | 69 +++++++++++++++++ 5 files changed, 160 insertions(+), 2 deletions(-) create mode 100644 src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.continue.txt create mode 100644 src/test/run-make-fulldeps/coverage/continue.rs diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index b637b9b70bdc6..2727e70f278db 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -618,6 +618,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } else { assert!(value.is_none(), "`return` and `break` should have a destination"); + // `continue` statements generate no MIR statement with the `continue` statement's Span, + // and the `InstrumentCoverage` statement will have no way to generate a coverage + // code region for the `continue` statement, unless we add a dummy `Assign` here: + let mut local_decl = LocalDecl::new(self.tcx.mk_unit(), span); + local_decl = local_decl.immutable(); + let temp = self.local_decls.push(local_decl); + let temp_place = Place::from(temp); + self.cfg.push(block, Statement { source_info, kind: StatementKind::StorageLive(temp) }); + self.cfg.push_assign_unit(block, source_info, temp_place, self.tcx); + self.cfg.push(block, Statement { source_info, kind: StatementKind::StorageDead(temp) }); } let region_scope = self.scopes.breakable_scopes[break_index].region_scope; diff --git a/src/test/mir-opt/loop_test.main.SimplifyCfg-promote-consts.after.mir b/src/test/mir-opt/loop_test.main.SimplifyCfg-promote-consts.after.mir index 99c7ac8d5b708..f6120447017d9 100644 --- a/src/test/mir-opt/loop_test.main.SimplifyCfg-promote-consts.after.mir +++ b/src/test/mir-opt/loop_test.main.SimplifyCfg-promote-consts.after.mir @@ -8,6 +8,7 @@ fn main() -> () { let mut _4: !; // in scope 0 at $DIR/loop_test.rs:13:5: 16:6 let mut _5: (); // in scope 0 at $DIR/loop_test.rs:6:1: 17:2 let _6: i32; // in scope 0 at $DIR/loop_test.rs:14:13: 14:14 + let _7: (); // in scope 0 at $DIR/loop_test.rs:15:9: 15:17 scope 1 { debug x => _6; // in scope 1 at $DIR/loop_test.rs:14:13: 14:14 } @@ -42,6 +43,9 @@ fn main() -> () { StorageLive(_6); // scope 0 at $DIR/loop_test.rs:14:13: 14:14 _6 = const 1_i32; // scope 0 at $DIR/loop_test.rs:14:17: 14:18 FakeRead(ForLet(None), _6); // scope 0 at $DIR/loop_test.rs:14:13: 14:14 + StorageLive(_7); // scope 1 at $DIR/loop_test.rs:15:9: 15:17 + _7 = const (); // scope 1 at $DIR/loop_test.rs:15:9: 15:17 + StorageDead(_7); // scope 1 at $DIR/loop_test.rs:15:9: 15:17 StorageDead(_6); // scope 0 at $DIR/loop_test.rs:16:5: 16:6 goto -> bb3; // scope 0 at $DIR/loop_test.rs:1:1: 1:1 } diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.continue.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.continue.txt new file mode 100644 index 0000000000000..28e0f1953e049 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.continue.txt @@ -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|} + diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt index cdcbd8fca9482..f5beb9ef24a0e 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt @@ -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>: + | used_crate::used_only_from_bin_crate_generic_function::<&str>: | 17| 1|pub fn used_only_from_bin_crate_generic_function(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>: | 17| 1|pub fn used_only_from_bin_crate_generic_function(arg: T) { | 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg); | 19| 1|} diff --git a/src/test/run-make-fulldeps/coverage/continue.rs b/src/test/run-make-fulldeps/coverage/continue.rs new file mode 100644 index 0000000000000..624aa98341b80 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage/continue.rs @@ -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; +}