Improve error messages around invalid literals in attribute arguments

Signed-off-by: Jonathan Brouwer <[email protected]>

Author: JonathanBrouwer

compiler/rustc_attr_parsing/src/parser.rs

@@ -10,11 +10,11 @@ use rustc_ast::token::{self, Delimiter, MetaVarKind};
 use rustc_ast::tokenstream::TokenStream;
 use rustc_ast::{AttrArgs, DelimArgs, Expr, ExprKind, LitKind, MetaItemLit, NormalAttr, Path};
 use rustc_ast_pretty::pprust;
-use rustc_errors::PResult;
+use rustc_errors::{Diag, PResult};
 use rustc_hir::{self as hir, AttrPath};
 use rustc_parse::exp;
 use rustc_parse::parser::{Parser, PathStyle, token_descr};
-use rustc_session::errors::report_lit_error;
+use rustc_session::errors::{create_lit_error, report_lit_error};
 use rustc_session::parse::ParseSess;
 use rustc_span::{ErrorGuaranteed, Ident, Span, Symbol, sym};
 use thin_vec::ThinVec;
@@ -379,22 +379,23 @@ struct MetaItemListParserContext<'a, 'sess> {
 
 impl<'a, 'sess> MetaItemListParserContext<'a, 'sess> {
     fn parse_unsuffixed_meta_item_lit(&mut self) -> PResult<'sess, MetaItemLit> {
-        let uninterpolated_span = self.parser.token_uninterpolated_span();
-        let Some(token_lit) = self.parser.eat_token_lit() else {
-            return self.parser.handle_missing_lit(Parser::mk_meta_item_lit_char);
-        };
+        let Some(token_lit) = self.parser.eat_token_lit() else { return Err(self.expected_lit()) };
+        self.unsuffixed_meta_item_from_lit(token_lit)
+    }
 
+    fn unsuffixed_meta_item_from_lit(
+        &mut self,
+        token_lit: token::Lit,
+    ) -> PResult<'sess, MetaItemLit> {
         let lit = match MetaItemLit::from_token_lit(token_lit, self.parser.prev_token.span) {
             Ok(lit) => lit,
             Err(err) => {
-                let guar =
-                    report_lit_error(&self.parser.psess, err, token_lit, uninterpolated_span);
-                // Pack possible quotes and prefixes from the original literal into
-                // the error literal's symbol so they can be pretty-printed faithfully.
-                let suffixless_lit = token::Lit::new(token_lit.kind, token_lit.symbol, None);
-                let symbol = Symbol::intern(&suffixless_lit.to_string());
-                let token_lit = token::Lit::new(token::Err(guar), symbol, token_lit.suffix);
-                MetaItemLit::from_token_lit(token_lit, uninterpolated_span).unwrap()
+                return Err(create_lit_error(
+                    &self.parser.psess,
+                    err,
+                    token_lit,
+                    self.parser.prev_token_uninterpolated_span(),
+                ));
             }
         };
 
@@ -448,16 +449,28 @@ impl<'a, 'sess> MetaItemListParserContext<'a, 'sess> {
     }
 
     fn parse_meta_item_inner(&mut self) -> PResult<'sess, MetaItemOrLitParser<'static>> {
-        match self.parse_unsuffixed_meta_item_lit() {
-            Ok(lit) => return Ok(MetaItemOrLitParser::Lit(lit)),
-            Err(err) => err.cancel(), // we provide a better error below
-        }
-
-        match self.parse_attr_item() {
-            Ok(mi) => return Ok(MetaItemOrLitParser::MetaItemParser(mi)),
-            Err(err) => err.cancel(), // we provide a better error below
+        if let Some(token_lit) = self.parser.eat_token_lit() {
+            // If a literal token is parsed, we commit to parsing a MetaItemLit for better errors
+            Ok(MetaItemOrLitParser::Lit(self.unsuffixed_meta_item_from_lit(token_lit)?))
+        } else {
+            let prev_pros = self.parser.approx_token_stream_pos();
+            match self.parse_attr_item() {
+                Ok(item) => Ok(MetaItemOrLitParser::MetaItemParser(item)),
+                Err(err) => {
+                    // If `parse_attr_item` made any progress, it likely has a more precise error we should prefer
+                    // If it didn't make progress we use the `expected_lit` from below
+                    if self.parser.approx_token_stream_pos() != prev_pros {
+                        Err(err)
+                    } else {
+                        err.cancel();
+                        Err(self.expected_lit())
+                    }
+                }
+            }
         }
+    }
 
+    fn expected_lit(&mut self) -> Diag<'sess> {
         let mut err = InvalidMetaItem {
             span: self.parser.token.span,
             descr: token_descr(&self.parser.token),
@@ -492,7 +505,7 @@ impl<'a, 'sess> MetaItemListParserContext<'a, 'sess> {
             self.parser.bump();
         }
 
-        Err(self.parser.dcx().create_err(err))
+        self.parser.dcx().create_err(err)
     }
 
     fn parse(

compiler/rustc_parse/src/parser/expr.rs

@@ -2077,7 +2077,7 @@ impl<'a> Parser<'a> {
         (token::Lit { symbol: name, suffix: None, kind: token::Char }, span)
     }
 
-    pub fn mk_meta_item_lit_char(name: Symbol, span: Span) -> MetaItemLit {
+    fn mk_meta_item_lit_char(name: Symbol, span: Span) -> MetaItemLit {
         ast::MetaItemLit {
             symbol: name,
             suffix: None,
@@ -2086,7 +2086,7 @@ impl<'a> Parser<'a> {
         }
     }
 
-    pub fn handle_missing_lit<L>(
+    fn handle_missing_lit<L>(
         &mut self,
         mk_lit_char: impl FnOnce(Symbol, Span) -> L,
     ) -> PResult<'a, L> {

compiler/rustc_session/src/errors.rs

@@ -384,6 +384,10 @@ pub fn report_lit_error(
     lit: token::Lit,
     span: Span,
 ) -> ErrorGuaranteed {
+    create_lit_error(psess, err, lit, span).emit()
+}
+
+pub fn create_lit_error(psess: &ParseSess, err: LitError, lit: token::Lit, span: Span) -> Diag<'_> {
     // Checks if `s` looks like i32 or u1234 etc.
     fn looks_like_width_suffix(first_chars: &[char], s: &str) -> bool {
         s.len() > 1 && s.starts_with(first_chars) && s[1..].chars().all(|c| c.is_ascii_digit())
@@ -414,32 +418,32 @@ pub fn report_lit_error(
     let dcx = psess.dcx();
     match err {
         LitError::InvalidSuffix(suffix) => {
-            dcx.emit_err(InvalidLiteralSuffix { span, kind: lit.kind.descr(), suffix })
+            dcx.create_err(InvalidLiteralSuffix { span, kind: lit.kind.descr(), suffix })
         }
         LitError::InvalidIntSuffix(suffix) => {
             let suf = suffix.as_str();
             if looks_like_width_suffix(&['i', 'u'], suf) {
                 // If it looks like a width, try to be helpful.
-                dcx.emit_err(InvalidIntLiteralWidth { span, width: suf[1..].into() })
+                dcx.create_err(InvalidIntLiteralWidth { span, width: suf[1..].into() })
             } else if let Some(fixed) = fix_base_capitalisation(lit.symbol.as_str(), suf) {
-                dcx.emit_err(InvalidNumLiteralBasePrefix { span, fixed })
+                dcx.create_err(InvalidNumLiteralBasePrefix { span, fixed })
             } else {
-                dcx.emit_err(InvalidNumLiteralSuffix { span, suffix: suf.to_string() })
+                dcx.create_err(InvalidNumLiteralSuffix { span, suffix: suf.to_string() })
             }
         }
         LitError::InvalidFloatSuffix(suffix) => {
             let suf = suffix.as_str();
             if looks_like_width_suffix(&['f'], suf) {
                 // If it looks like a width, try to be helpful.
-                dcx.emit_err(InvalidFloatLiteralWidth { span, width: suf[1..].to_string() })
+                dcx.create_err(InvalidFloatLiteralWidth { span, width: suf[1..].to_string() })
             } else {
-                dcx.emit_err(InvalidFloatLiteralSuffix { span, suffix: suf.to_string() })
+                dcx.create_err(InvalidFloatLiteralSuffix { span, suffix: suf.to_string() })
             }
         }
         LitError::NonDecimalFloat(base) => match base {
-            16 => dcx.emit_err(HexadecimalFloatLiteralNotSupported { span }),
-            8 => dcx.emit_err(OctalFloatLiteralNotSupported { span }),
-            2 => dcx.emit_err(BinaryFloatLiteralNotSupported { span }),
+            16 => dcx.create_err(HexadecimalFloatLiteralNotSupported { span }),
+            8 => dcx.create_err(OctalFloatLiteralNotSupported { span }),
+            2 => dcx.create_err(BinaryFloatLiteralNotSupported { span }),
             _ => unreachable!(),
         },
         LitError::IntTooLarge(base) => {
@@ -450,7 +454,7 @@ pub fn report_lit_error(
                 16 => format!("{max:#x}"),
                 _ => format!("{max}"),
             };
-            dcx.emit_err(IntLiteralTooLarge { span, limit })
+            dcx.create_err(IntLiteralTooLarge { span, limit })
         }
     }
 }