diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index d19fda50bf6c0..30f125ca3beae 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -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; @@ -281,11 +280,8 @@ fn add_unused_functions<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { let mut unused_def_ids_by_file: FxHashMap> = 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); diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async2.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async2.txt index 8a445433ab65f..322f5681b3fd9 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async2.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async2.txt @@ -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; @@ -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|} diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.no_cov_crate.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.no_cov_crate.txt index c4a7b0cc7e9f3..324b9138c4d9c 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.no_cov_crate.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.no_cov_crate.txt @@ -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|} diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.panic_unwind.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.panic_unwind.txt index c77ee5ddc207b..114507dc9fd2a 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.panic_unwind.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.panic_unwind.txt @@ -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. diff --git a/src/test/run-make-fulldeps/coverage/async2.rs b/src/test/run-make-fulldeps/coverage/async2.rs index 6171d95ff5543..959d48ce9db16 100644 --- a/src/test/run-make-fulldeps/coverage/async2.rs +++ b/src/test/run-make-fulldeps/coverage/async2.rs @@ -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; @@ -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"); } diff --git a/src/test/run-make-fulldeps/coverage/no_cov_crate.rs b/src/test/run-make-fulldeps/coverage/no_cov_crate.rs index 300570db7e8f7..6f8586d9f5ca6 100644 --- a/src/test/run-make-fulldeps/coverage/no_cov_crate.rs +++ b/src/test/run-make-fulldeps/coverage/no_cov_crate.rs @@ -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(); } diff --git a/src/test/run-make-fulldeps/coverage/panic_unwind.rs b/src/test/run-make-fulldeps/coverage/panic_unwind.rs index b6c0c080762b2..03128c2cce628 100644 --- a/src/test/run-make-fulldeps/coverage/panic_unwind.rs +++ b/src/test/run-make-fulldeps/coverage/panic_unwind.rs @@ -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.