Rollup merge of #143808 - JonathanBrouwer:should_panic_parser, r=jdonszelmann

Port `#[should_panic]` to the new attribute parsing infrastructure

Ports `#[should_panic]` to the new attribute parsing infrastructure for https://github.com/rust-lang/rust/issues/131229#issuecomment-2971351163

r? ```@jdonszelmann```

13 files changed, 172 insertions, 101 deletions

diff --git a/compiler/rustc_attr_parsing/src/attributes/test_attrs.rs b/compiler/rustc_attr_parsing/src/attributes/test_attrs.rs
index a90ed830cd1..77b494328c7 100644
--- a/compiler/rustc_attr_parsing/src/attributes/test_attrs.rs
+++ b/compiler/rustc_attr_parsing/src/attributes/test_attrs.rs
@@ -44,3 +44,55 @@ impl<S: Stage> SingleAttributeParser<S> for IgnoreParser {
         })
     }
 }
+
+pub(crate) struct ShouldPanicParser;
+
+impl<S: Stage> SingleAttributeParser<S> for ShouldPanicParser {
+    const PATH: &[Symbol] = &[sym::should_panic];
+    const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepOutermost;
+    const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::WarnButFutureError;
+    const TEMPLATE: AttributeTemplate =
+        template!(Word, List: r#"expected = "reason""#, NameValueStr: "reason");
+
+    fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> {
+        Some(AttributeKind::ShouldPanic {
+            span: cx.attr_span,
+            reason: match args {
+                ArgParser::NoArgs => None,
+                ArgParser::NameValue(name_value) => {
+                    let Some(str_value) = name_value.value_as_str() else {
+                        cx.expected_string_literal(
+                            name_value.value_span,
+                            Some(name_value.value_as_lit()),
+                        );
+                        return None;
+                    };
+                    Some(str_value)
+                }
+                ArgParser::List(list) => {
+                    let Some(single) = list.single() else {
+                        cx.expected_single_argument(list.span);
+                        return None;
+                    };
+                    let Some(single) = single.meta_item() else {
+                        cx.expected_name_value(single.span(), Some(sym::expected));
+                        return None;
+                    };
+                    if !single.path().word_is(sym::expected) {
+                        cx.expected_specific_argument_strings(list.span, vec!["expected"]);
+                        return None;
+                    }
+                    let Some(nv) = single.args().name_value() else {
+                        cx.expected_name_value(single.span(), Some(sym::expected));
+                        return None;
+                    };
+                    let Some(expected) = nv.value_as_str() else {
+                        cx.expected_string_literal(nv.value_span, Some(nv.value_as_lit()));
+                        return None;
+                    };
+                    Some(expected)
+                }
+            },
+        })
+    }
+}
diff --git a/compiler/rustc_attr_parsing/src/context.rs b/compiler/rustc_attr_parsing/src/context.rs
index 911d2e13310..80dfdffdb55 100644
--- a/compiler/rustc_attr_parsing/src/context.rs
+++ b/compiler/rustc_attr_parsing/src/context.rs
@@ -50,7 +50,7 @@ use crate::attributes::semantics::MayDangleParser;
 use crate::attributes::stability::{
     BodyStabilityParser, ConstStabilityIndirectParser, ConstStabilityParser, StabilityParser,
 };
-use crate::attributes::test_attrs::IgnoreParser;
+use crate::attributes::test_attrs::{IgnoreParser, ShouldPanicParser};
 use crate::attributes::traits::{
     AllowIncoherentImplParser, CoherenceIsCoreParser, CoinductiveParser, ConstTraitParser,
     DenyExplicitImplParser, DoNotImplementViaObjectParser, FundamentalParser, MarkerParser,
@@ -174,6 +174,7 @@ attribute_parsers!(
         Single<RustcLayoutScalarValidRangeEnd>,
         Single<RustcLayoutScalarValidRangeStart>,
         Single<RustcObjectLifetimeDefaultParser>,
+        Single<ShouldPanicParser>,
         Single<SkipDuringMethodDispatchParser>,
         Single<TransparencyParser>,
         Single<WithoutArgs<AllowIncoherentImplParser>>,
diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs
index ba3d8368b2a..7b57c02b197 100644
--- a/compiler/rustc_builtin_macros/src/test.rs
+++ b/compiler/rustc_builtin_macros/src/test.rs
@@ -5,10 +5,13 @@ use std::assert_matches::assert_matches;
 use std::iter;
 
 use rustc_ast::ptr::P;
-use rustc_ast::{self as ast, GenericParamKind, attr, join_path_idents};
+use rustc_ast::{self as ast, GenericParamKind, HasNodeId, attr, join_path_idents};
 use rustc_ast_pretty::pprust;
+use rustc_attr_parsing::AttributeParser;
 use rustc_errors::{Applicability, Diag, Level};
 use rustc_expand::base::*;
+use rustc_hir::Attribute;
+use rustc_hir::attrs::AttributeKind;
 use rustc_span::{ErrorGuaranteed, FileNameDisplayPreference, Ident, Span, Symbol, sym};
 use thin_vec::{ThinVec, thin_vec};
 use tracing::debug;
@@ -473,39 +476,19 @@ fn should_ignore_message(i: &ast::Item) -> Option<Symbol> {
 }
 
 fn should_panic(cx: &ExtCtxt<'_>, i: &ast::Item) -> ShouldPanic {
-    match attr::find_by_name(&i.attrs, sym::should_panic) {
-        Some(attr) => {
-            match attr.meta_item_list() {
-                // Handle #[should_panic(expected = "foo")]
-                Some(list) => {
-                    let msg = list
-                        .iter()
-                        .find(|mi| mi.has_name(sym::expected))
-                        .and_then(|mi| mi.meta_item())
-                        .and_then(|mi| mi.value_str());
-                    if list.len() != 1 || msg.is_none() {
-                        cx.dcx()
-                            .struct_span_warn(
-                                attr.span,
-                                "argument must be of the form: \
-                             `expected = \"error message\"`",
-                            )
-                            .with_note(
-                                "errors in this attribute were erroneously \
-                                allowed and will become a hard error in a \
-                                future release",
-                            )
-                            .emit();
-                        ShouldPanic::Yes(None)
-                    } else {
-                        ShouldPanic::Yes(msg)
-                    }
-                }
-                // Handle #[should_panic] and #[should_panic = "expected"]
-                None => ShouldPanic::Yes(attr.value_str()),
-            }
-        }
-        None => ShouldPanic::No,
+    if let Some(Attribute::Parsed(AttributeKind::ShouldPanic { reason, .. })) =
+        AttributeParser::parse_limited(
+            cx.sess,
+            &i.attrs,
+            sym::should_panic,
+            i.span,
+            i.node_id(),
+            None,
+        )
+    {
+        ShouldPanic::Yes(reason)
+    } else {
+        ShouldPanic::No
     }
 }
 
diff --git a/compiler/rustc_hir/src/attrs/data_structures.rs b/compiler/rustc_hir/src/attrs/data_structures.rs
index d9d2ec48948..5f419315467 100644
--- a/compiler/rustc_hir/src/attrs/data_structures.rs
+++ b/compiler/rustc_hir/src/attrs/data_structures.rs
@@ -436,6 +436,9 @@ pub enum AttributeKind {
     /// Represents `#[rustc_object_lifetime_default]`.
     RustcObjectLifetimeDefault,
 
+    /// Represents `#[should_panic]`
+    ShouldPanic { reason: Option<Symbol>, span: Span },
+
     /// Represents `#[rustc_skip_during_method_dispatch]`.
     SkipDuringMethodDispatch { array: bool, boxed_slice: bool, span: Span },
 
diff --git a/compiler/rustc_hir/src/attrs/encode_cross_crate.rs b/compiler/rustc_hir/src/attrs/encode_cross_crate.rs
index b66e5bbeabe..e3a7f0b97a8 100644
--- a/compiler/rustc_hir/src/attrs/encode_cross_crate.rs
+++ b/compiler/rustc_hir/src/attrs/encode_cross_crate.rs
@@ -70,6 +70,7 @@ impl AttributeKind {
             RustcLayoutScalarValidRangeEnd(..) => Yes,
             RustcLayoutScalarValidRangeStart(..) => Yes,
             RustcObjectLifetimeDefault => No,
+            ShouldPanic { .. } => No,
             SkipDuringMethodDispatch { .. } => No,
             SpecializationTrait(..) => No,
             Stability { .. } => Yes,
diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
index c30c830f9af..34db6f92d92 100644
--- a/compiler/rustc_hir/src/hir.rs
+++ b/compiler/rustc_hir/src/hir.rs
@@ -1308,6 +1308,7 @@ impl AttributeExt for Attribute {
             Attribute::Parsed(AttributeKind::MacroUse { span, .. }) => *span,
             Attribute::Parsed(AttributeKind::MayDangle(span)) => *span,
             Attribute::Parsed(AttributeKind::Ignore { span, .. }) => *span,
+            Attribute::Parsed(AttributeKind::ShouldPanic { span, .. }) => *span,
             Attribute::Parsed(AttributeKind::AutomaticallyDerived(span)) => *span,
             a => panic!("can't get the span of an arbitrary parsed attribute: {a:?}"),
         }
diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs
index 890028d977d..10c532b436a 100644
--- a/compiler/rustc_passes/src/check_attr.rs
+++ b/compiler/rustc_passes/src/check_attr.rs
@@ -315,6 +315,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                 Attribute::Parsed(AttributeKind::Used { span: attr_span, .. }) => {
                     self.check_used(*attr_span, target, span);
                 }
+                Attribute::Parsed(AttributeKind::ShouldPanic { span: attr_span, .. }) => self
+                    .check_generic_attr(hir_id, sym::should_panic, *attr_span, target, Target::Fn),
                 &Attribute::Parsed(AttributeKind::PassByValue(attr_span)) => {
                     self.check_pass_by_value(attr_span, span, target)
                 }
@@ -387,9 +389,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                         [sym::link, ..] => self.check_link(hir_id, attr, span, target),
                         [sym::path, ..] => self.check_generic_attr_unparsed(hir_id, attr, target, Target::Mod),
                         [sym::macro_export, ..] => self.check_macro_export(hir_id, attr, target),
-                        [sym::should_panic, ..] => {
-                            self.check_generic_attr_unparsed(hir_id, attr, target, Target::Fn)
-                        }
                         [sym::autodiff_forward, ..] | [sym::autodiff_reverse, ..] => {
                             self.check_autodiff(hir_id, attr, span, target)
                         }
diff --git a/tests/ui/attributes/check-builtin-attr-ice.rs b/tests/ui/attributes/check-builtin-attr-ice.rs
index 7745849acd0..811210e2cca 100644
--- a/tests/ui/attributes/check-builtin-attr-ice.rs
+++ b/tests/ui/attributes/check-builtin-attr-ice.rs
@@ -44,12 +44,10 @@
 struct Foo {
     #[should_panic::skip]
     //~^ ERROR failed to resolve
-    //~| ERROR `#[should_panic::skip]` only has an effect on functions
     pub field: u8,
 
     #[should_panic::a::b::c]
     //~^ ERROR failed to resolve
-    //~| ERROR `#[should_panic::a::b::c]` only has an effect on functions
     pub field2: u8,
 }
 
diff --git a/tests/ui/attributes/check-builtin-attr-ice.stderr b/tests/ui/attributes/check-builtin-attr-ice.stderr
index 4f26f71efb7..07bbe01898a 100644
--- a/tests/ui/attributes/check-builtin-attr-ice.stderr
+++ b/tests/ui/attributes/check-builtin-attr-ice.stderr
@@ -5,35 +5,17 @@ LL |     #[should_panic::skip]
    |       ^^^^^^^^^^^^ use of unresolved module or unlinked crate `should_panic`
 
 error[E0433]: failed to resolve: use of unresolved module or unlinked crate `should_panic`
-  --> $DIR/check-builtin-attr-ice.rs:50:7
+  --> $DIR/check-builtin-attr-ice.rs:49:7
    |
 LL |     #[should_panic::a::b::c]
    |       ^^^^^^^^^^^^ use of unresolved module or unlinked crate `should_panic`
 
 error[E0433]: failed to resolve: use of unresolved module or unlinked crate `deny`
-  --> $DIR/check-builtin-attr-ice.rs:59:7
+  --> $DIR/check-builtin-attr-ice.rs:57:7
    |
 LL |     #[deny::skip]
    |       ^^^^ use of unresolved module or unlinked crate `deny`
 
-error: `#[should_panic::skip]` only has an effect on functions
-  --> $DIR/check-builtin-attr-ice.rs:45:5
-   |
-LL |     #[should_panic::skip]
-   |     ^^^^^^^^^^^^^^^^^^^^^
-   |
-note: the lint level is defined here
-  --> $DIR/check-builtin-attr-ice.rs:42:9
-   |
-LL | #![deny(unused_attributes)]
-   |         ^^^^^^^^^^^^^^^^^
-
-error: `#[should_panic::a::b::c]` only has an effect on functions
-  --> $DIR/check-builtin-attr-ice.rs:50:5
-   |
-LL |     #[should_panic::a::b::c]
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^
-
-error: aborting due to 5 previous errors
+error: aborting due to 3 previous errors
 
 For more information about this error, try `rustc --explain E0433`.
diff --git a/tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr b/tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr
index 8bac1f6155e..f2ae50b75a3 100644
--- a/tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr
+++ b/tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr
@@ -361,12 +361,6 @@ warning: crate-level attribute should be an inner attribute: add an exclamation
 LL | #[type_length_limit="0100"]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-warning: `#[should_panic]` only has an effect on functions
-  --> $DIR/issue-43106-gating-of-builtin-attrs.rs:53:1
-   |
-LL | #![should_panic]
-   | ^^^^^^^^^^^^^^^^
-
 warning: attribute should be applied to an `extern` block with non-Rust ABI
   --> $DIR/issue-43106-gating-of-builtin-attrs.rs:64:1
    |
@@ -409,6 +403,12 @@ warning: `#[proc_macro_derive]` only has an effect on functions
 LL | #![proc_macro_derive(Test)]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+warning: `#[should_panic]` only has an effect on functions
+  --> $DIR/issue-43106-gating-of-builtin-attrs.rs:53:1
+   |
+LL | #![should_panic]
+   | ^^^^^^^^^^^^^^^^
+
 warning: attribute should be applied to a function definition
   --> $DIR/issue-43106-gating-of-builtin-attrs.rs:62:1
    |
diff --git a/tests/ui/lint/unused/unused-attr-duplicate.stderr b/tests/ui/lint/unused/unused-attr-duplicate.stderr
index e277f5203c6..6c44e884ba5 100644
--- a/tests/ui/lint/unused/unused-attr-duplicate.stderr
+++ b/tests/ui/lint/unused/unused-attr-duplicate.stderr
@@ -16,19 +16,6 @@ LL | #![deny(unused_attributes)]
    |         ^^^^^^^^^^^^^^^^^
 
 error: unused attribute
-  --> $DIR/unused-attr-duplicate.rs:55:1
-   |
-LL | #[should_panic(expected = "values don't match")]
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
-   |
-note: attribute also specified here
-  --> $DIR/unused-attr-duplicate.rs:54: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!
-
-error: unused attribute
   --> $DIR/unused-attr-duplicate.rs:14:1
    |
 LL | #![crate_name = "unused_attr_duplicate2"]
@@ -154,6 +141,19 @@ LL | #[ignore]
    | ^^^^^^^^^
 
 error: unused attribute
+  --> $DIR/unused-attr-duplicate.rs:55:1
+   |
+LL | #[should_panic(expected = "values don't match")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
+   |
+note: attribute also specified here
+  --> $DIR/unused-attr-duplicate.rs:54: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!
+
+error: unused attribute
   --> $DIR/unused-attr-duplicate.rs:60:1
    |
 LL | #[must_use = "some message"]
diff --git a/tests/ui/test-attrs/test-should-panic-attr.rs b/tests/ui/test-attrs/test-should-panic-attr.rs
index df2893b63ed..af54689551c 100644
--- a/tests/ui/test-attrs/test-should-panic-attr.rs
+++ b/tests/ui/test-attrs/test-should-panic-attr.rs
@@ -1,4 +1,3 @@
-//@ check-pass
 //@ compile-flags: --test
 
 #[test]
@@ -9,28 +8,32 @@ fn test1() {
 
 #[test]
 #[should_panic(expected)]
-//~^ WARN: argument must be of the form:
+//~^ ERROR malformed `should_panic` attribute input
+//~| NOTE expected this to be of the form `expected = "..."`
 fn test2() {
     panic!();
 }
 
 #[test]
 #[should_panic(expect)]
-//~^ WARN: argument must be of the form:
+//~^ ERROR malformed `should_panic` attribute input
+//~| NOTE the only valid argument here is "expected"
 fn test3() {
     panic!();
 }
 
 #[test]
 #[should_panic(expected(foo, bar))]
-//~^ WARN: argument must be of the form:
+//~^ ERROR malformed `should_panic` attribute input
+//~| NOTE expected this to be of the form `expected = "..."`
 fn test4() {
     panic!();
 }
 
 #[test]
 #[should_panic(expected = "foo", bar)]
-//~^ WARN: argument must be of the form:
+//~^ ERROR malformed `should_panic` attribute input
+//~| NOTE expected a single argument here
 fn test5() {
     panic!();
 }
diff --git a/tests/ui/test-attrs/test-should-panic-attr.stderr b/tests/ui/test-attrs/test-should-panic-attr.stderr
index 492d1d5e03a..5dfc8e503e8 100644
--- a/tests/ui/test-attrs/test-should-panic-attr.stderr
+++ b/tests/ui/test-attrs/test-should-panic-attr.stderr
@@ -1,34 +1,82 @@
-warning: argument must be of the form: `expected = "error message"`
-  --> $DIR/test-should-panic-attr.rs:11:1
+error[E0539]: malformed `should_panic` attribute input
+  --> $DIR/test-should-panic-attr.rs:10:1
    |
 LL | #[should_panic(expected)]
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
+   | ^^^^^^^^^^^^^^^--------^^
+   |                |
+   |                expected this to be of the form `expected = "..."`
+   |
+help: try changing it to one of the following valid forms of the attribute
+   |
+LL - #[should_panic(expected)]
+LL + #[should_panic = "reason"]
+   |
+LL | #[should_panic(expected = "reason")]
+   |                         ++++++++++
+LL - #[should_panic(expected)]
+LL + #[should_panic]
    |
-   = note: errors in this attribute were erroneously allowed and will become a hard error in a future release
 
-warning: argument must be of the form: `expected = "error message"`
+error[E0539]: malformed `should_panic` attribute input
   --> $DIR/test-should-panic-attr.rs:18:1
    |
 LL | #[should_panic(expect)]
-   | ^^^^^^^^^^^^^^^^^^^^^^^
+   | ^^^^^^^^^^^^^^--------^
+   |               |
+   |               the only valid argument here is "expected"
+   |
+help: try changing it to one of the following valid forms of the attribute
+   |
+LL - #[should_panic(expect)]
+LL + #[should_panic = "reason"]
+   |
+LL | #[should_panic(expected = "reason")]
+   |                      +++++++++++++
+LL - #[should_panic(expect)]
+LL + #[should_panic]
    |
-   = note: errors in this attribute were erroneously allowed and will become a hard error in a future release
 
-warning: argument must be of the form: `expected = "error message"`
-  --> $DIR/test-should-panic-attr.rs:25:1
+error[E0539]: malformed `should_panic` attribute input
+  --> $DIR/test-should-panic-attr.rs:26:1
    |
 LL | #[should_panic(expected(foo, bar))]
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   | ^^^^^^^^^^^^^^^------------------^^
+   |                |
+   |                expected this to be of the form `expected = "..."`
+   |
+help: try changing it to one of the following valid forms of the attribute
+   |
+LL - #[should_panic(expected(foo, bar))]
+LL + #[should_panic = "reason"]
+   |
+LL - #[should_panic(expected(foo, bar))]
+LL + #[should_panic(expected = "reason")]
+   |
+LL - #[should_panic(expected(foo, bar))]
+LL + #[should_panic]
    |
-   = note: errors in this attribute were erroneously allowed and will become a hard error in a future release
 
-warning: argument must be of the form: `expected = "error message"`
-  --> $DIR/test-should-panic-attr.rs:32:1
+error[E0805]: malformed `should_panic` attribute input
+  --> $DIR/test-should-panic-attr.rs:34:1
    |
 LL | #[should_panic(expected = "foo", bar)]
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   | ^^^^^^^^^^^^^^-----------------------^
+   |               |
+   |               expected a single argument here
+   |
+help: try changing it to one of the following valid forms of the attribute
+   |
+LL - #[should_panic(expected = "foo", bar)]
+LL + #[should_panic = "reason"]
+   |
+LL - #[should_panic(expected = "foo", bar)]
+LL + #[should_panic(expected = "reason")]
+   |
+LL - #[should_panic(expected = "foo", bar)]
+LL + #[should_panic]
    |
-   = note: errors in this attribute were erroneously allowed and will become a hard error in a future release
 
-warning: 4 warnings emitted
+error: aborting due to 4 previous errors
 
+Some errors have detailed explanations: E0539, E0805.
+For more information about an error, try `rustc --explain E0539`.