Skip to content

Commit

Permalink
Auto merge of #114656 - bossmc:rework-no-coverage-attr, r=oli-obk
Browse files Browse the repository at this point in the history
Rework `no_coverage` to `coverage(off)`

As discussed at the tail of #84605 this replaces the `no_coverage` attribute with a `coverage` attribute that takes sub-parameters (currently `off` and `on`) to control the coverage instrumentation.

Allows future-proofing for things like `coverage(off, reason="Tested live", issue="#12345")` or similar.
  • Loading branch information
bors committed Sep 14, 2023
2 parents 8142a31 + 0ca6c38 commit c728bf3
Show file tree
Hide file tree
Showing 38 changed files with 253 additions and 213 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub fn expand_deriving_eq(
attributes: thin_vec![
cx.attr_word(sym::inline, span),
cx.attr_nested_word(sym::doc, sym::hidden, span),
cx.attr_word(sym::no_coverage, span)
cx.attr_nested_word(sym::coverage, sym::off, span)
],
fieldless_variants_strategy: FieldlessVariantsStrategy::Unify,
combine_substructure: combine_substructure(Box::new(|a, b, c| {
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_builtin_macros/src/test_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ fn generate_test_harness(
let expn_id = ext_cx.resolver.expansion_for_ast_pass(
DUMMY_SP,
AstPass::TestHarness,
&[sym::test, sym::rustc_attrs, sym::no_coverage],
&[sym::test, sym::rustc_attrs, sym::coverage_attribute],
None,
);
let def_site = DUMMY_SP.with_def_site_ctxt(expn_id.to_expn_id());
Expand Down Expand Up @@ -335,8 +335,8 @@ fn mk_main(cx: &mut TestCtxt<'_>) -> P<ast::Item> {

// #[rustc_main]
let main_attr = ecx.attr_word(sym::rustc_main, sp);
// #[no_coverage]
let no_coverage_attr = ecx.attr_word(sym::no_coverage, sp);
// #[coverage(off)]
let coverage_attr = ecx.attr_nested_word(sym::coverage, sym::off, sp);

// pub fn main() { ... }
let main_ret_ty = ecx.ty(sp, ast::TyKind::Tup(ThinVec::new()));
Expand Down Expand Up @@ -366,7 +366,7 @@ fn mk_main(cx: &mut TestCtxt<'_>) -> P<ast::Item> {

let main = P(ast::Item {
ident: main_id,
attrs: thin_vec![main_attr, no_coverage_attr],
attrs: thin_vec![main_attr, coverage_attr],
id: ast::DUMMY_NODE_ID,
kind: main,
vis: ast::Visibility { span: sp, kind: ast::VisibilityKind::Public, tokens: None },
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,10 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) {
{
let codegen_fn_attrs = tcx.codegen_fn_attrs(non_codegenned_def_id);

// If a function is marked `#[no_coverage]`, then skip generating a
// If a function is marked `#[coverage(off)]`, then skip generating a
// dead code stub for it.
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
debug!("skipping unused fn marked #[no_coverage]: {:?}", non_codegenned_def_id);
debug!("skipping unused fn marked #[coverage(off)]: {:?}", non_codegenned_def_id);
continue;
}

Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_codegen_ssa/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ codegen_ssa_erroneous_constant = erroneous constant encountered
codegen_ssa_error_creating_remark_dir = failed to create remark directory: {$error}
codegen_ssa_expected_coverage_symbol = expected `coverage(off)` or `coverage(on)`
codegen_ssa_expected_used_symbol = expected `used`, `used(compiler)` or `used(linker)`
codegen_ssa_extern_funcs_not_found = some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified
Expand Down
21 changes: 19 additions & 2 deletions compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ use rustc_target::spec::{abi, SanitizerSet};

use crate::errors;
use crate::target_features::from_target_feature;
use crate::{errors::ExpectedUsedSymbol, target_features::check_target_feature_trait_unsafe};
use crate::{
errors::{ExpectedCoverageSymbol, ExpectedUsedSymbol},
target_features::check_target_feature_trait_unsafe,
};

fn linkage_by_name(tcx: TyCtxt<'_>, def_id: LocalDefId, name: &str) -> Linkage {
use rustc_middle::mir::mono::Linkage::*;
Expand Down Expand Up @@ -128,7 +131,21 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
.emit();
}
}
sym::no_coverage => codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_COVERAGE,
sym::coverage => {
let inner = attr.meta_item_list();
match inner.as_deref() {
Some([item]) if item.has_name(sym::off) => {
codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_COVERAGE;
}
Some([item]) if item.has_name(sym::on) => {
// Allow #[coverage(on)] for being explicit, maybe also in future to enable
// coverage on a smaller scope within an excluded larger scope.
}
Some(_) | None => {
tcx.sess.emit_err(ExpectedCoverageSymbol { span: attr.span });
}
}
}
sym::rustc_std_internal_symbol => {
codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL
}
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,13 @@ pub struct UnknownArchiveKind<'a> {
pub kind: &'a str,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_expected_coverage_symbol)]
pub struct ExpectedCoverageSymbol {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_expected_used_symbol)]
pub struct ExpectedUsedSymbol {
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_error_codes/src/error_codes/E0788.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
A `#[no_coverage]` attribute was applied to something which does not show up
A `#[coverage]` attribute was applied to something which does not show up
in code coverage, or is too granular to be excluded from the coverage report.

For now, this attribute can only be applied to function, method, and closure
Expand All @@ -9,18 +9,18 @@ will just emit an `unused_attributes` lint instead of this error.
Example of erroneous code:

```compile_fail,E0788
#[no_coverage]
#[coverage(off)]
struct Foo;
#[no_coverage]
#[coverage(on)]
const FOO: Foo = Foo;
```

`#[no_coverage]` tells the compiler to not generate coverage instrumentation for
a piece of code when the `-C instrument-coverage` flag is passed. Things like
structs and consts are not coverable code, and thus cannot do anything with this
attribute.
`#[coverage(off)]` tells the compiler to not generate coverage instrumentation
for a piece of code when the `-C instrument-coverage` flag is passed. Things
like structs and consts are not coverable code, and thus cannot do anything
with this attribute.

If you wish to apply this attribute to all methods in an impl or module,
manually annotate each method; it is not possible to annotate the entire impl
with a `#[no_coverage]` attribute.
with a `#[coverage]` attribute.
6 changes: 3 additions & 3 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,9 @@ declare_features! (
(active, const_trait_impl, "1.42.0", Some(67792), None),
/// Allows the `?` operator in const contexts.
(active, const_try, "1.56.0", Some(74935), None),
/// Allows function attribute `#[coverage(on/off)]`, to control coverage
/// instrumentation of that function.
(active, coverage_attribute, "CURRENT_RUSTC_VERSION", Some(84605), None),
/// Allows non-builtin attributes in inner attribute position.
(active, custom_inner_attributes, "1.30.0", Some(54726), None),
/// Allows custom test frameworks with `#![test_runner]` and `#[test_case]`.
Expand Down Expand Up @@ -509,9 +512,6 @@ declare_features! (
(active, never_type_fallback, "1.41.0", Some(65992), None),
/// Allows `#![no_core]`.
(active, no_core, "1.3.0", Some(29639), None),
/// Allows function attribute `#[no_coverage]`, to bypass coverage
/// instrumentation of that function.
(active, no_coverage, "1.53.0", Some(84605), None),
/// Allows the use of `no_sanitize` attribute.
(active, no_sanitize, "1.42.0", Some(39699), None),
/// Allows using the `non_exhaustive_omitted_patterns` lint.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
template!(List: "address, kcfi, memory, thread"), DuplicatesOk,
experimental!(no_sanitize)
),
gated!(no_coverage, Normal, template!(Word), WarnFollowing, experimental!(no_coverage)),
gated!(coverage, Normal, template!(Word, List: "on|off"), WarnFollowing, coverage_attribute, experimental!(coverage)),

ungated!(
doc, Normal, template!(List: "hidden|inline|...", NameValueStr: "string"), DuplicatesOk
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_feature/src/removed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ declare_features! (
Some("subsumed by `#![feature(allocator_internals)]`")),
/// Allows use of unary negate on unsigned integers, e.g., -e for e: u8
(removed, negate_unsigned, "1.0.0", Some(29645), None, None),
/// Allows `#[no_coverage]` on functions.
/// The feature was renamed to `coverage` and the attribute to `#[coverage(on|off)]`
(removed, no_coverage, "CURRENT_RUSTC_VERSION", Some(84605), None, Some("renamed to `coverage`")),
/// Allows `#[no_debug]`.
(removed, no_debug, "1.43.0", Some(29721), None, Some("removed due to lack of demand")),
/// Allows using `#[on_unimplemented(..)]` on traits.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/middle/codegen_fn_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ 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
/// `#[coverage(off)]`: 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
26 changes: 13 additions & 13 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,19 @@ passes_continue_labeled_block =
.label = labeled blocks cannot be `continue`'d
.block_label = labeled block the `continue` points to
passes_coverage_fn_defn =
`#[coverage]` may only be applied to function definitions
passes_coverage_ignored_function_prototype =
`#[coverage]` is ignored on function prototypes
passes_coverage_not_coverable =
`#[coverage]` must be applied to coverable code
.label = not coverable code
passes_coverage_propagate =
`#[coverage]` does not propagate into items and must be applied to the contained functions directly
passes_dead_codes =
{ $multiple ->
*[true] multiple {$descr}s are
Expand Down Expand Up @@ -499,19 +512,6 @@ passes_naked_functions_operands =
passes_naked_tracked_caller =
cannot use `#[track_caller]` with `#[naked]`
passes_no_coverage_fn_defn =
`#[no_coverage]` may only be applied to function definitions
passes_no_coverage_ignored_function_prototype =
`#[no_coverage]` is ignored on function prototypes
passes_no_coverage_not_coverable =
`#[no_coverage]` must be applied to coverable code
.label = not coverable code
passes_no_coverage_propagate =
`#[no_coverage]` does not propagate into items and must be applied to the contained functions directly
passes_no_link =
attribute should be applied to an `extern crate` item
.label = not an `extern crate` item
Expand Down
22 changes: 8 additions & 14 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ impl CheckAttrVisitor<'_> {
match attr.name_or_empty() {
sym::do_not_recommend => self.check_do_not_recommend(attr.span, target),
sym::inline => self.check_inline(hir_id, attr, span, target),
sym::no_coverage => self.check_no_coverage(hir_id, attr, span, target),
sym::coverage => self.check_coverage(hir_id, attr, span, target),
sym::non_exhaustive => self.check_non_exhaustive(hir_id, attr, span, target),
sym::marker => self.check_marker(hir_id, attr, span, target),
sym::target_feature => self.check_target_feature(hir_id, attr, span, target),
Expand Down Expand Up @@ -327,16 +327,10 @@ impl CheckAttrVisitor<'_> {
}
}

/// Checks if a `#[no_coverage]` is applied directly to a function
fn check_no_coverage(
&self,
hir_id: HirId,
attr: &Attribute,
span: Span,
target: Target,
) -> bool {
/// Checks if a `#[coverage]` is applied directly to a function
fn check_coverage(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) -> bool {
match target {
// no_coverage on function is fine
// #[coverage] on function is fine
Target::Fn
| Target::Closure
| Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => true,
Expand All @@ -347,7 +341,7 @@ impl CheckAttrVisitor<'_> {
UNUSED_ATTRIBUTES,
hir_id,
attr.span,
errors::IgnoredNoCoverageFnProto,
errors::IgnoredCoverageFnProto,
);
true
}
Expand All @@ -357,7 +351,7 @@ impl CheckAttrVisitor<'_> {
UNUSED_ATTRIBUTES,
hir_id,
attr.span,
errors::IgnoredNoCoveragePropagate,
errors::IgnoredCoveragePropagate,
);
true
}
Expand All @@ -367,13 +361,13 @@ impl CheckAttrVisitor<'_> {
UNUSED_ATTRIBUTES,
hir_id,
attr.span,
errors::IgnoredNoCoverageFnDefn,
errors::IgnoredCoverageFnDefn,
);
true
}

_ => {
self.tcx.sess.emit_err(errors::IgnoredNoCoverageNotCoverable {
self.tcx.sess.emit_err(errors::IgnoredCoverageNotCoverable {
attr_span: attr.span,
defn_span: span,
});
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,20 +64,20 @@ pub struct InlineNotFnOrClosure {
}

#[derive(LintDiagnostic)]
#[diag(passes_no_coverage_ignored_function_prototype)]
pub struct IgnoredNoCoverageFnProto;
#[diag(passes_coverage_ignored_function_prototype)]
pub struct IgnoredCoverageFnProto;

#[derive(LintDiagnostic)]
#[diag(passes_no_coverage_propagate)]
pub struct IgnoredNoCoveragePropagate;
#[diag(passes_coverage_propagate)]
pub struct IgnoredCoveragePropagate;

#[derive(LintDiagnostic)]
#[diag(passes_no_coverage_fn_defn)]
pub struct IgnoredNoCoverageFnDefn;
#[diag(passes_coverage_fn_defn)]
pub struct IgnoredCoverageFnDefn;

#[derive(Diagnostic)]
#[diag(passes_no_coverage_not_coverable, code = "E0788")]
pub struct IgnoredNoCoverageNotCoverable {
#[diag(passes_coverage_not_coverable, code = "E0788")]
pub struct IgnoredCoverageNotCoverable {
#[primary_span]
pub attr_span: Span,
#[label]
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,8 @@ symbols! {
cosf32,
cosf64,
count,
coverage,
coverage_attribute,
cr,
crate_id,
crate_in_paths,
Expand Down Expand Up @@ -1069,6 +1071,7 @@ symbols! {
note,
object_safe_for_dispatch,
of,
off,
offset,
offset_of,
omit_gdb_pretty_printer_section,
Expand Down
7 changes: 5 additions & 2 deletions library/core/src/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,8 @@ pub trait Eq: PartialEq<Self> {
//
// This should never be implemented by hand.
#[doc(hidden)]
#[no_coverage] // rust-lang/rust#84605
#[cfg_attr(bootstrap, no_coverage)] // rust-lang/rust#84605
#[cfg_attr(not(bootstrap), coverage(off))] //
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
fn assert_receiver_is_total_eq(&self) {}
Expand All @@ -298,7 +299,9 @@ pub trait Eq: PartialEq<Self> {
/// Derive macro generating an impl of the trait [`Eq`].
#[rustc_builtin_macro]
#[stable(feature = "builtin_macro_prelude", since = "1.38.0")]
#[allow_internal_unstable(core_intrinsics, derive_eq, structural_match, no_coverage)]
#[allow_internal_unstable(core_intrinsics, derive_eq, structural_match)]
#[cfg_attr(bootstrap, allow_internal_unstable(no_coverage))]
#[cfg_attr(not(bootstrap), allow_internal_unstable(coverage_attribute))]
pub macro Eq($item:item) {
/* compiler built-in */
}
Expand Down
3 changes: 2 additions & 1 deletion library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@
//
// Library features:
// tidy-alphabetical-start
#![cfg_attr(bootstrap, feature(no_coverage))] // rust-lang/rust#84605
#![cfg_attr(not(bootstrap), feature(coverage_attribute))] // rust-lang/rust#84605
#![feature(char_indices_offset)]
#![feature(const_align_of_val)]
#![feature(const_align_of_val_raw)]
Expand Down Expand Up @@ -235,7 +237,6 @@
#![feature(negative_impls)]
#![feature(never_type)]
#![feature(no_core)]
#![feature(no_coverage)] // rust-lang/rust#84605
#![feature(platform_intrinsics)]
#![feature(prelude_import)]
#![feature(repr_simd)]
Expand Down
4 changes: 2 additions & 2 deletions src/doc/rustc/src/instrument-coverage.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,9 @@ Some of the more notable options in this example include:
[`llvm-cov report`]: https://llvm.org/docs/CommandGuide/llvm-cov.html#llvm-cov-report
[`llvm-cov show`]: https://llvm.org/docs/CommandGuide/llvm-cov.html#llvm-cov-show

> **Note**: Coverage can also be disabled on an individual function by annotating the function with the [`no_coverage` attribute] (which requires the feature flag `#![feature(no_coverage)]`).
> **Note**: Coverage can also be disabled on an individual function by annotating the function with the [`coverage(off)` attribute] (which requires the feature flag `#![feature(coverage)]`).
[`no_coverage` attribute]: ../unstable-book/language-features/no-coverage.html
[`coverage` attribute]: ../unstable-book/language-features/coverage.html

## Interpreting reports

Expand Down
Loading

0 comments on commit c728bf3

Please sign in to comment.