Rollup merge of rust-lang#151516 - mejrs:ignore_nonmeta, r=jdonszelmann,jonathanbrouwer

Do not emit errors on non-metaitem diagnostic attr input

This unblocks porting over diagnostic attributes by properly addressing rust-lang#151056 (comment)

r? @JonathanBrouwer

Author: matthiaskrgr

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,

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(),

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(),
         }
     }

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<MetaItemLit> {
+) -> PResult<'sess, Option<MetaItemLit>> {
     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)

compiler/rustc_builtin_macros/src/cfg.rs

@@ -40,8 +40,11 @@ fn parse_cfg(cx: &ExtCtxt<'_>, span: Span, tts: TokenStream) -> Result<CfgEntry,
         return Err(cx.dcx().emit_err(errors::RequiresCfgPattern { span }));
     }
 
-    let meta = MetaItemOrLitParser::parse_single(&mut parser, ShouldEmit::ErrorsAndLints)
-        .map_err(|diag| diag.emit())?;
+    let meta = MetaItemOrLitParser::parse_single(
+        &mut parser,
+        ShouldEmit::ErrorsAndLints { recover: true },
+    )
+    .map_err(|diag| diag.emit())?;
     let cfg = AttributeParser::parse_single_args(
         cx.sess,
         span,
@@ -55,7 +58,7 @@ fn parse_cfg(cx: &ExtCtxt<'_>, span: Span, tts: TokenStream) -> Result<CfgEntry,
         // Doesn't matter what the target actually is here.
         Target::Crate,
         Some(cx.ecfg.features),
-        ShouldEmit::ErrorsAndLints,
+        ShouldEmit::ErrorsAndLints { recover: true },
         &meta,
         parse_cfg_entry,
         &CFG_TEMPLATE,

compiler/rustc_expand/src/config.rs

@@ -393,9 +393,10 @@ impl<'a> 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 {

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 {