Skip to content

Commit 888d0b4

Browse files
committed
Derived Eq no longer shows uncovered
The Eq trait has a special hidden function. MIR `InstrumentCoverage` would add this function to the coverage map, but it is never called, so the `Eq` trait would always appear uncovered. Fixes: #83601 The fix required creating a new function attribute `no_coverage` to mark functions that should be ignored by `InstrumentCoverage` and the coverage `mapgen` (during codegen). While testing, I also noticed two other issues: * spanview debug file output ICEd on a function with no body. The workaround for this is included in this PR. * `assert_*!()` macro coverage can appear covered if followed by another `assert_*!()` macro. Normally they appear uncovered. I submitted a new Issue #84561, and added a coverage test to demonstrate this issue.
1 parent 1919b3f commit 888d0b4

File tree

13 files changed

+119
-2
lines changed

13 files changed

+119
-2
lines changed

compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@ pub fn expand_deriving_eq(
1616
push: &mut dyn FnMut(Annotatable),
1717
) {
1818
let inline = cx.meta_word(span, sym::inline);
19+
let no_coverage = cx.meta_word(span, sym::no_coverage);
1920
let hidden = rustc_ast::attr::mk_nested_word_item(Ident::new(sym::hidden, span));
2021
let doc = rustc_ast::attr::mk_list_item(Ident::new(sym::doc, span), vec![hidden]);
21-
let attrs = vec![cx.attribute(inline), cx.attribute(doc)];
22+
let attrs = vec![cx.attribute(inline), cx.attribute(no_coverage), cx.attribute(doc)];
2223
let trait_def = TraitDef {
2324
span,
2425
attributes: Vec::new(),

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

+5
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use rustc_codegen_ssa::traits::{ConstMethods, CoverageInfoMethods};
88
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
99
use rustc_hir::def_id::{DefId, DefIdSet, LOCAL_CRATE};
1010
use rustc_llvm::RustString;
11+
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
1112
use rustc_middle::mir::coverage::CodeRegion;
1213
use rustc_span::Symbol;
1314

@@ -280,6 +281,10 @@ fn add_unused_functions<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) {
280281

281282
let mut unused_def_ids_by_file: FxHashMap<Symbol, Vec<DefId>> = FxHashMap::default();
282283
for &non_codegenned_def_id in all_def_ids.difference(codegenned_def_ids) {
284+
let codegen_fn_attrs = tcx.codegen_fn_attrs(non_codegenned_def_id);
285+
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
286+
continue;
287+
}
283288
// Make sure the non-codegenned (unused) function has a file_name
284289
if let Some(non_codegenned_file_name) = tcx.covered_file_name(non_codegenned_def_id) {
285290
let def_ids =

compiler/rustc_feature/src/builtin_attrs.rs

+1
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
264264

265265
// Code generation:
266266
ungated!(inline, AssumedUsed, template!(Word, List: "always|never")),
267+
ungated!(no_coverage, AssumedUsed, template!(Word)),
267268
ungated!(cold, AssumedUsed, template!(Word)),
268269
ungated!(no_builtins, AssumedUsed, template!(Word)),
269270
ungated!(target_feature, AssumedUsed, template!(List: r#"enable = "name""#)),

compiler/rustc_middle/src/middle/codegen_fn_attrs.rs

+4
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ bitflags! {
8989
/// #[cmse_nonsecure_entry]: with a TrustZone-M extension, declare a
9090
/// function as an entry function from Non-Secure code.
9191
const CMSE_NONSECURE_ENTRY = 1 << 14;
92+
/// `#[no_coverage]`: indicates that the function should be ignored by
93+
/// the MIR `InstrumentCoverage` pass and not added to the coverage map
94+
/// during codegen.
95+
const NO_COVERAGE = 1 << 15;
9296
}
9397
}
9498

compiler/rustc_mir/src/transform/coverage/mod.rs

+6
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use rustc_index::vec::IndexVec;
2323
use rustc_middle::hir;
2424
use rustc_middle::hir::map::blocks::FnLikeNode;
2525
use rustc_middle::ich::StableHashingContext;
26+
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
2627
use rustc_middle::mir::coverage::*;
2728
use rustc_middle::mir::{
2829
self, BasicBlock, BasicBlockData, Coverage, SourceInfo, Statement, StatementKind, Terminator,
@@ -87,6 +88,11 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage {
8788
_ => {}
8889
}
8990

91+
let codegen_fn_attrs = tcx.codegen_fn_attrs(mir_source.def_id());
92+
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
93+
return;
94+
}
95+
9096
trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());
9197
Instrumentor::new(&self.name(), tcx, mir_body).inject_counters();
9298
trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());

compiler/rustc_span/src/symbol.rs

+1
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,7 @@ symbols! {
781781
no,
782782
no_builtins,
783783
no_core,
784+
no_coverage,
784785
no_crate_inject,
785786
no_debug,
786787
no_default_passes,

compiler/rustc_typeck/src/collect.rs

+2
Original file line numberDiff line numberDiff line change
@@ -2724,6 +2724,8 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
27242724
codegen_fn_attrs.flags |= CodegenFnAttrFlags::NAKED;
27252725
} else if tcx.sess.check_name(attr, sym::no_mangle) {
27262726
codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE;
2727+
} else if tcx.sess.check_name(attr, sym::no_coverage) {
2728+
codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_COVERAGE;
27272729
} else if tcx.sess.check_name(attr, sym::rustc_std_internal_symbol) {
27282730
codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL;
27292731
} else if tcx.sess.check_name(attr, sym::used) {

library/core/src/cmp.rs

+1
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ pub trait Eq: PartialEq<Self> {
274274
//
275275
// This should never be implemented by hand.
276276
#[doc(hidden)]
277+
#[cfg_attr(not(bootstrap), no_coverage)]
277278
#[inline]
278279
#[stable(feature = "rust1", since = "1.0.0")]
279280
fn assert_receiver_is_total_eq(&self) {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
1| |// Shows that rust-lang/rust/83601 is resolved
2+
2| |
3+
3| 3|#[derive(Debug, PartialEq, Eq)]
4+
^2
5+
------------------
6+
| <issue_83601::Foo as core::cmp::PartialEq>::eq:
7+
| 3| 2|#[derive(Debug, PartialEq, Eq)]
8+
------------------
9+
| Unexecuted instantiation: <issue_83601::Foo as core::cmp::PartialEq>::ne
10+
------------------
11+
4| |struct Foo(u32);
12+
5| |
13+
6| 1|fn main() {
14+
7| 1| let bar = Foo(1);
15+
8| 0| assert_eq!(bar, Foo(1));
16+
9| 1| let baz = Foo(0);
17+
10| 0| assert_ne!(baz, Foo(1));
18+
11| 1| println!("{:?}", Foo(1));
19+
12| 1| println!("{:?}", bar);
20+
13| 1| println!("{:?}", baz);
21+
14| 1|}
22+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
1| |// FIXME(#84561): function-like macros produce unintuitive coverage results.
2+
2| |// This test demonstrates some of the problems.
3+
3| |
4+
4| 9|#[derive(Debug, PartialEq, Eq)]
5+
^5
6+
------------------
7+
| <issue_84561::Foo as core::cmp::PartialEq>::eq:
8+
| 4| 9|#[derive(Debug, PartialEq, Eq)]
9+
------------------
10+
| Unexecuted instantiation: <issue_84561::Foo as core::cmp::PartialEq>::ne
11+
------------------
12+
5| |struct Foo(u32);
13+
6| |
14+
7| 1|fn main() {
15+
8| 1| let bar = Foo(1);
16+
9| 0| assert_eq!(bar, Foo(1));
17+
10| 1| let baz = Foo(0);
18+
11| 0| assert_ne!(baz, Foo(1));
19+
12| 1| println!("{:?}", Foo(1));
20+
13| 1| println!("{:?}", bar);
21+
14| 1| println!("{:?}", baz);
22+
15| |
23+
16| 1| assert_eq!(Foo(1), Foo(1));
24+
17| 1| assert_ne!(Foo(0), Foo(1));
25+
18| 0| assert_eq!(Foo(2), Foo(2));
26+
19| 1| let bar = Foo(1);
27+
20| 1| assert_ne!(Foo(0), Foo(3));
28+
21| 1| assert_ne!(Foo(0), Foo(4));
29+
22| 1| assert_eq!(Foo(3), Foo(3));
30+
23| 0| assert_ne!(Foo(0), Foo(5));
31+
24| 1| println!("{:?}", bar);
32+
25| 1| println!("{:?}", Foo(1));
33+
26| 1|}
34+

src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.partial_eq.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
2| |// structure of this test.
33
3| |
44
4| 2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
5-
^0 ^0 ^0 ^0 ^1 ^1 ^0^0
5+
^0 ^0 ^0 ^1 ^1 ^0^0
66
------------------
77
| Unexecuted instantiation: <partial_eq::Version as core::cmp::PartialEq>::ne
88
------------------
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Shows that rust-lang/rust/83601 is resolved
2+
3+
#[derive(Debug, PartialEq, Eq)]
4+
struct Foo(u32);
5+
6+
fn main() {
7+
let bar = Foo(1);
8+
assert_eq!(bar, Foo(1));
9+
let baz = Foo(0);
10+
assert_ne!(baz, Foo(1));
11+
println!("{:?}", Foo(1));
12+
println!("{:?}", bar);
13+
println!("{:?}", baz);
14+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// FIXME(#84561): function-like macros produce unintuitive coverage results.
2+
// This test demonstrates some of the problems.
3+
4+
#[derive(Debug, PartialEq, Eq)]
5+
struct Foo(u32);
6+
7+
fn main() {
8+
let bar = Foo(1);
9+
assert_eq!(bar, Foo(1));
10+
let baz = Foo(0);
11+
assert_ne!(baz, Foo(1));
12+
println!("{:?}", Foo(1));
13+
println!("{:?}", bar);
14+
println!("{:?}", baz);
15+
16+
assert_eq!(Foo(1), Foo(1));
17+
assert_ne!(Foo(0), Foo(1));
18+
assert_eq!(Foo(2), Foo(2));
19+
let bar = Foo(1);
20+
assert_ne!(Foo(0), Foo(3));
21+
assert_ne!(Foo(0), Foo(4));
22+
assert_eq!(Foo(3), Foo(3));
23+
assert_ne!(Foo(0), Foo(5));
24+
println!("{:?}", bar);
25+
println!("{:?}", Foo(1));
26+
}

0 commit comments

Comments
 (0)