Rollup merge of #84875 - richkadel:no-coverage-dont-check-unused, r=tmandry
Removes unneeded check of `#[no_coverage]` in mapgen There is an anticipated feature request to support a compiler flag that only adds coverage for specific files (or perhaps mods). As I thought about where that change would need to be supported, I realized that checking the attribute in mapgen (for unused functions) was unnecessary. The unused functions are only synthesized if they have MIR coverage, and functions with the `no_coverage` attribute will not have been instrumented with MIR coverage statements in the first place. New tests confirm this. Also, while adding tests, I updated resolved comments and FIXMEs in other tests, and expanded comments and tests on one remaining issue that is still not resolved. r? `@tmandry` cc: `@wesleywiser`
This commit is contained in:
commit
b088318985
@ -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<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);
|
||||
|
@ -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|}
|
||||
|
@ -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|}
|
||||
|
||||
|
@ -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.
|
||||
|
||||
|
@ -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");
|
||||
}
|
||||
|
@ -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();
|
||||
}
|
||||
|
@ -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.
|
||||
|
Loading…
x
Reference in New Issue
Block a user