Skip to content

Commit

Permalink
Unrolled build for rust-lang#134750
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#134750 - Zalathar:coverage-attr-errors, r=jieyouxu

Update `#[coverage(..)]` attribute error messages to match the current implementation

The allowed positions for `#[coverage(..)]` attributes were expanded by rust-lang#126721, but the corresponding error messages were never updated to reflect the new behaviour.

Part of rust-lang#134749.
  • Loading branch information
rust-timer authored Dec 25, 2024
2 parents a0a5c42 + e48fc62 commit 1a48e23
Show file tree
Hide file tree
Showing 12 changed files with 431 additions and 246 deletions.
36 changes: 17 additions & 19 deletions compiler/rustc_error_codes/src/error_codes/E0788.md
Original file line number Diff line number Diff line change
@@ -1,26 +1,24 @@
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.
A `#[coverage(off|on)]` attribute was found in a position where it is not
allowed.

For now, this attribute can only be applied to function, method, and closure
definitions. In the future, it may be added to statements, blocks, and
expressions, and for the time being, using this attribute in those places
will just emit an `unused_attributes` lint instead of this error.
Coverage attributes can be applied to:
- Function and method declarations that have a body, including trait methods
that have a default implementation.
- Closure expressions, in situations where attributes can be applied to
expressions.
- `impl` blocks (inherent or trait), and modules.

Example of erroneous code:

```compile_fail,E0788
#[coverage(off)]
struct Foo;
#[coverage(on)]
const FOO: Foo = Foo;
unsafe extern "C" {
#[coverage(off)]
fn foreign_fn();
}
```

