mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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

Browse Source 
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. #105055, https://github.com/rust-lang/rust/issues/84605#issuecomment-1902069040), 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 #105055.
This commit is contained in:
León Orell Valerian Liehr 2024-01-24 15:43:12 +01:00 committed by GitHub
commit 8bd126cb18
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 17 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
compiler/rustc_mir_transform/src/coverage/mod.rs
View File

@ -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;
}

5
tests/coverage-run-rustdoc/doctest.coverage
View File

@ -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;

16
tests/coverage/bad_counter_ids.cov-map
View File

@ -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

2
tests/coverage/bad_counter_ids.coverage
View File

@ -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() {

16
tests/coverage/issue-83601.cov-map
View File

@ -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

3
tests/coverage/issue-83601.coverage
View File

@ -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() {

8
tests/coverage/issue-84561.cov-map
View File

@ -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

2
tests/coverage/issue-84561.coverage
View File

@ -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]

48
tests/coverage/partial_eq.cov-map
View File

@ -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

3
tests/coverage/partial_eq.coverage
View File

@ -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,