Skip to content

Commit 995ca3e

Browse files
ZalatharZalathar
authored
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 #131229 (comment) r? ```@jdonszelmann```
2 parents 5e781d0 + 4281e05 commit 995ca3e

File tree

13 files changed

+172
-101
lines changed

13 files changed

+172
-101
lines changed

‎compiler/rustc_attr_parsing/src/attributes/test_attrs.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,55 @@ impl<S: Stage> SingleAttributeParser<S> for IgnoreParser {
4444
})
4545
}
4646
}
47+
48+
pub(crate) struct ShouldPanicParser;
49+
50+
impl<S: Stage> SingleAttributeParser<S> for ShouldPanicParser {
51+
const PATH: &[Symbol] = &[sym::should_panic];
52+
const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepOutermost;
53+
const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::WarnButFutureError;
54+
const TEMPLATE: AttributeTemplate =
55+
template!(Word, List: r#"expected = "reason""#, NameValueStr: "reason");
56+
57+
fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> {
58+
Some(AttributeKind::ShouldPanic {
59+
span: cx.attr_span,
60+
reason: match args {
61+
ArgParser::NoArgs => None,
62+
ArgParser::NameValue(name_value) => {
63+
let Some(str_value) = name_value.value_as_str() else {
64+
cx.expected_string_literal(
65+
name_value.value_span,
66+
Some(name_value.value_as_lit()),
67+
);
68+
return None;
69+
};
70+
Some(str_value)
71+
}
72+
ArgParser::List(list) => {
73+
let Some(single) = list.single() else {
74+
cx.expected_single_argument(list.span);
75+
return None;
76+
};
77+
let Some(single) = single.meta_item() else {
78+
cx.expected_name_value(single.span(), Some(sym::expected));
79+
return None;
80+
};
81+
if !single.path().word_is(sym::expected) {
82+
cx.expected_specific_argument_strings(list.span, vec!["expected"]);
83+
return None;
84+
}
85+
let Some(nv) = single.args().name_value() else {
86+
cx.expected_name_value(single.span(), Some(sym::expected));
87+
return None;
88+
};
89+
let Some(expected) = nv.value_as_str() else {
90+
cx.expected_string_literal(nv.value_span, Some(nv.value_as_lit()));
91+
return None;
92+
};
93+
Some(expected)
94+
}
95+
},
96+
})
97+
}
98+
}

‎compiler/rustc_attr_parsing/src/context.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ use crate::attributes::semantics::MayDangleParser;
5050
use crate::attributes::stability::{
5151
BodyStabilityParser, ConstStabilityIndirectParser, ConstStabilityParser, StabilityParser,
5252
};
53-
use crate::attributes::test_attrs::IgnoreParser;
53+
use crate::attributes::test_attrs::{IgnoreParser, ShouldPanicParser};
5454
use crate::attributes::traits::{
5555
AllowIncoherentImplParser, CoherenceIsCoreParser, CoinductiveParser, ConstTraitParser,
5656
DenyExplicitImplParser, DoNotImplementViaObjectParser, FundamentalParser, MarkerParser,
@@ -174,6 +174,7 @@ attribute_parsers!(
174174
Single<RustcLayoutScalarValidRangeEnd>,
175175
Single<RustcLayoutScalarValidRangeStart>,
176176
Single<RustcObjectLifetimeDefaultParser>,
177+
Single<ShouldPanicParser>,
177178
Single<SkipDuringMethodDispatchParser>,
178179
Single<TransparencyParser>,
179180
Single<WithoutArgs<AllowIncoherentImplParser>>,

‎compiler/rustc_builtin_macros/src/test.rs

Lines changed: 17 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@ use std::assert_matches::assert_matches;
55
use std::iter;
66

77
use rustc_ast::ptr::P;
8-
use rustc_ast::{self as ast, GenericParamKind, attr, join_path_idents};
8+
use rustc_ast::{self as ast, GenericParamKind, HasNodeId, attr, join_path_idents};
99
use rustc_ast_pretty::pprust;
10+
use rustc_attr_parsing::AttributeParser;
1011
use rustc_errors::{Applicability, Diag, Level};
1112
use rustc_expand::base::*;
13+
use rustc_hir::Attribute;
14+
use rustc_hir::attrs::AttributeKind;
1215
use rustc_span::{ErrorGuaranteed, FileNameDisplayPreference, Ident, Span, Symbol, sym};
1316
use thin_vec::{ThinVec, thin_vec};
1417
use tracing::debug;
@@ -473,39 +476,19 @@ fn should_ignore_message(i: &ast::Item) -> Option<Symbol> {
473476
}
474477

475478
fn should_panic(cx: &ExtCtxt<'_>, i: &ast::Item) -> ShouldPanic {
476-
match attr::find_by_name(&i.attrs, sym::should_panic) {
477-
Some(attr) => {
478-
match attr.meta_item_list() {
479-
// Handle #[should_panic(expected = "foo")]
480-
Some(list) => {
481-
let msg = list
482-
.iter()
483-
.find(|mi| mi.has_name(sym::expected))
484-
.and_then(|mi| mi.meta_item())
485-
.and_then(|mi| mi.value_str());
486-
if list.len() != 1 || msg.is_none() {
487-
cx.dcx()
488-
.struct_span_warn(
489-
attr.span,
490-
"argument must be of the form: \
491-
`expected = \"error message\"`",
492-
)
493-
.with_note(
494-
"errors in this attribute were erroneously \
495-
allowed and will become a hard error in a \
496-
future release",
497-
)
498-
.emit();
499-
ShouldPanic::Yes(None)
500-
} else {
501-
ShouldPanic::Yes(msg)
502-
}
503-
}
504-
// Handle #[should_panic] and #[should_panic = "expected"]
505-
None => ShouldPanic::Yes(attr.value_str()),
506-
}
507-
}
508-
None => ShouldPanic::No,
479+
if let Some(Attribute::Parsed(AttributeKind::ShouldPanic { reason, .. })) =
480+
AttributeParser::parse_limited(
481+
cx.sess,
482+
&i.attrs,
483+
sym::should_panic,
484+
i.span,
485+
i.node_id(),
486+
None,
487+
)
488+
{
489+
ShouldPanic::Yes(reason)
490+
} else {
491+
ShouldPanic::No
509492
}
510493
}
511494

‎compiler/rustc_hir/src/attrs/data_structures.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,9 @@ pub enum AttributeKind {
436436
/// Represents `#[rustc_object_lifetime_default]`.
437437
RustcObjectLifetimeDefault,
438438

439+
/// Represents `#[should_panic]`
440+
ShouldPanic { reason: Option<Symbol>, span: Span },
441+
439442
/// Represents `#[rustc_skip_during_method_dispatch]`.
440443
SkipDuringMethodDispatch { array: bool, boxed_slice: bool, span: Span },
441444

‎compiler/rustc_hir/src/attrs/encode_cross_crate.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ impl AttributeKind {
7070
RustcLayoutScalarValidRangeEnd(..) => Yes,
7171
RustcLayoutScalarValidRangeStart(..) => Yes,
7272
RustcObjectLifetimeDefault => No,
73+
ShouldPanic { .. } => No,
7374
SkipDuringMethodDispatch { .. } => No,
7475
SpecializationTrait(..) => No,
7576
Stability { .. } => Yes,

‎compiler/rustc_hir/src/hir.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1308,6 +1308,7 @@ impl AttributeExt for Attribute {
13081308
Attribute::Parsed(AttributeKind::MacroUse { span, .. }) => *span,
13091309
Attribute::Parsed(AttributeKind::MayDangle(span)) => *span,
13101310
Attribute::Parsed(AttributeKind::Ignore { span, .. }) => *span,
1311+
Attribute::Parsed(AttributeKind::ShouldPanic { span, .. }) => *span,
13111312
Attribute::Parsed(AttributeKind::AutomaticallyDerived(span)) => *span,
13121313
a => panic!("can't get the span of an arbitrary parsed attribute: {a:?}"),
13131314
}

‎compiler/rustc_passes/src/check_attr.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
315315
Attribute::Parsed(AttributeKind::Used { span: attr_span, .. }) => {
316316
self.check_used(*attr_span, target, span);
317317
}
318+
Attribute::Parsed(AttributeKind::ShouldPanic { span: attr_span, .. }) => self
319+
.check_generic_attr(hir_id, sym::should_panic, *attr_span, target, Target::Fn),
318320
&Attribute::Parsed(AttributeKind::PassByValue(attr_span)) => {
319321
self.check_pass_by_value(attr_span, span, target)
320322
}
@@ -387,9 +389,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
387389
[sym::link, ..] => self.check_link(hir_id, attr, span, target),
388390
[sym::path, ..] => self.check_generic_attr_unparsed(hir_id, attr, target, Target::Mod),
389391
[sym::macro_export, ..] => self.check_macro_export(hir_id, attr, target),
390-
[sym::should_panic, ..] => {
391-
self.check_generic_attr_unparsed(hir_id, attr, target, Target::Fn)
392-
}
393392
[sym::autodiff_forward, ..] | [sym::autodiff_reverse, ..] => {
394393
self.check_autodiff(hir_id, attr, span, target)
395394
}

‎tests/ui/attributes/check-builtin-attr-ice.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,10 @@
4444
struct Foo {
4545
#[should_panic::skip]
4646
//~^ ERROR failed to resolve
47-
//~| ERROR `#[should_panic::skip]` only has an effect on functions
4847
pub field: u8,
4948

5049
#[should_panic::a::b::c]
5150
//~^ ERROR failed to resolve
52-
//~| ERROR `#[should_panic::a::b::c]` only has an effect on functions
5351
pub field2: u8,
5452
}
5553

‎tests/ui/attributes/check-builtin-attr-ice.stderr

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,35 +5,17 @@ LL | #[should_panic::skip]
55
| ^^^^^^^^^^^^ use of unresolved module or unlinked crate `should_panic`
66

77
error[E0433]: failed to resolve: use of unresolved module or unlinked crate `should_panic`
8-
--> $DIR/check-builtin-attr-ice.rs:50:7
8+
--> $DIR/check-builtin-attr-ice.rs:49:7
99
|
1010
LL | #[should_panic::a::b::c]
1111
| ^^^^^^^^^^^^ use of unresolved module or unlinked crate `should_panic`
1212

1313
error[E0433]: failed to resolve: use of unresolved module or unlinked crate `deny`
14-
--> $DIR/check-builtin-attr-ice.rs:59:7
14+
--> $DIR/check-builtin-attr-ice.rs:57:7
1515
|
1616
LL | #[deny::skip]
1717
| ^^^^ use of unresolved module or unlinked crate `deny`
1818

19-
error: `#[should_panic::skip]` only has an effect on functions
20-
--> $DIR/check-builtin-attr-ice.rs:45:5
21-
|
22-
LL | #[should_panic::skip]
23-
| ^^^^^^^^^^^^^^^^^^^^^
24-
|
25-
note: the lint level is defined here
26-
--> $DIR/check-builtin-attr-ice.rs:42:9
27-
|
28-
LL | #![deny(unused_attributes)]
29-
| ^^^^^^^^^^^^^^^^^
30-
31-
error: `#[should_panic::a::b::c]` only has an effect on functions
32-
--> $DIR/check-builtin-attr-ice.rs:50:5
33-
|
34-
LL | #[should_panic::a::b::c]
35-
| ^^^^^^^^^^^^^^^^^^^^^^^^
36-
37-
error: aborting due to 5 previous errors
19+
error: aborting due to 3 previous errors
3820

3921
For more information about this error, try `rustc --explain E0433`.

‎tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -361,12 +361,6 @@ warning: crate-level attribute should be an inner attribute: add an exclamation
361361
LL | #[type_length_limit="0100"]
362362
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
363363

364-
warning: `#[should_panic]` only has an effect on functions
365-
--> $DIR/issue-43106-gating-of-builtin-attrs.rs:53:1
366-
|
367-
LL | #![should_panic]
368-
| ^^^^^^^^^^^^^^^^
369-
370364
warning: attribute should be applied to an `extern` block with non-Rust ABI
371365
--> $DIR/issue-43106-gating-of-builtin-attrs.rs:64:1
372366
|
@@ -409,6 +403,12 @@ warning: `#[proc_macro_derive]` only has an effect on functions
409403
LL | #![proc_macro_derive(Test)]
410404
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
411405

406+
warning: `#[should_panic]` only has an effect on functions
407+
--> $DIR/issue-43106-gating-of-builtin-attrs.rs:53:1
408+
|
409+
LL | #![should_panic]
410+
| ^^^^^^^^^^^^^^^^
411+
412412
warning: attribute should be applied to a function definition
413413
--> $DIR/issue-43106-gating-of-builtin-attrs.rs:62:1
414414
|

0 commit comments

Comments
 (0)