`#[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 `#[coverage]` attribute.
When using the `-C instrument-coverage` flag, coverage attributes act as a
hint to the compiler that it should instrument or not instrument the
corresponding function or enclosed functions. The precise effect of applying
a coverage attribute is not guaranteed and may change in future compiler
versions.
8 changes: 5 additions & 3 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,11 @@ passes_coroutine_on_non_closure =
attribute should be applied to closures
.label = not a closure
passes_coverage_not_fn_or_closure =
attribute should be applied to a function definition or closure
.label = not a function or closure
passes_coverage_attribute_not_allowed =
coverage attribute not allowed here
.not_fn_impl_mod = not a function, impl block, or module
.no_body = function has no body
.help = coverage attribute can be applied to a function (with body), impl block, or module
passes_dead_codes =
{ $multiple ->
Expand Down
25 changes: 19 additions & 6 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,21 +432,34 @@ impl<'tcx> CheckAttrVisitor<'tcx> {

/// Checks that `#[coverage(..)]` is applied to a function/closure/method,
/// or to an impl block or module.
fn check_coverage(&self, attr: &Attribute, span: Span, target: Target) {
fn check_coverage(&self, attr: &Attribute, target_span: Span, target: Target) {
let mut not_fn_impl_mod = None;
let mut no_body = None;

match target {
Target::Fn
| Target::Closure
| Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent)
| Target::Impl
| Target::Mod => {}
| Target::Mod => return,

// These are "functions", but they aren't allowed because they don't
// have a body, so the usual explanation would be confusing.
Target::Method(MethodKind::Trait { body: false }) | Target::ForeignFn => {
no_body = Some(target_span);
}

_ => {
self.dcx().emit_err(errors::CoverageNotFnOrClosure {
attr_span: attr.span,
defn_span: span,
});
not_fn_impl_mod = Some(target_span);
}
}

self.dcx().emit_err(errors::CoverageAttributeNotAllowed {
attr_span: attr.span,
not_fn_impl_mod,
no_body,
help: (),
});
}

/// Checks that `#[optimize(..)]` is applied to a function/closure/method,
Expand Down
16 changes: 12 additions & 4 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,21 @@ pub(crate) struct InlineNotFnOrClosure {
pub defn_span: Span,
}

/// "coverage attribute not allowed here"
#[derive(Diagnostic)]
#[diag(passes_coverage_not_fn_or_closure, code = E0788)]
pub(crate) struct CoverageNotFnOrClosure {
#[diag(passes_coverage_attribute_not_allowed, code = E0788)]
pub(crate) struct CoverageAttributeNotAllowed {
#[primary_span]
pub attr_span: Span,
#[label]
pub defn_span: Span,
/// "not a function, impl block, or module"
#[label(passes_not_fn_impl_mod)]
pub not_fn_impl_mod: Option<Span>,
/// "function has no body"
#[label(passes_no_body)]
pub no_body: Option<Span>,
/// "coverage attribute can be applied to a function (with body), impl block, or module"
#[help]
pub help: (),
}

#[derive(Diagnostic)]
Expand Down
116 changes: 116 additions & 0 deletions tests/ui/coverage-attr/allowed-positions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
//! Tests where the `#[coverage(..)]` attribute can and cannot be used.
//@ reference: attributes.coverage.allowed-positions

#![feature(coverage_attribute)]
#![feature(extern_types)]
#![feature(impl_trait_in_assoc_type)]
#![warn(unused_attributes)]
#![coverage(off)]

#[coverage(off)]
mod submod {}

#[coverage(off)] //~ ERROR coverage attribute not allowed here [E0788]
type MyTypeAlias = ();

#[coverage(off)] //~ ERROR [E0788]
trait MyTrait {
#[coverage(off)] //~ ERROR [E0788]
const TRAIT_ASSOC_CONST: u32;

#[coverage(off)] //~ ERROR [E0788]
type TraitAssocType;

#[coverage(off)] //~ ERROR [E0788]
fn trait_method(&self);

#[coverage(off)]
fn trait_method_with_default(&self) {}

#[coverage(off)] //~ ERROR [E0788]
fn trait_assoc_fn();
}

#[coverage(off)]
impl MyTrait for () {
const TRAIT_ASSOC_CONST: u32 = 0;

#[coverage(off)] //~ ERROR [E0788]
type TraitAssocType = Self;

#[coverage(off)]
fn trait_method(&self) {}
#[coverage(off)]
fn trait_method_with_default(&self) {}
#[coverage(off)]
fn trait_assoc_fn() {}
}

trait HasAssocType {
type T;
fn constrain_assoc_type() -> Self::T;
}

impl HasAssocType for () {
#[coverage(off)] //~ ERROR [E0788]
type T = impl Copy;
fn constrain_assoc_type() -> Self::T {}
}

#[coverage(off)] //~ ERROR [E0788]
struct MyStruct {
#[coverage(off)] //~ ERROR [E0788]
field: u32,
}

#[coverage(off)]
impl MyStruct {
#[coverage(off)]
fn method(&self) {}
#[coverage(off)]
fn assoc_fn() {}
}

extern "C" {
#[coverage(off)] //~ ERROR [E0788]
static X: u32;

#[coverage(off)] //~ ERROR [E0788]
type T;

#[coverage(off)] //~ ERROR [E0788]
fn foreign_fn();
}

#[coverage(off)]
fn main() {
#[coverage(off)] //~ ERROR [E0788]
let _ = ();

// Currently not allowed on let statements, even if they bind to a closure.
// It might be nice to support this as a special case someday, but trying
// to define the precise boundaries of that special case might be tricky.
#[coverage(off)] //~ ERROR [E0788]
let _let_closure = || ();

// In situations where attributes can already be applied to expressions,
// the coverage attribute is allowed on closure expressions.
let _closure_tail_expr = {
#[coverage(off)]
|| ()
};

// Applying attributes to arbitrary expressions requires an unstable
// feature, but if that feature were enabled then this would be allowed.
let _closure_expr = #[coverage(off)] || ();
//~^ ERROR attributes on expressions are experimental [E0658]

match () {
#[coverage(off)] //~ ERROR [E0788]
() => (),
}

#[coverage(off)] //~ ERROR [E0788]
return ();
}
Loading

0 comments on commit 1a48e23

Please sign in to comment.