Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
add FCW to #[should_panic] when applied to non #[test] or `#[benc…
…h]` fn
  • Loading branch information
Bryntet committed Jan 18, 2026
commit 05556efc991126d67d07f35c0784dc3c9aac871d
2 changes: 1 addition & 1 deletion compiler/rustc_ast_pretty/src/pprust/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
}

fn print_attribute_inline(&mut self, attr: &ast::Attribute, is_inline: bool) -> bool {
if attr.has_name(sym::cfg_trace) || attr.has_name(sym::cfg_attr_trace) {
if attr.has_any_name(&[sym::cfg_trace, sym::cfg_attr_trace, sym::test_trace]) {
// It's not a valid identifier, so avoid printing it
// to keep the printed code reasonably parse-able.
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ impl<S: Stage> AttributeParser<S> for NakedParser {
sym::cfg_attr_trace,
// testing (allowed here so better errors can be generated in `rustc_builtin_macros::test`)
sym::test,
sym::test_trace,
sym::ignore,
sym::should_panic,
sym::bench,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_attr_parsing/src/attributes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ pub(crate) mod rustc_internal;
pub(crate) mod semantics;
pub(crate) mod stability;
pub(crate) mod test_attrs;
pub(crate) mod trace;
pub(crate) mod traits;
pub(crate) mod transparency;
pub(crate) mod util;
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_attr_parsing/src/attributes/trace.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use super::prelude::*;

pub(crate) struct TestTraceParser;

impl<S: Stage> NoArgsAttributeParser<S> for TestTraceParser {
const PATH: &[Symbol] = &[sym::test_trace];

const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::Ignore;

const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowList(&[Allow(Target::Fn)]);

const CREATE: fn(Span) -> AttributeKind = |_| AttributeKind::TestTrace;
}
2 changes: 2 additions & 0 deletions compiler/rustc_attr_parsing/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ use crate::attributes::stability::{
BodyStabilityParser, ConstStabilityIndirectParser, ConstStabilityParser, StabilityParser,
};
use crate::attributes::test_attrs::{IgnoreParser, ShouldPanicParser};
use crate::attributes::trace::TestTraceParser;
use crate::attributes::traits::{
AllowIncoherentImplParser, CoinductiveParser, DenyExplicitImplParser,
DoNotImplementViaObjectParser, FundamentalParser, MarkerParser, ParenSugarParser,
Expand Down Expand Up @@ -291,6 +292,7 @@ attribute_parsers!(
Single<WithoutArgs<RustcShouldNotBeCalledOnConstItems>>,
Single<WithoutArgs<SpecializationTraitParser>>,
Single<WithoutArgs<StdInternalSymbolParser>>,
Single<WithoutArgs<TestTraceParser>>,
Single<WithoutArgs<ThreadLocalParser>>,
Single<WithoutArgs<TrackCallerParser>>,
Single<WithoutArgs<TypeConstParser>>,
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_attr_parsing/src/validate_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ use rustc_span::{Span, Symbol, sym};
use crate::{AttributeParser, Late, session_diagnostics as errors};

pub fn check_attr(psess: &ParseSess, attr: &Attribute) {
if attr.is_doc_comment() || attr.has_name(sym::cfg_trace) || attr.has_name(sym::cfg_attr_trace)
if attr.is_doc_comment()
|| attr.has_any_name(&[sym::cfg_trace, sym::cfg_attr_trace, sym::test_trace])
{
return;
}
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_builtin_macros/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use rustc_ast_pretty::pprust;
use rustc_attr_parsing::AttributeParser;
use rustc_errors::{Applicability, Diag, Level};
use rustc_expand::base::*;
use rustc_expand::config::attr_into_trace;
use rustc_hir::Attribute;
use rustc_hir::attrs::AttributeKind;
use rustc_span::{ErrorGuaranteed, Ident, RemapPathScopeComponents, Span, Symbol, sym};
Expand Down Expand Up @@ -113,7 +114,7 @@ pub(crate) fn expand_test_or_bench(
item: Annotatable,
is_bench: bool,
) -> Vec<Annotatable> {
let (item, is_stmt) = match item {
let (mut item, is_stmt) = match item {
Annotatable::Item(i) => (i, false),
Annotatable::Stmt(box ast::Stmt { kind: ast::StmtKind::Item(i), .. }) => (i, true),
other => {
Expand All @@ -136,6 +137,14 @@ pub(crate) fn expand_test_or_bench(
return vec![];
}

// Add a trace to the originating test function, so that we can
// check if attributes that have `#[test]` or `#[bench]` as a requirement
// actually are annotated with said attributes
item.attrs.push(attr_into_trace(
cx.attr_word(sym::test_trace, cx.with_def_site_ctxt(attr_sp)),
sym::test_trace,
));

