Auto merge of #84562 - richkadel:issue-83601, r=tmandry

Adds feature-gated `#[no_coverage]` function attribute, to fix derived Eq `0` coverage issue #83601

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).

Adding a `no_coverage` feature gate with tracking issue #84605.

r? `@tmandry`
cc: `@wesleywiser`
This commit is contained in:
bors 2021-04-28 13:05:16 +00:00
commit 20040fa332
18 changed files with 198 additions and 2 deletions

View File

@ -16,9 +16,19 @@ pub fn expand_deriving_eq(
push: &mut dyn FnMut(Annotatable),
) {
let inline = cx.meta_word(span, sym::inline);
let no_coverage_ident =
rustc_ast::attr::mk_nested_word_item(Ident::new(sym::no_coverage, span));
let no_coverage_feature =
rustc_ast::attr::mk_list_item(Ident::new(sym::feature, span), vec![no_coverage_ident]);
let no_coverage = cx.meta_word(span, sym::no_coverage);
let hidden = rustc_ast::attr::mk_nested_word_item(Ident::new(sym::hidden, span));
let doc = rustc_ast::attr::mk_list_item(Ident::new(sym::doc, span), vec![hidden]);
let attrs = vec![cx.attribute(inline), cx.attribute(doc)];
let attrs = vec![
cx.attribute(inline),
cx.attribute(no_coverage_feature),
cx.attribute(no_coverage),
cx.attribute(doc),
];
let trait_def = TraitDef {
span,
attributes: Vec::new(),

View File

@ -8,6 +8,7 @@
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
use rustc_hir::def_id::{DefId, DefIdSet, LOCAL_CRATE};
use rustc_llvm::RustString;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::mir::coverage::CodeRegion;
use rustc_span::Symbol;
@ -280,6 +281,10 @@ fn add_unused_functions<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) {
let mut unused_def_ids_by_file: FxHashMap<Symbol, Vec<DefId>> = FxHashMap::default();
for &non_codegenned_def_id in all_def_ids.difference(codegenned_def_ids) {
let codegen_fn_attrs = tcx.codegen_fn_attrs(non_codegenned_def_id);
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
continue;
}
// Make sure the non-codegenned (unused) function has a file_name
if let Some(non_codegenned_file_name) = tcx.covered_file_name(non_codegenned_def_id) {
let def_ids =

View File

@ -646,6 +646,10 @@ pub fn set(&self, features: &mut Features, span: Span) {
/// Allows `extern "wasm" fn`
(active, wasm_abi, "1.53.0", Some(83788), None),
/// Allows function attribute `#[no_coverage]`, to bypass coverage
/// instrumentation of that function.
(active, no_coverage, "1.53.0", Some(84605), None),
/// Allows trait bounds in `const fn`.
(active, const_fn_trait_bound, "1.53.0", Some(57563), None),

View File

@ -273,6 +273,13 @@ macro_rules! experimental {
template!(List: "address, memory, thread"),
experimental!(no_sanitize)
),
ungated!(
// Not exclusively gated at the crate level (though crate-level is
// supported). The feature can alternatively be enabled on individual
// functions.
no_coverage, AssumedUsed,
template!(Word),
),
// FIXME: #14408 assume docs are used since rustdoc looks at them.
ungated!(doc, AssumedUsed, template!(List: "hidden|inline|...", NameValueStr: "string")),

View File

@ -89,6 +89,10 @@ pub struct CodegenFnAttrFlags: u32 {
/// #[cmse_nonsecure_entry]: with a TrustZone-M extension, declare a
/// function as an entry function from Non-Secure code.
const CMSE_NONSECURE_ENTRY = 1 << 14;
/// `#[no_coverage]`: indicates that the function should be ignored by
/// the MIR `InstrumentCoverage` pass and not added to the coverage map
/// during codegen.
const NO_COVERAGE = 1 << 15;
}
}

View File

@ -23,6 +23,7 @@
use rustc_middle::hir;
use rustc_middle::hir::map::blocks::FnLikeNode;
use rustc_middle::ich::StableHashingContext;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::mir::coverage::*;
use rustc_middle::mir::{
self, BasicBlock, BasicBlockData, Coverage, SourceInfo, Statement, StatementKind, Terminator,
@ -87,6 +88,11 @@ fn run_pass(&self, tcx: TyCtxt<'tcx>, mir_body: &mut mir::Body<'tcx>) {
_ => {}
}
let codegen_fn_attrs = tcx.codegen_fn_attrs(mir_source.def_id());
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
return;
}
trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());
Instrumentor::new(&self.name(), tcx, mir_body).inject_counters();
trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());

View File

@ -781,6 +781,7 @@
no,
no_builtins,
no_core,
no_coverage,
no_crate_inject,
no_debug,
no_default_passes,

View File

@ -2661,6 +2661,8 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
let mut inline_span = None;
let mut link_ordinal_span = None;
let mut no_sanitize_span = None;
let mut no_coverage_feature_enabled = false;
let mut no_coverage_attr = None;
for attr in attrs.iter() {
if tcx.sess.check_name(attr, sym::cold) {
codegen_fn_attrs.flags |= CodegenFnAttrFlags::COLD;
@ -2724,6 +2726,15 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
codegen_fn_attrs.flags |= CodegenFnAttrFlags::NAKED;
} else if tcx.sess.check_name(attr, sym::no_mangle) {
codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE;
} else if attr.has_name(sym::feature) {
if let Some(list) = attr.meta_item_list() {
if list.iter().any(|nested_meta_item| nested_meta_item.has_name(sym::no_coverage)) {
tcx.sess.mark_attr_used(attr);
no_coverage_feature_enabled = true;
}
}
} else if tcx.sess.check_name(attr, sym::no_coverage) {
no_coverage_attr = Some(attr);
} else if tcx.sess.check_name(attr, sym::rustc_std_internal_symbol) {
codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL;
} else if tcx.sess.check_name(attr, sym::used) {
@ -2934,6 +2945,23 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
}
}
if let Some(no_coverage_attr) = no_coverage_attr {
if tcx.sess.features_untracked().no_coverage || no_coverage_feature_enabled {
codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_COVERAGE
} else {
let mut err = feature_err(
&tcx.sess.parse_sess,
sym::no_coverage,
no_coverage_attr.span,
"the `#[no_coverage]` attribute is an experimental feature",
);
if tcx.sess.parse_sess.unstable_features.is_nightly_build() {
err.help("or, alternatively, add `#[feature(no_coverage)]` to the function");
}
err.emit();
}
}
codegen_fn_attrs.inline = attrs.iter().fold(InlineAttr::None, |ia, attr| {
if !attr.has_name(sym::inline) {
return ia;

View File

@ -274,6 +274,8 @@ pub trait Eq: PartialEq<Self> {
//
// This should never be implemented by hand.
#[doc(hidden)]
#[cfg_attr(not(bootstrap), feature(no_coverage))]
#[cfg_attr(not(bootstrap), no_coverage)]
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
fn assert_receiver_is_total_eq(&self) {}

View File

@ -0,0 +1,22 @@
1| |// Shows that rust-lang/rust/83601 is resolved
2| |
3| 3|#[derive(Debug, PartialEq, Eq)]
^2
------------------
| <issue_83601::Foo as core::cmp::PartialEq>::eq:
| 3| 2|#[derive(Debug, PartialEq, Eq)]
------------------
| Unexecuted instantiation: <issue_83601::Foo as core::cmp::PartialEq>::ne
------------------
4| |struct Foo(u32);
5| |
6| 1|fn main() {
7| 1| let bar = Foo(1);
8| 0| assert_eq!(bar, Foo(1));
9| 1| let baz = Foo(0);
10| 0| assert_ne!(baz, Foo(1));
11| 1| println!("{:?}", Foo(1));
12| 1| println!("{:?}", bar);
13| 1| println!("{:?}", baz);
14| 1|}

View File

@ -0,0 +1,18 @@
1| |// Enables `no_coverage` on the entire crate
2| |#![feature(no_coverage)]
3| |
4| |#[no_coverage]
5| |fn do_not_add_coverage_1() {
6| | println!("called but not covered");
7| |}
8| |
9| |#[no_coverage]
10| |fn do_not_add_coverage_2() {
11| | println!("called but not covered");
12| |}
13| |
14| 1|fn main() {
15| 1| do_not_add_coverage_1();
16| 1| do_not_add_coverage_2();
17| 1|}

View File

@ -0,0 +1,19 @@
1| |// Enables `no_coverage` on individual functions
2| |
3| |#[feature(no_coverage)]
4| |#[no_coverage]
5| |fn do_not_add_coverage_1() {
6| | println!("called but not covered");
7| |}
8| |
9| |#[no_coverage]
10| |#[feature(no_coverage)]
11| |fn do_not_add_coverage_2() {
12| | println!("called but not covered");
13| |}
14| |
15| 1|fn main() {
16| 1| do_not_add_coverage_1();
17| 1| do_not_add_coverage_2();
18| 1|}

View File

@ -2,7 +2,7 @@
2| |// structure of this test.
3| |
4| 2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
^0 ^0 ^0 ^0 ^1 ^1 ^0^0
^0 ^0 ^0 ^1 ^1 ^0^0
------------------
| Unexecuted instantiation: <partial_eq::Version as core::cmp::PartialEq>::ne
------------------

View File

@ -0,0 +1,14 @@
// Shows that rust-lang/rust/83601 is resolved
#[derive(Debug, PartialEq, Eq)]
struct Foo(u32);
fn main() {
let bar = Foo(1);
assert_eq!(bar, Foo(1));
let baz = Foo(0);
assert_ne!(baz, Foo(1));
println!("{:?}", Foo(1));
println!("{:?}", bar);
println!("{:?}", baz);
}

View File

@ -0,0 +1,17 @@
// Enables `no_coverage` on the entire crate
#![feature(no_coverage)]
#[no_coverage]
fn do_not_add_coverage_1() {
println!("called but not covered");
}
#[no_coverage]
fn do_not_add_coverage_2() {
println!("called but not covered");
}
fn main() {
do_not_add_coverage_1();
do_not_add_coverage_2();
}

View File

@ -0,0 +1,18 @@
// Enables `no_coverage` on individual functions
#[feature(no_coverage)]
#[no_coverage]
fn do_not_add_coverage_1() {
println!("called but not covered");
}
#[no_coverage]
#[feature(no_coverage)]
fn do_not_add_coverage_2() {
println!("called but not covered");
}
fn main() {
do_not_add_coverage_1();
do_not_add_coverage_2();
}

View File

@ -0,0 +1,8 @@
#![crate_type = "lib"]
#[no_coverage]
#[feature(no_coverage)] // does not have to be enabled before `#[no_coverage]`
fn no_coverage_is_enabled_on_this_function() {}
#[no_coverage] //~ ERROR the `#[no_coverage]` attribute is an experimental feature
fn requires_feature_no_coverage() {}

View File

@ -0,0 +1,13 @@
error[E0658]: the `#[no_coverage]` attribute is an experimental feature
--> $DIR/feature-gate-no_coverage.rs:7:1
|
LL | #[no_coverage]
| ^^^^^^^^^^^^^^
|
= note: see issue #84605 <https://github.com/rust-lang/rust/issues/84605> for more information
= help: add `#![feature(no_coverage)]` to the crate attributes to enable
= help: or, alternatively, add `#[feature(no_coverage)]` to the function
error: aborting due to previous error
For more information about this error, try `rustc --explain E0658`.