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

Rework no_coverage to coverage(off) #114656

Merged
merged 5 commits into from
Sep 14, 2023
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
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 @@ -317,10 +317,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),
bossmc marked this conversation as resolved.
Show resolved Hide resolved
/// 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`")),
Comment on lines +139 to +141
Copy link
Contributor

Choose a reason for hiding this comment

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

I only just noticed that this wasn't updated when the feature flag was renamed from coverage to coverage_attribute.

(This also affects the relevant UI tests.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I've filed PR #115832.

/// 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 @@ -113,6 +113,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 @@ -497,19 +510,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 @@ -573,6 +573,8 @@ symbols! {
cosf32,
cosf64,
count,
coverage,
coverage_attribute,
cr,
crate_id,
crate_in_paths,
Expand Down Expand Up @@ -1068,6 +1070,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
bossmc marked this conversation as resolved.
Show resolved Hide resolved
#[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
Loading