From 215a82c7df4fa6bf5a3967c31a4129709e692ef0 Mon Sep 17 00:00:00 2001 From: mejrs <59372212+mejrs@users.noreply.github.com> Date: Thu, 22 Jan 2026 23:42:01 +0100 Subject: [PATCH] Do not emit errors on non-metaitem diagnostic attr input --- .../rustc_attr_parsing/src/attributes/cfg.rs | 5 +- .../src/attributes/cfg_select.rs | 7 +- compiler/rustc_attr_parsing/src/context.rs | 21 ++-- compiler/rustc_attr_parsing/src/parser.rs | 99 +++++++++++-------- compiler/rustc_builtin_macros/src/cfg.rs | 9 +- compiler/rustc_expand/src/config.rs | 7 +- compiler/rustc_expand/src/expand.rs | 4 +- 7 files changed, 90 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_attr_parsing/src/attributes/cfg.rs b/compiler/rustc_attr_parsing/src/attributes/cfg.rs index a9b2a4c96c334..3a540d80998d0 100644 --- a/compiler/rustc_attr_parsing/src/attributes/cfg.rs +++ b/compiler/rustc_attr_parsing/src/attributes/cfg.rs @@ -360,7 +360,8 @@ fn parse_cfg_attr_internal<'a>( ) -> PResult<'a, (CfgEntry, Vec<(ast::AttrItem, Span)>)> { // Parse cfg predicate let pred_start = parser.token.span; - let meta = MetaItemOrLitParser::parse_single(parser, ShouldEmit::ErrorsAndLints)?; + let meta = + MetaItemOrLitParser::parse_single(parser, ShouldEmit::ErrorsAndLints { recover: true })?; let pred_span = pred_start.with_hi(parser.token.span.hi()); let cfg_predicate = AttributeParser::parse_single_args( @@ -375,7 +376,7 @@ fn parse_cfg_attr_internal<'a>( CRATE_NODE_ID, Target::Crate, features, - ShouldEmit::ErrorsAndLints, + ShouldEmit::ErrorsAndLints { recover: true }, &meta, parse_cfg_entry, &CFG_ATTR_TEMPLATE, diff --git a/compiler/rustc_attr_parsing/src/attributes/cfg_select.rs b/compiler/rustc_attr_parsing/src/attributes/cfg_select.rs index e80084021a849..ca844758daaa6 100644 --- a/compiler/rustc_attr_parsing/src/attributes/cfg_select.rs +++ b/compiler/rustc_attr_parsing/src/attributes/cfg_select.rs @@ -78,8 +78,9 @@ pub fn parse_cfg_select( } } } else { - let meta = MetaItemOrLitParser::parse_single(p, ShouldEmit::ErrorsAndLints) - .map_err(|diag| diag.emit())?; + let meta = + MetaItemOrLitParser::parse_single(p, ShouldEmit::ErrorsAndLints { recover: true }) + .map_err(|diag| diag.emit())?; let cfg_span = meta.span(); let cfg = AttributeParser::parse_single_args( sess, @@ -94,7 +95,7 @@ pub fn parse_cfg_select( // Doesn't matter what the target actually is here. Target::Crate, features, - ShouldEmit::ErrorsAndLints, + ShouldEmit::ErrorsAndLints { recover: true }, &meta, parse_cfg_entry, &AttributeTemplate::default(), diff --git a/compiler/rustc_attr_parsing/src/context.rs b/compiler/rustc_attr_parsing/src/context.rs index e500be68e2410..20bdfa45a013d 100644 --- a/compiler/rustc_attr_parsing/src/context.rs +++ b/compiler/rustc_attr_parsing/src/context.rs @@ -381,7 +381,7 @@ impl Stage for Late { } fn should_emit(&self) -> ShouldEmit { - ShouldEmit::ErrorsAndLints + ShouldEmit::ErrorsAndLints { recover: true } } } @@ -438,7 +438,7 @@ impl<'f, 'sess: 'f, S: Stage> SharedContext<'f, 'sess, S> { pub(crate) fn emit_lint(&mut self, lint: &'static Lint, kind: AttributeLintKind, span: Span) { if !matches!( self.stage.should_emit(), - ShouldEmit::ErrorsAndLints | ShouldEmit::EarlyFatal { also_emit_lints: true } + ShouldEmit::ErrorsAndLints { .. } | ShouldEmit::EarlyFatal { also_emit_lints: true } ) { return; } @@ -765,9 +765,18 @@ pub enum ShouldEmit { EarlyFatal { also_emit_lints: bool }, /// The operation will emit errors and lints. /// This is usually what you need. - ErrorsAndLints, - /// The operation will emit *not* errors and lints. - /// Use this if you are *sure* that this operation will be called at a different time with `ShouldEmit::ErrorsAndLints`. + ErrorsAndLints { + /// Whether [`ArgParser`] will attempt to recover from errors. + /// + /// If true, it will attempt to recover from bad input (like an invalid literal). Setting + /// this to false will instead return early, and not raise errors except at the top level + /// (in [`ArgParser::from_attr_args`]). + recover: bool, + }, + /// The operation will *not* emit errors and lints. + /// + /// The parser can still call `delay_bug`, so you *must* ensure that this operation will also be + /// called with `ShouldEmit::ErrorsAndLints`. Nothing, } @@ -776,7 +785,7 @@ impl ShouldEmit { match self { ShouldEmit::EarlyFatal { .. } if diag.level() == Level::DelayedBug => diag.emit(), ShouldEmit::EarlyFatal { .. } => diag.upgrade_to_fatal().emit(), - ShouldEmit::ErrorsAndLints => diag.emit(), + ShouldEmit::ErrorsAndLints { .. } => diag.emit(), ShouldEmit::Nothing => diag.delay_as_bug(), } } diff --git a/compiler/rustc_attr_parsing/src/parser.rs b/compiler/rustc_attr_parsing/src/parser.rs index 99e36a78d6aeb..7f3c6d28005ff 100644 --- a/compiler/rustc_attr_parsing/src/parser.rs +++ b/compiler/rustc_attr_parsing/src/parser.rs @@ -16,7 +16,7 @@ use rustc_errors::{Diag, PResult}; use rustc_hir::{self as hir, AttrPath}; use rustc_parse::exp; use rustc_parse::parser::{ForceCollect, Parser, PathStyle, token_descr}; -use rustc_session::errors::{create_lit_error, report_lit_error}; +use rustc_session::errors::create_lit_error; use rustc_session::parse::ParseSess; use rustc_span::{Ident, Span, Symbol, sym}; use thin_vec::ThinVec; @@ -113,16 +113,29 @@ impl ArgParser { Some(match value { AttrArgs::Empty => Self::NoArgs, AttrArgs::Delimited(args) => { - // The arguments of rustc_dummy and diagnostic::do_not_recommend are not validated - // if the arguments are delimited. - // See https://doc.rust-lang.org/reference/attributes/diagnostics.html#r-attributes.diagnostic.namespace.unknown-invalid-syntax - if parts == &[sym::rustc_dummy] - || parts == &[sym::diagnostic, sym::do_not_recommend] - { - return Some(ArgParser::List(MetaItemListParser { - sub_parsers: ThinVec::new(), - span: args.dspan.entire(), - })); + // Diagnostic attributes can't error if they encounter non meta item syntax. + // However, the current syntax for diagnostic attributes is meta item syntax. + // Therefore we can substitute with a dummy value on invalid syntax. + if matches!(parts, [sym::rustc_dummy] | [sym::diagnostic, ..]) { + match MetaItemListParser::new( + &args.tokens, + args.dspan.entire(), + psess, + ShouldEmit::ErrorsAndLints { recover: false }, + ) { + Ok(p) => return Some(ArgParser::List(p)), + Err(e) => { + // We can just dispose of the diagnostic and not bother with a lint, + // because this will look like `#[diagnostic::attr()]` was used. This + // is invalid for all diagnostic attrs, so a lint explaining the proper + // form will be issued later. + e.cancel(); + return Some(ArgParser::List(MetaItemListParser { + sub_parsers: ThinVec::new(), + span: args.dspan.entire(), + })); + } + } } if args.delim != Delimiter::Parenthesis { @@ -141,7 +154,9 @@ impl ArgParser { } AttrArgs::Eq { eq_span, expr } => Self::NameValue(NameValueParser { eq_span: *eq_span, - value: expr_to_lit(psess, &expr, expr.span, should_emit)?, + value: expr_to_lit(psess, &expr, expr.span, should_emit) + .map_err(|e| should_emit.emit_err(e)) + .ok()??, value_span: expr.span, }), }) @@ -336,58 +351,53 @@ impl NameValueParser { } } -fn expr_to_lit( - psess: &ParseSess, +fn expr_to_lit<'sess>( + psess: &'sess ParseSess, expr: &Expr, span: Span, should_emit: ShouldEmit, -) -> Option { +) -> PResult<'sess, Option> { if let ExprKind::Lit(token_lit) = expr.kind { let res = MetaItemLit::from_token_lit(token_lit, expr.span); match res { Ok(lit) => { if token_lit.suffix.is_some() { - should_emit.emit_err( - psess.dcx().create_err(SuffixedLiteralInAttribute { span: lit.span }), - ); - None + Err(psess.dcx().create_err(SuffixedLiteralInAttribute { span: lit.span })) } else { - if !lit.kind.is_unsuffixed() { - // Emit error and continue, we can still parse the attribute as if the suffix isn't there - should_emit.emit_err( - psess.dcx().create_err(SuffixedLiteralInAttribute { span: lit.span }), - ); + if lit.kind.is_unsuffixed() { + Ok(Some(lit)) + } else { + Err(psess.dcx().create_err(SuffixedLiteralInAttribute { span: lit.span })) } - - Some(lit) } } Err(err) => { - let guar = report_lit_error(psess, err, token_lit, expr.span); - let lit = MetaItemLit { - symbol: token_lit.symbol, - suffix: token_lit.suffix, - kind: LitKind::Err(guar), - span: expr.span, - }; - Some(lit) + let err = create_lit_error(psess, err, token_lit, expr.span); + if matches!(should_emit, ShouldEmit::ErrorsAndLints { recover: false }) { + Err(err) + } else { + let lit = MetaItemLit { + symbol: token_lit.symbol, + suffix: token_lit.suffix, + kind: LitKind::Err(err.emit()), + span: expr.span, + }; + Ok(Some(lit)) + } } } } else { if matches!(should_emit, ShouldEmit::Nothing) { - return None; + return Ok(None); } // Example cases: // - `#[foo = 1+1]`: results in `ast::ExprKind::BinOp`. // - `#[foo = include_str!("nonexistent-file.rs")]`: - // results in `ast::ExprKind::Err`. In that case we delay - // the error because an earlier error will have already - // been reported. + // results in `ast::ExprKind::Err`. let msg = "attribute value must be a literal"; let err = psess.dcx().struct_span_err(span, msg); - should_emit.emit_err(err); - None + Err(err) } } @@ -420,9 +430,12 @@ impl<'a, 'sess> MetaItemListParserContext<'a, 'sess> { if !lit.kind.is_unsuffixed() { // Emit error and continue, we can still parse the attribute as if the suffix isn't there - self.should_emit.emit_err( - self.parser.dcx().create_err(SuffixedLiteralInAttribute { span: lit.span }), - ); + let err = self.parser.dcx().create_err(SuffixedLiteralInAttribute { span: lit.span }); + if matches!(self.should_emit, ShouldEmit::ErrorsAndLints { recover: false }) { + return Err(err); + } else { + self.should_emit.emit_err(err) + }; } Ok(lit) diff --git a/compiler/rustc_builtin_macros/src/cfg.rs b/compiler/rustc_builtin_macros/src/cfg.rs index be1ce5a06d5ea..8e925cfe09a2e 100644 --- a/compiler/rustc_builtin_macros/src/cfg.rs +++ b/compiler/rustc_builtin_macros/src/cfg.rs @@ -40,8 +40,11 @@ fn parse_cfg(cx: &ExtCtxt<'_>, span: Span, tts: TokenStream) -> Result, span: Span, tts: TokenStream) -> Result StripUnconfigured<'a> { /// Determines if a node with the given attributes should be included in this configuration. fn in_cfg(&self, attrs: &[Attribute]) -> bool { - attrs - .iter() - .all(|attr| !is_cfg(attr) || self.cfg_true(attr, ShouldEmit::ErrorsAndLints).as_bool()) + attrs.iter().all(|attr| { + !is_cfg(attr) + || self.cfg_true(attr, ShouldEmit::ErrorsAndLints { recover: true }).as_bool() + }) } pub(crate) fn cfg_true(&self, attr: &Attribute, emit_errors: ShouldEmit) -> EvalConfigResult { diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 20cdffac2d670..2cbb97b117ee5 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -2170,7 +2170,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { call.span(), self.cx.current_expansion.lint_node_id, Some(self.cx.ecfg.features), - ShouldEmit::ErrorsAndLints, + ShouldEmit::ErrorsAndLints { recover: true }, ); let current_span = if let Some(sp) = span { sp.to(attr.span) } else { attr.span }; @@ -2220,7 +2220,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { // Target doesn't matter for `cfg` parsing. Target::Crate, self.cfg().features, - ShouldEmit::ErrorsAndLints, + ShouldEmit::ErrorsAndLints { recover: true }, parse_cfg, &CFG_TEMPLATE, ) else {