Skip to content

Commit 3653826

Browse files
rust-timerrust-timer
authored
Unrolled build for rust-lang#120185
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.
2 parents cd6d8f2 + 41dcba8 commit 3653826

10 files changed

+17
-97
lines changed

‎compiler/rustc_mir_transform/src/coverage/mod.rs

+11
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,18 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
351351
return false;
352352
}
353353

354+
// Don't instrument functions with `#[automatically_derived]` on their
355+
// enclosing impl block, on the assumption that most users won't care about
356+
// coverage for derived impls.
357+
if let Some(impl_of) = tcx.impl_of_method(def_id.to_def_id())
358+
&& tcx.is_automatically_derived(impl_of)
359+
{
360+
trace!("InstrumentCoverage skipped for {def_id:?} (automatically derived)");
361+
return false;
362+
}
363+
354364
if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
365+
trace!("InstrumentCoverage skipped for {def_id:?} (`#[coverage(off)]`)");
355366
return false;
356367
}
357368

‎tests/coverage-run-rustdoc/doctest.coverage

+2-3
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@ $DIR/doctest.rs:
3434
LL| |//!
3535
LL| |//! doctest returning a result:
3636
LL| 1|//! ```
37-
LL| 2|//! #[derive(Debug, PartialEq)]
38-
^1
37+
LL| 1|//! #[derive(Debug, PartialEq)]
3938
LL| 1|//! struct SomeError {
4039
LL| 1|//! msg: String,
4140
LL| 1|//! }
@@ -63,7 +62,7 @@ $DIR/doctest.rs:
6362
LL| 1|//! println!("called some_func()");
6463
LL| 1|//! }
6564
LL| |//!
66-
LL| 0|//! #[derive(Debug)]
65+
LL| |//! #[derive(Debug)]
6766
LL| |//! struct SomeError;
6867
LL| |//!
6968
LL| |//! extern crate doctest_crate;

‎tests/coverage/bad_counter_ids.cov-map

-16
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,3 @@
1-
Function name: <bad_counter_ids::Foo as core::cmp::PartialEq>::eq
2-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 0c, 11, 00, 1a]
3-
Number of files: 1
4-
- file 0 => global file 1
5-
Number of expressions: 0
6-
Number of file 0 mappings: 1
7-
- Code(Counter(0)) at (prev + 12, 17) to (start + 0, 26)
8-
9-
Function name: <bad_counter_ids::Foo as core::fmt::Debug>::fmt
10-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 0c, 0a, 00, 0f]
11-
Number of files: 1
12-
- file 0 => global file 1
13-
Number of expressions: 0
14-
Number of file 0 mappings: 1
15-
- Code(Counter(0)) at (prev + 12, 10) to (start + 0, 15)
16-
171
Function name: bad_counter_ids::eq_bad
182
Raw bytes (14): 0x[01, 01, 00, 02, 01, 23, 01, 02, 1f, 00, 03, 01, 00, 02]
193
Number of files: 1

‎tests/coverage/bad_counter_ids.coverage

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
LL| |// a too-large counter ID and silently discard the entire function from its
1010
LL| |// coverage reports.
1111
LL| |
12-
LL| 8|#[derive(Debug, PartialEq, Eq)]
12+
LL| |#[derive(Debug, PartialEq, Eq)]
1313
LL| |struct Foo(u32);
1414
LL| |
1515
LL| 1|fn eq_good() {

‎tests/coverage/issue-83601.cov-map

-16
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,3 @@
1-
Function name: <issue_83601::Foo as core::cmp::PartialEq>::eq
2-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 03, 11, 00, 1a]
3-
Number of files: 1
4-
- file 0 => global file 1
5-
Number of expressions: 0
6-
Number of file 0 mappings: 1
7-
- Code(Counter(0)) at (prev + 3, 17) to (start + 0, 26)
8-
9-
Function name: <issue_83601::Foo as core::fmt::Debug>::fmt
10-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 03, 0a, 00, 0f]
11-
Number of files: 1
12-
- file 0 => global file 1
13-
Number of expressions: 0
14-
Number of file 0 mappings: 1
15-
- Code(Counter(0)) at (prev + 3, 10) to (start + 0, 15)
16-
171
Function name: issue_83601::main
182
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]
193
Number of files: 1

