Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Apr 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
];
richkadel marked this conversation as resolved.
Show resolved Hide resolved
let trait_def = TraitDef {
span,
attributes: Vec::new(),
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use rustc_codegen_ssa::traits::{ConstMethods, CoverageInfoMethods};
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;

Expand Down Expand Up @@ -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 =
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,10 @@ declare_features! (
/// 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),

Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,13 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
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")),
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/middle/codegen_fn_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ bitflags! {
/// #[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;
}
}

Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_mir/src/transform/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use rustc_index::vec::IndexVec;
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,
Expand Down Expand Up @@ -87,6 +88,11 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage {
_ => {}
}

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());
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,7 @@ symbols! {
no,
no_builtins,
no_core,
no_coverage,
no_crate_inject,
no_debug,
no_default_passes,
Expand Down
28 changes: 28 additions & 0 deletions compiler/rustc_typeck/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Comment on lines +2731 to +2734
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't AssumeUsed make this unnecessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it appears to be necessary. I tried removing it, and when compiling rustc I now get the error:


error: unused attribute
   --> library/core/src/cmp.rs:277:32
    |
277 |     #[cfg_attr(not(bootstrap), feature(no_coverage))]
    |                                ^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D unused-attributes` implied by `-D warnings`

}
} 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) {
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
richkadel marked this conversation as resolved.
Show resolved Hide resolved
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
fn assert_receiver_is_total_eq(&self) {}
Expand Down
Original file line number Diff line number Diff line change
@@ -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|}

Original file line number Diff line number Diff line change
@@ -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|}

Original file line number Diff line number Diff line change
@@ -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|}

Original file line number Diff line number Diff line change
Expand Up @@ -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
------------------
Expand Down
14 changes: 14 additions & 0 deletions src/test/run-make-fulldeps/coverage/issue-83601.rs
Original file line number Diff line number Diff line change
@@ -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);
}
17 changes: 17 additions & 0 deletions src/test/run-make-fulldeps/coverage/no_cov_crate.rs
Original file line number Diff line number Diff line change
@@ -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();
}
18 changes: 18 additions & 0 deletions src/test/run-make-fulldeps/coverage/no_cov_func.rs
Original file line number Diff line number Diff line change
@@ -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();
}
8 changes: 8 additions & 0 deletions src/test/ui/feature-gates/feature-gate-no_coverage.rs
Original file line number Diff line number Diff line change
@@ -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() {}
13 changes: 13 additions & 0 deletions src/test/ui/feature-gates/feature-gate-no_coverage.stderr
Original file line number Diff line number Diff line change
@@ -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`.