Rollup merge of rust-lang#120185 - Zalathar:auto-derived, r=wesleywiser

coverage: Don't instrument `#[automatically_derived]` functions This PR makes the coverage instrumentor detect and skip functions that have [`#[automatically_derived]`](https://doc.rust-lang.org/reference/attributes/derive.html#the-automatically_derived-attribute) on their enclosing impl block. Most notably, this means that methods generated by built-in derives (e.g. `Clone`, `Debug`, `PartialEq`) are now ignored by coverage instrumentation, and won't appear as executed or not-executed in coverage reports. This is a noticeable change in user-visible behaviour, but overall I think it's a net improvement. For example, we've had a few user requests for this sort of change (e.g. rust-lang#105055, rust-lang#84605 (comment)), and I believe it's the behaviour that most users will expect/prefer by default. It's possible to imagine situations where users would want to instrument these derived implementations, but I think it's OK to treat that as an opportunity to consider adding more fine-grained option flags to control the details of coverage instrumentation, while leaving this new behaviour as the default. (Also note that while `-Cinstrument-coverage` is a stable feature, the exact details of coverage instrumentation are allowed to change. So we *can* make this change; the main question is whether we *should*.) Fixes rust-lang#105055.
fmease · Jan 24, 2024 · c0e9c75 · c0e9c75
2 parents dceb83c + 41dcba8
commit c0e9c75
Show file tree

Hide file tree

Showing 10 changed files with 17 additions and 97 deletions.
diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs
@@ -384,7 +384,18 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
         return false;
     }
 
+    // Don't instrument functions with `#[automatically_derived]` on their
+    // enclosing impl block, on the assumption that most users won't care about
+    // coverage for derived impls.
+    if let Some(impl_of) = tcx.impl_of_method(def_id.to_def_id())
+        && tcx.is_automatically_derived(impl_of)
+    {
+        trace!("InstrumentCoverage skipped for {def_id:?} (automatically derived)");
+        return false;
+    }
+
     if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
+        trace!("InstrumentCoverage skipped for {def_id:?} (`#[coverage(off)]`)");
         return false;
     }
 

diff --git a/tests/coverage-run-rustdoc/doctest.coverage b/tests/coverage-run-rustdoc/doctest.coverage
@@ -34,8 +34,7 @@ $DIR/doctest.rs:
    LL|       |//!
    LL|       |//! doctest returning a result:
    LL|      1|//! ```
-   LL|      2|//! #[derive(Debug, PartialEq)]
-                       ^1
+   LL|      1|//! #[derive(Debug, PartialEq)]
    LL|      1|//! struct SomeError {
    LL|      1|//!     msg: String,
    LL|      1|//! }
@@ -63,7 +62,7 @@ $DIR/doctest.rs:
    LL|      1|//!     println!("called some_func()");
    LL|      1|//! }
    LL|       |//!
-   LL|      0|//! #[derive(Debug)]
+   LL|       |//! #[derive(Debug)]
    LL|       |//! struct SomeError;
    LL|       |//!
    LL|       |//! extern crate doctest_crate;

diff --git a/tests/coverage/bad_counter_ids.cov-map b/tests/coverage/bad_counter_ids.cov-map
@@ -1,19 +1,3 @@
-Function name: <bad_counter_ids::Foo as core::cmp::PartialEq>::eq
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 0c, 11, 00, 1a]
-Number of files: 1
-- file 0 => global file 1
-Number of expressions: 0
-Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 12, 17) to (start + 0, 26)
-
-Function name: <bad_counter_ids::Foo as core::fmt::Debug>::fmt
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 0c, 0a, 00, 0f]
-Number of files: 1
-- file 0 => global file 1
-Number of expressions: 0
-Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 12, 10) to (start + 0, 15)
-
 Function name: bad_counter_ids::eq_bad
 Raw bytes (14): 0x[01, 01, 00, 02, 01, 23, 01, 02, 1f, 00, 03, 01, 00, 02]
 Number of files: 1

diff --git a/tests/coverage/bad_counter_ids.coverage b/tests/coverage/bad_counter_ids.coverage
@@ -9,7 +9,7 @@
    LL|       |// a too-large counter ID and silently discard the entire function from its
    LL|       |// coverage reports.
    LL|       |
-   LL|      8|#[derive(Debug, PartialEq, Eq)]
+   LL|       |#[derive(Debug, PartialEq, Eq)]
    LL|       |struct Foo(u32);
    LL|       |
    LL|      1|fn eq_good() {

diff --git a/tests/coverage/issue-83601.cov-map b/tests/coverage/issue-83601.cov-map
@@ -1,19 +1,3 @@
-Function name: <issue_83601::Foo as core::cmp::PartialEq>::eq
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 03, 11, 00, 1a]
-Number of files: 1
-- file 0 => global file 1
-Number of expressions: 0
-Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 3, 17) to (start + 0, 26)
-
-Function name: <issue_83601::Foo as core::fmt::Debug>::fmt
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 03, 0a, 00, 0f]
-Number of files: 1
-- file 0 => global file 1
-Number of expressions: 0
-Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 3, 10) to (start + 0, 15)
-
 Function name: issue_83601::main
 Raw bytes (21): 0x[01, 01, 01, 05, 09, 03, 01, 06, 01, 02, 1c, 05, 03, 09, 01, 1c, 02, 02, 05, 03, 02]
 Number of files: 1

