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.

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

Removes unneeded check of #[no_coverage] in mapgen #84875

Merged
merged 1 commit into from
May 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 2 additions & 6 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use rustc_codegen_ssa::traits::{ConstMethods, CoverageInfoMethods};
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
use rustc_hir::def_id::{DefId, DefIdSet, LOCAL_CRATE};
use rustc_llvm::RustString;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::mir::coverage::CodeRegion;
use rustc_span::Symbol;

Expand Down Expand Up @@ -281,11 +280,8 @@ fn add_unused_functions<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) {

let mut unused_def_ids_by_file: FxHashMap<Symbol, Vec<DefId>> = FxHashMap::default();
for &non_codegenned_def_id in all_def_ids.difference(codegenned_def_ids) {
let codegen_fn_attrs = tcx.codegen_fn_attrs(non_codegenned_def_id);
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
continue;
}
// Make sure the non-codegenned (unused) function has a file_name
// Make sure the non-codegenned (unused) function has at least one MIR
// `Coverage` statement with a code region, and return its file name.
if let Some(non_codegenned_file_name) = tcx.covered_file_name(non_codegenned_def_id) {
let def_ids =
unused_def_ids_by_file.entry(*non_codegenned_file_name).or_insert_with(Vec::new);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
14| 1| }
15| 1|}
16| |
17| |// FIXME(#83985): The auto-generated closure in an async function is failing to include
18| |// the println!() and `let` assignment lines in the coverage code region(s), as it does in the
19| |// non-async function above, unless the `println!()` is inside a covered block.
17| |
18| |
19| |
20| 1|async fn async_func() {
21| 1| println!("async_func was covered");
22| 1| let b = true;
Expand All @@ -26,9 +26,9 @@
^0
26| 1|}
27| |
28| |// FIXME(#83985): As above, this async function only has the `println!()` macro call, which is not
29| |// showing coverage, so the entire async closure _appears_ uncovered; but this is not exactly true.
30| |// It's only certain kinds of lines and/or their context that results in missing coverage.
28| |
29| |
30| |
31| 1|async fn async_func_just_println() {
32| 1| println!("async_func_just_println was covered");
33| 1|}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,27 @@
11| | println!("called but not covered");
12| |}
13| |
14| 1|fn main() {
15| 1| do_not_add_coverage_1();
16| 1| do_not_add_coverage_2();
17| 1|}
14| |#[no_coverage]
15| |fn do_not_add_coverage_not_called() {
16| | println!("not called and not covered");
17| |}
18| |
19| 1|fn add_coverage_1() {
20| 1| println!("called and covered");
21| 1|}
22| |
23| 1|fn add_coverage_2() {
24| 1| println!("called and covered");
25| 1|}
26| |
27| 0|fn add_coverage_not_called() {
28| 0| println!("not called but covered");
29| 0|}
30| |
31| 1|fn main() {
32| 1| do_not_add_coverage_1();
33| 1| do_not_add_coverage_2();
34| 1| add_coverage_1();
35| 1| add_coverage_2();
36| 1|}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,4 @@
29| |// 2. Since the `panic_unwind.rs` test is allowed to unwind, it is also allowed to execute the
30| |// normal program exit cleanup, including writing out the current values of the coverage
31| |// counters.
32| |// 3. The coverage results show (interestingly) that the `panic!()` call did execute, but it does
33| |// not show coverage of the `if countdown == 1` branch in `main()` that calls
34| |// `might_panic(true)` (causing the call to `panic!()`).
35| |// 4. The reason `main()`s `if countdown == 1` branch, calling `might_panic(true)`, appears
36| |// "uncovered" is, InstrumentCoverage (intentionally) treats `TerminatorKind::Call` terminators
37| |// as non-branching, because when a program executes normally, they always are. Errors handled
38| |// via the try `?` operator produce error handling branches that *are* treated as branches in
39| |// coverage results. By treating calls without try `?` operators as non-branching (assumed to
40| |// return normally and continue) the coverage graph can be simplified, producing smaller,
41| |// faster binaries, and cleaner coverage results.
42| |// 5. The reason the coverage results actually show `panic!()` was called is most likely because
43| |// `panic!()` is a macro, not a simple function call, and there are other `Statement`s and/or
44| |// `Terminator`s that execute with a coverage counter before the panic and unwind occur.
45| |// 6. Since the common practice is not to use `panic!()` for error handling, the coverage
46| |// implementation avoids incurring an additional cost (in program size and execution time) to
47| |// improve coverage results for an event that is generally not "supposed" to happen.
48| |// 7. FIXME(#78544): This issue describes a feature request for a proposed option to enable
49| |// more accurate coverage results for tests that intentionally panic.