if let Some(attr) = attr::find_by_name(&item.attrs, sym::naked) {
cx.dcx().emit_err(errors::NakedFunctionTestingAttribute {
testing_span: attr_sp,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ pub fn pre_configure_attrs(sess: &Session, attrs: &[Attribute]) -> ast::AttrVec
.collect()
}

pub(crate) fn attr_into_trace(mut attr: Attribute, trace_name: Symbol) -> Attribute {
pub fn attr_into_trace(mut attr: Attribute, trace_name: Symbol) -> Attribute {
match &mut attr.kind {
AttrKind::Normal(normal) => {
let NormalAttr { item, tokens } = &mut **normal;
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,10 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
cfg_attr_trace, Normal, template!(Word /* irrelevant */), DuplicatesOk,
EncodeCrossCrate::No
),
ungated!(
test_trace, Normal, template!(Word), ErrorFollowing,
EncodeCrossCrate::No
),

// ==========================================================================
// Internal attributes, Diagnostics related:
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_hir/src/attrs/data_structures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1021,6 +1021,9 @@ pub enum AttributeKind {
/// `#[unsafe(force_target_feature(enable = "...")]`.
TargetFeature { features: ThinVec<(Symbol, Span)>, attr_span: Span, was_forced: bool },

/// Represents `#[<test_trace>]`
TestTrace,

/// Represents `#[thread_local]`
ThreadLocal,

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/attrs/encode_cross_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ impl AttributeKind {
Stability { .. } => Yes,
StdInternalSymbol(..) => No,
TargetFeature { .. } => No,
TestTrace => No,
ThreadLocal => No,
TrackCaller(..) => Yes,
TypeConst(..) => Yes,
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,9 @@ passes_sanitize_attribute_not_allowed =
.no_body = function has no body
.help = sanitize attribute can be applied to a function (with body), impl block, or module

passes_should_panic_must_be_applied_to_test_or_bench = `#[should_panic]` should only be applied to functions annotated with `#[test]` or `#[bench]`
.warn = {-passes_previously_accepted}

passes_trait_impl_const_stability_mismatch = const stability on the impl does not match the const stability on the trait
passes_trait_impl_const_stability_mismatch_impl_stable = this impl is (implicitly) stable...
passes_trait_impl_const_stability_mismatch_impl_unstable = this impl is unstable...
Expand Down
14 changes: 13 additions & 1 deletion compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
self.check_rustc_must_implement_one_of(*attr_span, fn_names, hir_id,target)
},
Attribute::Parsed(AttributeKind::DoNotRecommend{attr_span}) => {self.check_do_not_recommend(*attr_span, hir_id, target, item)},
Attribute::Parsed(AttributeKind::ShouldPanic { span, .. }) => {
self.check_should_panic(attrs, *span, target);
},
Attribute::Parsed(
AttributeKind::EiiDeclaration { .. }
| AttributeKind::EiiForeignItem
Expand Down Expand Up @@ -286,7 +289,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
| AttributeKind::PassByValue (..)
| AttributeKind::StdInternalSymbol (..)
| AttributeKind::Coverage (..)
| AttributeKind::ShouldPanic { .. }
| AttributeKind::Coroutine(..)
| AttributeKind::Linkage(..)
| AttributeKind::MustUse { .. }
Expand Down Expand Up @@ -316,6 +318,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
| AttributeKind::RustcDumpDefParents
| AttributeKind::RustcDumpVtable(..)
| AttributeKind::NeedsAllocator
| AttributeKind::TestTrace
) => { /* do nothing */ }
Attribute::Unparsed(attr_item) => {
style = Some(attr_item.style);
Expand Down Expand Up @@ -484,6 +487,15 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
self.check_mix_no_mangle_export(hir_id, attrs);
}

fn check_should_panic(&self, attrs: &[Attribute], attr_span: Span, target: Target) {
// The error message only makes sense if it's actually being applied on a function
if matches!(target, Target::Fn) {
if !find_attr!(attrs, AttributeKind::TestTrace) {
self.dcx().emit_warn(errors::MustBeAppliedToTest { attr_span, warning: true });
}
}
}

fn check_rustc_must_implement_one_of(
&self,
attr_span: Span,
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1316,3 +1316,12 @@ pub(crate) struct FunctionNamesDuplicated {
#[primary_span]
pub spans: Vec<Span>,
}

#[derive(Diagnostic)]
#[diag(passes_should_panic_must_be_applied_to_test_or_bench)]
pub(crate) struct MustBeAppliedToTest {
#[primary_span]
pub attr_span: Span,
#[warning]
pub warning: bool,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a reason #[warning] is configurable here?
I think this can just always be a warning

(you can do this by applying #[warning] to the struct)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is what all the other FCW in rustc_passes/messages.ftl do AFAICT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Afaik you should just be able to put #[warning] on the struct for the same effect

Comment thread
JonathanBrouwer marked this conversation as resolved.
}
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2275,6 +2275,7 @@ symbols! {
test_case,
test_removed_feature,
test_runner,
test_trace: "<test_trace>",
test_unstable_lint,
thread,
thread_local,
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/attributes/should-panic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//@ check-pass
#![feature(test)]

#[should_panic]
//~^ WARN `#[should_panic]` should only be applied to functions annotated with `#[test]` or `#[bench]`
//~| WARN this was previously accepted
pub fn foo() {}


#[test]
#[should_panic]
pub fn bar() {}

#[should_panic]
#[bench]
pub fn bazz() {}

fn main() { }
10 changes: 10 additions & 0 deletions tests/ui/attributes/should-panic.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
warning: `#[should_panic]` should only be applied to functions annotated with `#[test]` or `#[bench]`
--> $DIR/should-panic.rs:4:1
|
LL | #[should_panic]
| ^^^^^^^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

warning: 1 warning emitted

20 changes: 20 additions & 0 deletions tests/ui/attributes/test_trace.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//@ compile-flags: --test

fn main() {}

#[test_trace]
//~^ ERROR cannot find attribute `test_trace` in this scope
fn foo() {}


#[test]
#[test_trace]
//~^ ERROR cannot find attribute `test_trace` in this scope
fn bar() {}

#[test]
#[test_trace]
//~^ ERROR cannot find attribute `test_trace` in this scope
#[test]
//~^ WARN duplicated attribute
fn bazz() {}
46 changes: 46 additions & 0 deletions tests/ui/attributes/test_trace.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
error: cannot find attribute `test_trace` in this scope
--> $DIR/test_trace.rs:5:3
|
LL | #[test_trace]
| ^^^^^^^^^^
|
help: a built-in attribute with a similar name exists
|
LL - #[test_trace]
LL + #[<test_trace>]
|

error: cannot find attribute `test_trace` in this scope
--> $DIR/test_trace.rs:11:3
|
LL | #[test_trace]
| ^^^^^^^^^^
|
help: a built-in attribute with a similar name exists
|
LL - #[test_trace]
LL + #[<test_trace>]
|

error: cannot find attribute `test_trace` in this scope
--> $DIR/test_trace.rs:16:3
|
LL | #[test_trace]
| ^^^^^^^^^^
|
help: a built-in attribute with a similar name exists
|
LL - #[test_trace]
LL + #[<test_trace>]
|

warning: duplicated attribute
--> $DIR/test_trace.rs:18:1
|
LL | #[test]
| ^^^^^^^
|
= note: `#[warn(duplicate_macro_attributes)]` on by default

error: aborting due to 3 previous errors; 1 warning emitted

5 changes: 4 additions & 1 deletion tests/ui/attributes/unsafe/extraneous-unsafe-attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ fn aa() {}
#[unsafe(ignore = "test")] //~ ERROR: is not an unsafe attribute
fn bb() {}

#[unsafe(should_panic(expected = "test"))] //~ ERROR: is not an unsafe attribute
#[unsafe(should_panic(expected = "test"))]
//~^ ERROR: is not an unsafe attribute
//~| WARN `#[should_panic]` should only be applied to functions annotated with `#[test]` or `#[bench]`
//~| WARN this was previously accepted
fn cc() {}

#[unsafe(macro_use)] //~ ERROR: is not an unsafe attribute
Expand Down
20 changes: 14 additions & 6 deletions tests/ui/attributes/unsafe/extraneous-unsafe-attributes.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ LL | #[unsafe(test)]
= note: extraneous unsafe is not allowed in attributes

error: expected identifier, found keyword `unsafe`
--> $DIR/extraneous-unsafe-attributes.rs:30:19
--> $DIR/extraneous-unsafe-attributes.rs:33:19
|
LL | let _a = cfg!(unsafe(foo));
| ^^^^^^ expected identifier, found keyword
Expand All @@ -34,7 +34,7 @@ LL | let _a = cfg!(r#unsafe(foo));
| ++

error[E0537]: invalid predicate `r#unsafe`
--> $DIR/extraneous-unsafe-attributes.rs:30:19
--> $DIR/extraneous-unsafe-attributes.rs:33:19
|
LL | let _a = cfg!(unsafe(foo));
| ^^^^^^^^^^^
Expand All @@ -56,29 +56,37 @@ LL | #[unsafe(should_panic(expected = "test"))]
= note: extraneous unsafe is not allowed in attributes

error: `macro_use` is not an unsafe attribute
--> $DIR/extraneous-unsafe-attributes.rs:18:3
--> $DIR/extraneous-unsafe-attributes.rs:21:3
|
LL | #[unsafe(macro_use)]
| ^^^^^^ this is not an unsafe attribute
|
= note: extraneous unsafe is not allowed in attributes

error: `macro_export` is not an unsafe attribute
--> $DIR/extraneous-unsafe-attributes.rs:20:7
--> $DIR/extraneous-unsafe-attributes.rs:23:7
|
LL | #[unsafe(macro_export)]
| ^^^^^^ this is not an unsafe attribute
|
= note: extraneous unsafe is not allowed in attributes

error: `used` is not an unsafe attribute
--> $DIR/extraneous-unsafe-attributes.rs:26:3
--> $DIR/extraneous-unsafe-attributes.rs:29:3
|
LL | #[unsafe(used)]
| ^^^^^^ this is not an unsafe attribute
|
= note: extraneous unsafe is not allowed in attributes

error: aborting due to 10 previous errors
warning: `#[should_panic]` should only be applied to functions annotated with `#[test]` or `#[bench]`
--> $DIR/extraneous-unsafe-attributes.rs:15:1
|
LL | #[unsafe(should_panic(expected = "test"))]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

error: aborting due to 10 previous errors; 1 warning emitted

For more information about this error, try `rustc --explain E0537`.
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,10 @@ mod should_panic {
//~| HELP can only be applied to
//~| HELP remove the attribute

#[should_panic] fn f() { }
#[should_panic]
//~^ WARN `#[should_panic]` should only be applied to functions annotated with `#[test]` or `#[bench]`
//~| WARN this was previously accepted
fn f() { }

#[should_panic] struct S;
//~^ WARN attribute cannot be used on
Expand Down
Loading