Skip to content

Commit

Permalink
Rollup merge of #118855 - nnethercote:improve-attribute-value-error, …
Browse files Browse the repository at this point in the history
…r=compiler-errors,petrochenkov

Improve an error involving attribute values.

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.

r? `@compiler-errors`
  • Loading branch information
matthiaskrgr authored Dec 12, 2023
2 parents 111c40e + 226edf6 commit cd7809b
Show file tree
Hide file tree
Showing 17 changed files with 87 additions and 62 deletions.
60 changes: 38 additions & 22 deletions compiler/rustc_parse/src/validate_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/attributes/issue-90873.rs
Original file line number Diff line number Diff line change
@@ -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`
9 changes: 2 additions & 7 deletions tests/ui/attributes/issue-90873.stderr
Original file line number Diff line number Diff line change
@@ -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 {}}]
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/attributes/key-value-expansion-on-mac.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
2 changes: 1 addition & 1 deletion tests/ui/attributes/key-value-expansion-on-mac.stderr
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/attributes/key-value-expansion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
};
}

Expand Down
6 changes: 3 additions & 3 deletions tests/ui/attributes/key-value-expansion.stderr
Original file line number Diff line number Diff line change
@@ -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));
Expand All @@ -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()}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/attributes/unused-item-in-attr.rs
Original file line number Diff line number Diff line change
@@ -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() {}

Expand Down
4 changes: 1 addition & 3 deletions tests/ui/attributes/unused-item-in-attr.stderr
Original file line number Diff line number Diff line change
@@ -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; }]
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/consts/issue-90878-2.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/consts/issue-90878-2.stderr
Original file line number Diff line number Diff line change
@@ -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 ]]
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/malformed/malformed-interpolated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
4 changes: 2 additions & 2 deletions tests/ui/malformed/malformed-interpolated.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions tests/ui/parser/bad-lit-suffixes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
27 changes: 20 additions & 7 deletions tests/ui/parser/bad-lit-suffixes.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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

2 changes: 1 addition & 1 deletion tests/ui/parser/issues/issue-104620.rs
Original file line number Diff line number Diff line change
@@ -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() {}
6 changes: 4 additions & 2 deletions tests/ui/parser/issues/issue-104620.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit cd7809b

Please sign in to comment.