‎tests/coverage/issue-83601.coverage

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
LL| |// Shows that rust-lang/rust/83601 is resolved
22
LL| |
3-
LL| 3|#[derive(Debug, PartialEq, Eq)]
4-
^2
3+
LL| |#[derive(Debug, PartialEq, Eq)]
54
LL| |struct Foo(u32);
65
LL| |
76
LL| 1|fn main() {

‎tests/coverage/issue-84561.cov-map

-8
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,3 @@
1-
Function name: <issue_84561::Foo as core::cmp::PartialEq>::eq
2-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 04, 0a, 00, 13]
3-
Number of files: 1
4-
- file 0 => global file 1
5-
Number of expressions: 0
6-
Number of file 0 mappings: 1
7-
- Code(Counter(0)) at (prev + 4, 10) to (start + 0, 19)
8-
91
Function name: <issue_84561::Foo as core::fmt::Debug>::fmt
102
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]
113
Number of files: 1

‎tests/coverage/issue-84561.coverage

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
LL| |// This demonstrated Issue #84561: function-like macros produce unintuitive coverage results.
22
LL| |
33
LL| |// failure-status: 101
4-
LL| 21|#[derive(PartialEq, Eq)]
4+
LL| |#[derive(PartialEq, Eq)]
55
LL| |struct Foo(u32);
66
LL| |
77
LL| |#[rustfmt::skip]

‎tests/coverage/partial_eq.cov-map

-48
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,3 @@
1-
Function name: <partial_eq::Version as core::clone::Clone>::clone (unused)
2-
Raw bytes (9): 0x[01, 01, 00, 01, 00, 04, 0a, 00, 0f]
3-
Number of files: 1
4-
- file 0 => global file 1
5-
Number of expressions: 0
6-
Number of file 0 mappings: 1
7-
- Code(Zero) at (prev + 4, 10) to (start + 0, 15)
8-
9-
Function name: <partial_eq::Version as core::cmp::Ord>::cmp (unused)
10-
Raw bytes (14): 0x[01, 01, 00, 02, 00, 04, 33, 00, 34, 00, 00, 35, 00, 36]
11-
Number of files: 1
12-
- file 0 => global file 1
13-
Number of expressions: 0
14-
Number of file 0 mappings: 2
15-
- Code(Zero) at (prev + 4, 51) to (start + 0, 52)
16-
- Code(Zero) at (prev + 0, 53) to (start + 0, 54)
17-
18-
Function name: <partial_eq::Version as core::cmp::PartialEq>::eq (unused)
19-
Raw bytes (14): 0x[01, 01, 00, 02, 00, 04, 18, 00, 19, 00, 00, 20, 00, 21]
20-
Number of files: 1
21-
- file 0 => global file 1
22-
Number of expressions: 0
23-
Number of file 0 mappings: 2
24-
- Code(Zero) at (prev + 4, 24) to (start + 0, 25)
25-
- Code(Zero) at (prev + 0, 32) to (start + 0, 33)
26-
27-
Function name: <partial_eq::Version as core::cmp::PartialOrd>::partial_cmp
28-
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]
29-
Number of files: 1
30-
- file 0 => global file 1
31-
Number of expressions: 4
32-
- expression 0 operands: lhs = Expression(1, Add), rhs = Expression(2, Add)
33-
- expression 1 operands: lhs = Zero, rhs = Counter(2)
34-
- expression 2 operands: lhs = Expression(3, Add), rhs = Counter(5)
35-
- expression 3 operands: lhs = Zero, rhs = Counter(4)
36-
Number of file 0 mappings: 2
37-
- Code(Counter(0)) at (prev + 4, 39) to (start + 0, 40)
38-
- Code(Expression(0, Add)) at (prev + 0, 48) to (start + 0, 49)
39-
= ((Zero + c2) + ((Zero + c4) + c5))
40-
41-
Function name: <partial_eq::Version as core::fmt::Debug>::fmt
42-
Raw bytes (9): 0x[01, 01, 00, 01, 01, 04, 11, 00, 16]
43-
Number of files: 1
44-
- file 0 => global file 1
45-
Number of expressions: 0
46-
Number of file 0 mappings: 1
47-
- Code(Counter(0)) at (prev + 4, 17) to (start + 0, 22)
48-
491
Function name: <partial_eq::Version>::new
502
Raw bytes (9): 0x[01, 01, 00, 01, 01, 0c, 05, 06, 06]
513
Number of files: 1

‎tests/coverage/partial_eq.coverage

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
LL| |// This test confirms an earlier problem was resolved, supporting the MIR graph generated by the
22
LL| |// structure of this test.
33
LL| |
4-
LL| 2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
5-
^0 ^0 ^0 ^1 ^1 ^0^0
4+
LL| |#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
65
LL| |pub struct Version {
76
LL| | major: usize,
87
LL| | minor: usize,

0 commit comments

Comments
 (0)