Improve an error involving attribute values.

nnethercote · nnethercote · commit 226edf64fa58 · 2023-12-12T15:54:25.000+11:00
Attribute values must be literals. The error you get when that doesn't
hold is pretty bad, e.g.:
```
unexpected expression: 1 + 1
```
You also get the same error if the attribute value is a literal, but an
invalid literal, e.g.:
```
unexpected expression: "foo"suffix
```

This commit does two things.
- Changes the error message to "attribute value must be a literal",
  which gives a better idea of what the problem is and how to fix it. It
  also no longer prints the invalid expression, because the carets below
  highlight it anyway.
- Separates the "not a literal" case from the "invalid literal" case.
  Which means invalid literals now get the specific error at the literal
  level, rather than at the attribute level.
diff --git a/compiler/rustc_parse/src/validate_attr.rs b/compiler/rustc_parse/src/validate_attr.rs
@@ -6,9 +6,9 @@ use rustc_ast::token::Delimiter;
 use rustc_ast::tokenstream::DelimSpan;
 use rustc_ast::MetaItemKind;
 use rustc_ast::{self as ast, AttrArgs, AttrArgsEq, Attribute, DelimArgs, MetaItem};
-use rustc_ast_pretty::pprust;
 use rustc_errors::{Applicability, FatalError, PResult};
 use rustc_feature::{AttributeTemplate, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP};
+use rustc_session::errors::report_lit_error;
 use rustc_session::lint::builtin::ILL_FORMED_ATTRIBUTE_INPUT;
 use rustc_session::parse::ParseSess;
 use rustc_span::{sym, Span, Symbol};