12 changes: 6 additions & 6 deletions src/test/run-make-fulldeps/coverage/async2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ fn non_async_func() {
}
}

// FIXME(#83985): The auto-generated closure in an async function is failing to include
// the println!() and `let` assignment lines in the coverage code region(s), as it does in the
// non-async function above, unless the `println!()` is inside a covered block.



async fn async_func() {
println!("async_func was covered");
let b = true;
Expand All @@ -25,9 +25,9 @@ async fn async_func() {
}
}

// FIXME(#83985): As above, this async function only has the `println!()` macro call, which is not
// showing coverage, so the entire async closure _appears_ uncovered; but this is not exactly true.
// It's only certain kinds of lines and/or their context that results in missing coverage.



async fn async_func_just_println() {
println!("async_func_just_println was covered");
}
Expand Down
19 changes: 19 additions & 0 deletions src/test/run-make-fulldeps/coverage/no_cov_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,26 @@ fn do_not_add_coverage_2() {
println!("called but not covered");
}

#[no_coverage]
fn do_not_add_coverage_not_called() {
println!("not called and not covered");
}

fn add_coverage_1() {
println!("called and covered");
}

fn add_coverage_2() {
println!("called and covered");
}

fn add_coverage_not_called() {
println!("not called but covered");
}

fn main() {
do_not_add_coverage_1();
do_not_add_coverage_2();
add_coverage_1();
add_coverage_2();
}
18 changes: 0 additions & 18 deletions src/test/run-make-fulldeps/coverage/no_cov_func.rs

This file was deleted.

18 changes: 0 additions & 18 deletions src/test/run-make-fulldeps/coverage/panic_unwind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,3 @@ fn main() -> Result<(), u8> {
// 2. Since the `panic_unwind.rs` test is allowed to unwind, it is also allowed to execute the
// normal program exit cleanup, including writing out the current values of the coverage
// counters.
// 3. The coverage results show (interestingly) that the `panic!()` call did execute, but it does
// not show coverage of the `if countdown == 1` branch in `main()` that calls
// `might_panic(true)` (causing the call to `panic!()`).
// 4. The reason `main()`s `if countdown == 1` branch, calling `might_panic(true)`, appears
// "uncovered" is, InstrumentCoverage (intentionally) treats `TerminatorKind::Call` terminators
// as non-branching, because when a program executes normally, they always are. Errors handled
// via the try `?` operator produce error handling branches that *are* treated as branches in
// coverage results. By treating calls without try `?` operators as non-branching (assumed to
// return normally and continue) the coverage graph can be simplified, producing smaller,
// faster binaries, and cleaner coverage results.
// 5. The reason the coverage results actually show `panic!()` was called is most likely because
// `panic!()` is a macro, not a simple function call, and there are other `Statement`s and/or
// `Terminator`s that execute with a coverage counter before the panic and unwind occur.
// 6. Since the common practice is not to use `panic!()` for error handling, the coverage
// implementation avoids incurring an additional cost (in program size and execution time) to
// improve coverage results for an event that is generally not "supposed" to happen.
// 7. FIXME(#78544): This issue describes a feature request for a proposed option to enable
// more accurate coverage results for tests that intentionally panic.