Skip to content

Commit

Permalink
Rollup merge of rust-lang#120185 - Zalathar:auto-derived, r=oli-obk
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
TaKO8Ki committed Jan 21, 2024
2 parents be401a7 + 7263a76 commit 96131e9
Show file tree
Hide file tree
Showing 9 changed files with 15 additions and 94 deletions.
11 changes: 11 additions & 0 deletions compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,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;
}

Expand Down
16 changes: 0 additions & 16 deletions tests/coverage/bad_counter_ids.cov-map
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/coverage/bad_counter_ids.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
16 changes: 0 additions & 16 deletions tests/coverage/issue-83601.cov-map
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 1 addition & 2 deletions tests/coverage/issue-83601.coverage
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down
8 changes: 0 additions & 8 deletions tests/coverage/issue-84561.cov-map
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/coverage/issue-84561.coverage
Original file line number Diff line number Diff line change
@@ -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]
Expand Down
48 changes: 0 additions & 48 deletions tests/coverage/partial_eq.cov-map
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 1 addition & 2 deletions tests/coverage/partial_eq.coverage
Original file line number Diff line number Diff line change
@@ -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,
Expand Down

0 comments on commit 96131e9

Please sign in to comment.