@@ -51,28 +51,44 @@ pub fn parse_meta<'a>(sess: &'a ParseSess, attr: &Attribute) -> PResult<'a, Meta
                 MetaItemKind::List(nmis)
             }
             AttrArgs::Eq(_, AttrArgsEq::Ast(expr)) => {
-                if let ast::ExprKind::Lit(token_lit) = expr.kind
-                    && let Ok(lit) = ast::MetaItemLit::from_token_lit(token_lit, expr.span)
-                {
-                    if token_lit.suffix.is_some() {
-                        let mut err = sess.span_diagnostic.struct_span_err(
-                            expr.span,
-                            "suffixed literals are not allowed in attributes",
-                        );
-                        err.help(
-                            "instead of using a suffixed literal (`1u8`, `1.0f32`, etc.), \
-                            use an unsuffixed version (`1`, `1.0`, etc.)",
-                        );
-                        return Err(err);
-                    } else {
-                        MetaItemKind::NameValue(lit)
-                    }
+                if let ast::ExprKind::Lit(token_lit) = expr.kind {
+                    let res = ast::MetaItemLit::from_token_lit(token_lit, expr.span);
+                    let res = match res {
+                        Ok(lit) => {
+                            if token_lit.suffix.is_some() {
+                                let mut err = sess.span_diagnostic.struct_span_err(
+                                    expr.span,
+                                    "suffixed literals are not allowed in attributes",
+                                );
+                                err.help(
+                                    "instead of using a suffixed literal (`1u8`, `1.0f32`, etc.), \
+                                    use an unsuffixed version (`1`, `1.0`, etc.)",
+                                );
+                                return Err(err);
+                            } else {
+                                MetaItemKind::NameValue(lit)
+                            }
+                        }
+                        Err(err) => {
+                            report_lit_error(sess, err, token_lit, expr.span);
+                            let lit = ast::MetaItemLit {
+                                symbol: token_lit.symbol,
+                                suffix: token_lit.suffix,
+                                kind: ast::LitKind::Err,
+                                span: expr.span,
+                            };
+                            MetaItemKind::NameValue(lit)
+                        }
+                    };
+                    res
                 } else {
-                    // The non-error case can happen with e.g. `#[foo = 1+1]`. The error case can
-                    // happen with e.g. `#[foo = include_str!("nonexistent-file.rs")]`; in that
-                    // case we delay the error because an earlier error will have already been
-                    // reported.
-                    let msg = format!("unexpected expression: `{}`", pprust::expr_to_string(expr));
+                    // 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.
+                    let msg = format!("attribute value must be a literal");
                     let mut err = sess.span_diagnostic.struct_span_err(expr.span, msg);
                     if let ast::ExprKind::Err = expr.kind {
                         err.downgrade_to_delayed_bug();
diff --git a/tests/ui/attributes/issue-90873.rs b/tests/ui/attributes/issue-90873.rs
@@ -1,9 +1,9 @@
 #![u=||{static d=||1;}]
-//~^ unexpected expression
+//~^ attribute value must be a literal
 //~| cannot find attribute `u` in this scope
 //~| missing type for `static` item
 
 #![a={impl std::ops::Neg for i8 {}}]
-//~^ ERROR unexpected expression
+//~^ ERROR attribute value must be a literal
 //~| ERROR cannot find attribute `a` in this scope
 //~| ERROR `main` function not found in crate `issue_90873`
diff --git a/tests/ui/attributes/issue-90873.stderr b/tests/ui/attributes/issue-90873.stderr
@@ -1,15 +1,10 @@
-error: unexpected expression: `||
-           {
-               static d: _ = || 1;
-           }`
+error: attribute value must be a literal
   --> $DIR/issue-90873.rs:1:6
    |
 LL | #![u=||{static d=||1;}]
    |      ^^^^^^^^^^^^^^^^^
 
-error: unexpected expression: `{
-           impl std::ops::Neg for i8 {}
-       }`
+error: attribute value must be a literal
   --> $DIR/issue-90873.rs:6:6
    |
 LL | #![a={impl std::ops::Neg for i8 {}}]
diff --git a/tests/ui/attributes/key-value-expansion-on-mac.rs b/tests/ui/attributes/key-value-expansion-on-mac.rs
@@ -7,8 +7,8 @@ macro_rules! bar {
 
 // FIXME?: `bar` here expands before `stringify` has a chance to expand.
 // `#[rustc_dummy = ...]` is validated and dropped during expansion of `bar`,
-// the "unexpected expression" errors comes from the validation.
-#[rustc_dummy = stringify!(b)] //~ ERROR unexpected expression: `stringify!(b)`
+// the "attribute value must be a literal" error comes from the validation.
+#[rustc_dummy = stringify!(b)] //~ ERROR attribute value must be a literal
 bar!();
 
 fn main() {}
diff --git a/tests/ui/attributes/key-value-expansion-on-mac.stderr b/tests/ui/attributes/key-value-expansion-on-mac.stderr
@@ -1,4 +1,4 @@
-error: unexpected expression: `stringify!(b)`
+error: attribute value must be a literal
   --> $DIR/key-value-expansion-on-mac.rs:11:17
    |
 LL | #[rustc_dummy = stringify!(b)]
diff --git a/tests/ui/attributes/key-value-expansion.rs b/tests/ui/attributes/key-value-expansion.rs
@@ -18,13 +18,13 @@ macro_rules! bug {
 
 // Any expressions containing macro call `X` that's more complex than `X` itself.
 // Parentheses will work.
-bug!((column!())); //~ ERROR unexpected expression: `(7u32)`
+bug!((column!())); //~ ERROR attribute value must be a literal
 
 // Original test case.
 
 macro_rules! bug {
     () => {
-        bug!("bug" + stringify!(found)); //~ ERROR unexpected expression: `"bug" + "found"`
+        bug!("bug" + stringify!(found)); //~ ERROR attribute value must be a literal
     };
     ($test:expr) => {
         #[doc = $test]
@@ -46,7 +46,7 @@ macro_rules! doc_comment {
 macro_rules! some_macro {
     ($t1: ty) => {
         doc_comment! {format!("{coor}", coor = stringify!($t1)).as_str()}
-        //~^ ERROR unexpected expression: `{
+        //~^ ERROR attribute value must be a literal
     };
 }
 
diff --git a/tests/ui/attributes/key-value-expansion.stderr b/tests/ui/attributes/key-value-expansion.stderr
@@ -1,10 +1,10 @@
-error: unexpected expression: `(7u32)`
+error: attribute value must be a literal
   --> $DIR/key-value-expansion.rs:21:6
    |
 LL | bug!((column!()));
    |      ^^^^^^^^^^^
 
-error: unexpected expression: `"bug" + "found"`
+error: attribute value must be a literal
   --> $DIR/key-value-expansion.rs:27:14
    |
 LL |         bug!("bug" + stringify!(found));
@@ -15,7 +15,7 @@ LL | bug!();
    |
    = note: this error originates in the macro `bug` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: unexpected expression: `{ let res = ::alloc::fmt::format(format_args!("{0}", "u8")); res }.as_str()`
+error: attribute value must be a literal
   --> $DIR/key-value-expansion.rs:48:23
    |
 LL |         doc_comment! {format!("{coor}", coor = stringify!($t1)).as_str()}
diff --git a/tests/ui/attributes/unused-item-in-attr.rs b/tests/ui/attributes/unused-item-in-attr.rs
@@ -1,5 +1,5 @@
 #[w = { extern crate alloc; }]
-//~^ ERROR unexpected expression: `{
+//~^ ERROR attribute value must be a literal
 //~| ERROR cannot find attribute `w` in this scope
 fn f() {}
 
diff --git a/tests/ui/attributes/unused-item-in-attr.stderr b/tests/ui/attributes/unused-item-in-attr.stderr
@@ -1,6 +1,4 @@
-error: unexpected expression: `{
-           extern crate alloc;
-       }`
+error: attribute value must be a literal
   --> $DIR/unused-item-in-attr.rs:1:7
    |
 LL | #[w = { extern crate alloc; }]
diff --git a/tests/ui/consts/issue-90878-2.rs b/tests/ui/consts/issue-90878-2.rs
@@ -1,4 +1,4 @@
- #![l=|x|[b;x ]] //~ ERROR unexpected expression: `|x| [b; x]`
+ #![l=|x|[b;x ]] //~ ERROR attribute value must be a literal
 //~^ ERROR cannot find attribute `l` in this scope
 
 // notice the space at the start,
diff --git a/tests/ui/consts/issue-90878-2.stderr b/tests/ui/consts/issue-90878-2.stderr
@@ -1,4 +1,4 @@
-error: unexpected expression: `|x| [b; x]`
+error: attribute value must be a literal
   --> $DIR/issue-90878-2.rs:1:7
    |
 LL |  #![l=|x|[b;x ]]
diff --git a/tests/ui/malformed/malformed-interpolated.rs b/tests/ui/malformed/malformed-interpolated.rs
@@ -10,7 +10,7 @@ macro_rules! check {
 check!("0"); // OK
 check!(0); // OK
 check!(0u8); //~ ERROR suffixed literals are not allowed in attributes
-check!(-0); //~ ERROR unexpected expression: `-0`
-check!(0 + 0); //~ ERROR unexpected expression: `0 + 0`
+check!(-0); //~ ERROR attribute value must be a literal
+check!(0 + 0); //~ ERROR attribute value must be a literal
 
 fn main() {}
diff --git a/tests/ui/malformed/malformed-interpolated.stderr b/tests/ui/malformed/malformed-interpolated.stderr
@@ -6,13 +6,13 @@ LL | check!(0u8);
    |
    = help: instead of using a suffixed literal (`1u8`, `1.0f32`, etc.), use an unsuffixed version (`1`, `1.0`, etc.)
 
-error: unexpected expression: `-0`
+error: attribute value must be a literal
   --> $DIR/malformed-interpolated.rs:13:8
    |
 LL | check!(-0);
    |        ^^
 
-error: unexpected expression: `0 + 0`
+error: attribute value must be a literal
   --> $DIR/malformed-interpolated.rs:14:8
    |
 LL | check!(0 + 0);
diff --git a/tests/ui/parser/bad-lit-suffixes.rs b/tests/ui/parser/bad-lit-suffixes.rs
@@ -28,11 +28,12 @@ fn main() {
 }
 
 #[rustc_dummy = "string"suffix]
-//~^ ERROR unexpected expression: `"string"suffix`
+//~^ ERROR suffixes on string literals are invalid
 fn f() {}
 
 #[must_use = "string"suffix]
-//~^ ERROR unexpected expression: `"string"suffix`
+//~^ ERROR suffixes on string literals are invalid
+//~| ERROR malformed `must_use` attribute input
 fn g() {}
 
 #[link(name = "string"suffix)]
diff --git a/tests/ui/parser/bad-lit-suffixes.stderr b/tests/ui/parser/bad-lit-suffixes.stderr
@@ -10,26 +10,39 @@ error: suffixes on string literals are invalid
 LL |     "C"suffix
    |     ^^^^^^^^^ invalid suffix `suffix`
 
-error: unexpected expression: `"string"suffix`
+error: suffixes on string literals are invalid
   --> $DIR/bad-lit-suffixes.rs:30:17
    |
 LL | #[rustc_dummy = "string"suffix]
-   |                 ^^^^^^^^^^^^^^
+   |                 ^^^^^^^^^^^^^^ invalid suffix `suffix`
 
-error: unexpected expression: `"string"suffix`
+error: suffixes on string literals are invalid
   --> $DIR/bad-lit-suffixes.rs:34:14
    |
 LL | #[must_use = "string"suffix]
-   |              ^^^^^^^^^^^^^^
+   |              ^^^^^^^^^^^^^^ invalid suffix `suffix`
+
+error: malformed `must_use` attribute input
+  --> $DIR/bad-lit-suffixes.rs:34:1
+   |
+LL | #[must_use = "string"suffix]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: the following are the possible correct uses
+   |
+LL | #[must_use = "reason"]
+   |
+LL | #[must_use]
+   |
 
 error: suffixes on string literals are invalid
-  --> $DIR/bad-lit-suffixes.rs:38:15
+  --> $DIR/bad-lit-suffixes.rs:39:15
    |
 LL | #[link(name = "string"suffix)]
    |               ^^^^^^^^^^^^^^ invalid suffix `suffix`
 
 error: invalid suffix `suffix` for number literal
-  --> $DIR/bad-lit-suffixes.rs:42:41
+  --> $DIR/bad-lit-suffixes.rs:43:41
    |
 LL | #[rustc_layout_scalar_valid_range_start(0suffix)]
    |                                         ^^^^^^^ invalid suffix `suffix`
@@ -136,5 +149,5 @@ LL |     1.0e10suffix;
    |
    = help: valid suffixes are `f32` and `f64`
 
-error: aborting due to 20 previous errors
+error: aborting due to 21 previous errors
 
diff --git a/tests/ui/parser/issues/issue-104620.rs b/tests/ui/parser/issues/issue-104620.rs
@@ -1,4 +1,4 @@
 #![feature(rustc_attrs)]
 
-#![rustc_dummy=5z] //~ ERROR unexpected expression: `5z`
+#![rustc_dummy=5z] //~ ERROR invalid suffix `z` for number literal
 fn main() {}
diff --git a/tests/ui/parser/issues/issue-104620.stderr b/tests/ui/parser/issues/issue-104620.stderr
@@ -1,8 +1,10 @@
-error: unexpected expression: `5z`
+error: invalid suffix `z` for number literal
   --> $DIR/issue-104620.rs:3:16
    |
 LL | #![rustc_dummy=5z]
-   |                ^^
+   |                ^^ invalid suffix `z`
+   |
+   = help: the suffix must be one of the numeric types (`u32`, `isize`, `f32`, etc.)
 
 error: aborting due to 1 previous error
 

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		-error: unexpected expression: `stringify!(b)`
	`1`	`+error: attribute value must be a literal`
`2`	`2`	`--> $DIR/key-value-expansion-on-mac.rs:11:17`
`3`	`3`	`\|`
`4`	`4`	`LL \| #[rustc_dummy = stringify!(b)]`
Original file line number	Diff line number	Diff line change
`@@ -1,10 +1,10 @@`
`1`		-error: unexpected expression: `(7u32)`
	`1`	`+error: attribute value must be a literal`
`2`	`2`	`--> $DIR/key-value-expansion.rs:21:6`
`3`	`3`	`\|`
`4`	`4`	`LL \| bug!((column!()));`
`5`	`5`	`\| ^^^^^^^^^^^`
`6`	`6`
`7`		-error: unexpected expression: `"bug" + "found"`
	`7`	`+error: attribute value must be a literal`
`8`	`8`	`--> $DIR/key-value-expansion.rs:27:14`
`9`	`9`	`\|`
`10`	`10`	`LL \| bug!("bug" + stringify!(found));`
`@@ -15,7 +15,7 @@ LL \| bug!();`
`15`	`15`	`\|`
`16`	`16`	= note: this error originates in the macro `bug` (in Nightly builds, run with -Z macro-backtrace for more info)
`17`	`17`
`18`		-error: unexpected expression: `{ let res = ::alloc::fmt::format(format_args!("{0}", "u8")); res }.as_str()`
	`18`	`+error: attribute value must be a literal`
`19`	`19`	`--> $DIR/key-value-expansion.rs:48:23`
`20`	`20`	`\|`
`21`	`21`	`LL \| doc_comment! {format!("{coor}", coor = stringify!($t1)).as_str()}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,4 @@`
`1`		-error: unexpected expression: `{
`2`		`- extern crate alloc;`
`3`		- }`
	`1`	`+error: attribute value must be a literal`
`4`	`2`	`--> $DIR/unused-item-in-attr.rs:1:7`
`5`	`3`	`\|`
`6`	`4`	`LL \| #[w = { extern crate alloc; }]`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		- #![l=\|x\|[b;x ]] //~ ERROR unexpected expression: `\|x\| [b; x]`
	`1`	`+ #![l=\|x\|[b;x ]] //~ ERROR attribute value must be a literal`
`2`	`2`	//~^ ERROR cannot find attribute `l` in this scope
`3`	`3`
`4`	`4`	`// notice the space at the start,`