diff --git a/tests/coverage/issue-83601.coverage b/tests/coverage/issue-83601.coverage
@@ -1,7 +1,6 @@
    LL|       |// Shows that rust-lang/rust/83601 is resolved
    LL|       |
-   LL|      3|#[derive(Debug, PartialEq, Eq)]
-                              ^2
+   LL|       |#[derive(Debug, PartialEq, Eq)]
    LL|       |struct Foo(u32);
    LL|       |
    LL|      1|fn main() {

diff --git a/tests/coverage/issue-84561.cov-map b/tests/coverage/issue-84561.cov-map
@@ -1,11 +1,3 @@
-Function name: <issue_84561::Foo as core::cmp::PartialEq>::eq
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 04, 0a, 00, 13]
-Number of files: 1
-- file 0 => global file 1
-Number of expressions: 0
-Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 4, 10) to (start + 0, 19)
-
 Function name: <issue_84561::Foo as core::fmt::Debug>::fmt
 Raw bytes (29): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 8a, 01, 05, 01, 25, 05, 01, 25, 00, 26, 02, 01, 09, 00, 0f, 07, 01, 05, 00, 06]
 Number of files: 1

diff --git a/tests/coverage/issue-84561.coverage b/tests/coverage/issue-84561.coverage
@@ -1,7 +1,7 @@
    LL|       |// This demonstrated Issue #84561: function-like macros produce unintuitive coverage results.
    LL|       |
    LL|       |// failure-status: 101
-   LL|     21|#[derive(PartialEq, Eq)]
+   LL|       |#[derive(PartialEq, Eq)]
    LL|       |struct Foo(u32);
    LL|       |
    LL|       |#[rustfmt::skip]

diff --git a/tests/coverage/partial_eq.cov-map b/tests/coverage/partial_eq.cov-map
@@ -1,51 +1,3 @@
-Function name: <partial_eq::Version as core::clone::Clone>::clone (unused)
-Raw bytes (9): 0x[01, 01, 00, 01, 00, 04, 0a, 00, 0f]
-Number of files: 1
-- file 0 => global file 1
-Number of expressions: 0
-Number of file 0 mappings: 1
-- Code(Zero) at (prev + 4, 10) to (start + 0, 15)
-
-Function name: <partial_eq::Version as core::cmp::Ord>::cmp (unused)
-Raw bytes (14): 0x[01, 01, 00, 02, 00, 04, 33, 00, 34, 00, 00, 35, 00, 36]
-Number of files: 1
-- file 0 => global file 1
-Number of expressions: 0
-Number of file 0 mappings: 2
-- Code(Zero) at (prev + 4, 51) to (start + 0, 52)
-- Code(Zero) at (prev + 0, 53) to (start + 0, 54)
-
-Function name: <partial_eq::Version as core::cmp::PartialEq>::eq (unused)
-Raw bytes (14): 0x[01, 01, 00, 02, 00, 04, 18, 00, 19, 00, 00, 20, 00, 21]
-Number of files: 1
-- file 0 => global file 1
-Number of expressions: 0
-Number of file 0 mappings: 2
-- Code(Zero) at (prev + 4, 24) to (start + 0, 25)
-- Code(Zero) at (prev + 0, 32) to (start + 0, 33)
-
-Function name: <partial_eq::Version as core::cmp::PartialOrd>::partial_cmp
-Raw bytes (22): 0x[01, 01, 04, 07, 0b, 00, 09, 0f, 15, 00, 11, 02, 01, 04, 27, 00, 28, 03, 00, 30, 00, 31]
-Number of files: 1
-- file 0 => global file 1
-Number of expressions: 4
-- expression 0 operands: lhs = Expression(1, Add), rhs = Expression(2, Add)
-- expression 1 operands: lhs = Zero, rhs = Counter(2)
-- expression 2 operands: lhs = Expression(3, Add), rhs = Counter(5)
-- expression 3 operands: lhs = Zero, rhs = Counter(4)
-Number of file 0 mappings: 2
-- Code(Counter(0)) at (prev + 4, 39) to (start + 0, 40)
-- Code(Expression(0, Add)) at (prev + 0, 48) to (start + 0, 49)
-    = ((Zero + c2) + ((Zero + c4) + c5))
-
-Function name: <partial_eq::Version as core::fmt::Debug>::fmt
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 04, 11, 00, 16]
-Number of files: 1
-- file 0 => global file 1
-Number of expressions: 0
-Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 4, 17) to (start + 0, 22)
-
 Function name: <partial_eq::Version>::new
 Raw bytes (9): 0x[01, 01, 00, 01, 01, 0c, 05, 06, 06]
 Number of files: 1

diff --git a/tests/coverage/partial_eq.coverage b/tests/coverage/partial_eq.coverage
@@ -1,8 +1,7 @@
    LL|       |// This test confirms an earlier problem was resolved, supporting the MIR graph generated by the
    LL|       |// structure of this test.
    LL|       |
-   LL|      2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
-                       ^0            ^0      ^0     ^1       ^1 ^0^0
+   LL|       |#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
    LL|       |pub struct Version {
    LL|       |    major: usize,
    LL|       |    minor: usize,