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`

6 files changed, 54 insertions, 52 deletions

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 8a445433ab6..322f5681b3f 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 c4a7b0cc7e9..324b9138c4d 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 c77ee5ddc20..114507dc9fd 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 6171d95ff55..959d48ce9db 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 300570db7e8..6f8586d9f5c 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 b6c0c080762..03128c2cce6 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.