Skip to content

Commit 2183597

Browse files
tgross35tgross35
committed
mbe: Refactor diagnostics for missing identifiers
The current messages have the potential to be more accurate; "expected identifier or string literal" is printed in a few cases where only an identifier should be expected, and it suggests removing string literals when that might not solve the problem. Add a new diagnostic for these kind of errors that gives some more context. For `count` and `ignore` this should likely be combined with the diagnositcs for `eat_dollar` to produce a helpful error if they get anything other than a metavariable first argument. I am planning to do this in a followup.
1 parent 44ef2e8 commit 2183597

File tree

5 files changed

+125
-46
lines changed

5 files changed

+125
-46
lines changed

‎compiler/rustc_expand/messages.ftl‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,14 @@ expand_module_multiple_candidates =
133133
expand_must_repeat_once =
134134
this must repeat at least once
135135

136+
expand_mve_expected_ident =
137+
expected an identifier
138+
.not_ident = not a valid identifier
139+
.expr_name = expected a metavariable expression name: `{"${expr( /* ... */ )}"}`
140+
.expr_name_note = valid metavariable expressions are {$valid_expr_list}
141+
.ignore_expr_note = `ignore` takes a metavariable argument
142+
.count_expr_note = `count` takes a metavariable argument
143+
136144
expand_mve_unrecognized_var =
137145
variable `{$key}` is not recognized in meta-variable expression
138146

‎compiler/rustc_expand/src/errors.rs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,30 @@ pub(crate) use metavar_exprs::*;
497497
mod metavar_exprs {
498498
use super::*;
499499

500+
#[derive(Diagnostic)]
501+
#[diag(expand_mve_expected_ident)]
502+
pub(crate) struct MveExpectedIdent {
503+
#[primary_span]
504+
pub span: Span,
505+
#[label(expand_not_ident)]
506+
pub not_ident_label: Option<Span>,
507+
/// This error is reused a handful of places, the context here tells us how to customize
508+
/// the message.
509+
#[subdiagnostic]
510+
pub context: MveExpectedIdentContext,
511+
}
512+
513+
#[derive(Subdiagnostic)]
514+
pub(crate) enum MveExpectedIdentContext {
515+
#[note(expand_expr_name)]
516+
#[note(expand_expr_name_note)]
517+
ExprName { valid_expr_list: &'static str },
518+
#[note(expand_ignore_expr_note)]
519+
Ignore,
520+
#[note(expand_count_expr_note)]
521+
Count,
522+
}
523+
500524
#[derive(Diagnostic)]
501525
#[diag(expand_mve_unrecognized_var)]
502526
pub(crate) struct MveUnrecognizedVar {

‎compiler/rustc_expand/src/mbe/metavar_expr.rs‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,13 @@ use rustc_macros::{Decodable, Encodable};
77
use rustc_session::parse::ParseSess;
88
use rustc_span::{Ident, Span, Symbol};
99

10+
use crate::errors::{self, MveExpectedIdentContext};
11+
1012
pub(crate) const RAW_IDENT_ERR: &str = "`${concat(..)}` currently does not support raw identifiers";
1113
pub(crate) const UNSUPPORTED_CONCAT_ELEM_ERR: &str = "expected identifier or string literal";
1214

15+
const VALID_METAVAR_EXPR_NAMES: &str = "`count`, `ignore`, `index`, `len`, and `concat`";
16+
1317
/// A meta-variable expression, for expansions based on properties of meta-variables.
1418
#[derive(Debug, PartialEq, Encodable, Decodable)]
1519
pub(crate) enum MetaVarExpr {
@@ -39,7 +43,12 @@ impl MetaVarExpr {
3943
psess: &'psess ParseSess,
4044
) -> PResult<'psess, MetaVarExpr> {
4145
let mut iter = input.iter();
42-
let ident = parse_ident(&mut iter, psess, outer_span)?;
46+
let ident = parse_ident(
47+
&mut iter,
48+
psess,
49+
outer_span,
50+
MveExpectedIdentContext::ExprName { valid_expr_list: VALID_METAVAR_EXPR_NAMES },
51+
)?;
4352
let Some(TokenTree::Delimited(.., Delimiter::Parenthesis, args)) = iter.next() else {
4453
let msg = "meta-variable expression parameter must be wrapped in parentheses";
4554
return Err(psess.dcx().struct_span_err(ident.span, msg));
@@ -51,7 +60,9 @@ impl MetaVarExpr {
5160
"count" => parse_count(&mut iter, psess, ident.span)?,
5261
"ignore" => {
5362
eat_dollar(&mut iter, psess, ident.span)?;
54-
MetaVarExpr::Ignore(parse_ident(&mut iter, psess, ident.span)?)
63+
let ident =
64+
parse_ident(&mut iter, psess, outer_span, MveExpectedIdentContext::Ignore)?;
65+
MetaVarExpr::Ignore(ident)
5566
}
5667
"index" => MetaVarExpr::Index(parse_depth(&mut iter, psess, ident.span)?),
5768
"len" => MetaVarExpr::Len(parse_depth(&mut iter, psess, ident.span)?),
@@ -168,7 +179,7 @@ fn parse_count<'psess>(
168179
span: Span,
169180
) -> PResult<'psess, MetaVarExpr> {
170181
eat_dollar(iter, psess, span)?;
171-
let ident = parse_ident(iter, psess, span)?;
182+
let ident = parse_ident(iter, psess, span, MveExpectedIdentContext::Count)?;
172183
let depth = if try_eat_comma(iter) {
173184
if iter.peek().is_none() {
174185
return Err(psess.dcx().struct_span_err(
@@ -206,14 +217,32 @@ fn parse_depth<'psess>(
206217
}
207218
}
208219

209-
/// Parses an generic ident
220+
/// Tries to parse a generic ident. If this fails, create a missing identifier diagnostic with
221+
/// `context` explanation.
210222
fn parse_ident<'psess>(
211223
iter: &mut TokenStreamIter<'_>,
212224
psess: &'psess ParseSess,
213225
fallback_span: Span,
226+
context: MveExpectedIdentContext,
214227
) -> PResult<'psess, Ident> {
215-
let token = parse_token(iter, psess, fallback_span)?;
216-
parse_ident_from_token(psess, token)
228+
let Some(tt) = iter.next() else {
229+
let err = errors::MveExpectedIdent { span: fallback_span, not_ident_label: None, context };
230+
return Err(psess.dcx().create_err(err));
231+
};
232+
233+
let TokenTree::Token(token, _) = tt else {
234+
let span = tt.span();
235+
let err = errors::MveExpectedIdent { span, not_ident_label: Some(span), context };
236+
return Err(psess.dcx().create_err(err));
237+
};
238+
239+
let Some((elem, _)) = token.ident() else {
240+
let span = token.span;
241+
let err = errors::MveExpectedIdent { span, not_ident_label: Some(span), context };
242+
return Err(psess.dcx().create_err(err));
243+
};
244+
245+
Ok(elem)
217246
}
218247

219248
fn parse_ident_from_token<'psess>(

‎tests/ui/macros/metavar-expressions/syntax-errors.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,13 @@ macro_rules! mve_without_parens {
3636
#[rustfmt::skip]
3737
macro_rules! empty_expression {
3838
() => { ${} };
39-
//~^ ERROR expected identifier or string literal
39+
//~^ ERROR expected an identifier
4040
}
4141

4242
#[rustfmt::skip]
4343
macro_rules! open_brackets_with_lit {
4444
() => { ${ "hi" } };
45-
//~^ ERROR expected identifier
45+
//~^ ERROR expected an identifier
4646
}
4747

4848
macro_rules! mve_wrong_delim {
@@ -52,12 +52,13 @@ macro_rules! mve_wrong_delim {
5252

5353
macro_rules! invalid_metavar {
5454
() => { ${ignore($123)} }
55+
//~^ ERROR expected an identifier
5556
}
5657

5758
#[rustfmt::skip]
5859
macro_rules! open_brackets_with_group {
5960
( $( $i:ident ),* ) => { ${ {} } };
60-
//~^ ERROR expected identifier
61+
//~^ ERROR expected an identifier
6162
}
6263

6364
macro_rules! extra_garbage_after_metavar {

‎tests/ui/macros/metavar-expressions/syntax-errors.stderr‎

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -41,178 +41,195 @@ LL | ( $( $i:ident ),* ) => { ${ index(1u32) } };
4141
| ^^^^^
4242

4343
error: meta-variable expression parameter must be wrapped in parentheses
44-
--> $DIR/syntax-errors.rs:32:34
44+
--> $DIR/syntax-errors.rs:32:33
4545
|
46-
LL | ( $( $i:ident ),* ) => { ${ count } };
47-
| ^^^^^
46+
LL | ( $( $i:ident ),* ) => { ${ count } };
47+
| ^^^^^
4848

4949
error: meta-variable expression parameter must be wrapped in parentheses
5050
--> $DIR/syntax-errors.rs:49:33
5151
|
5252
LL | ( $( $i:ident ),* ) => { ${ count{i} } };
5353
| ^^^^^
5454

55+
error: expected an identifier
56+
--> $DIR/syntax-errors.rs:54:23
57+
|
58+
LL | () => { ${ignore($123)} }
59+
| ^^^ not a valid identifier
60+
|
61+
= note: `ignore` takes a metavariable argument
62+
5563
error: unexpected token: a
56-
--> $DIR/syntax-errors.rs:61:19
64+
--> $DIR/syntax-errors.rs:66:19
5765
|
5866
LL | ${count() a b c}
5967
| ^
6068
|
6169
note: meta-variable expression must not have trailing tokens
62-
--> $DIR/syntax-errors.rs:61:19
70+
--> $DIR/syntax-errors.rs:66:19
6371
|
6472
LL | ${count() a b c}
6573
| ^
6674

6775
error: unexpected token: a
68-
--> $DIR/syntax-errors.rs:63:20
76+
--> $DIR/syntax-errors.rs:68:20
6977
|
7078
LL | ${count($i a b c)}
7179
| ^
7280
|
7381
note: meta-variable expression must not have trailing tokens
74-
--> $DIR/syntax-errors.rs:63:20
82+
--> $DIR/syntax-errors.rs:68:20
7583
|
7684
LL | ${count($i a b c)}
7785
| ^
7886

7987
error: unexpected token: a
80-
--> $DIR/syntax-errors.rs:65:23
88+
--> $DIR/syntax-errors.rs:70:23
8189
|
8290
LL | ${count($i, 1 a b c)}
8391
| ^
8492
|
8593
note: meta-variable expression must not have trailing tokens
86-
--> $DIR/syntax-errors.rs:65:23
94+
--> $DIR/syntax-errors.rs:70:23
8795
|
8896
LL | ${count($i, 1 a b c)}
8997
| ^
9098

9199
error: unexpected token: a
92-
--> $DIR/syntax-errors.rs:67:21
100+
--> $DIR/syntax-errors.rs:72:21
93101
|
94102
LL | ${count($i) a b c}
95103
| ^
96104
|
97105
note: meta-variable expression must not have trailing tokens
98-
--> $DIR/syntax-errors.rs:67:21
106+
--> $DIR/syntax-errors.rs:72:21
99107
|
100108
LL | ${count($i) a b c}
101109
| ^
102110

103111
error: unexpected token: a
104-
--> $DIR/syntax-errors.rs:70:22
112+
--> $DIR/syntax-errors.rs:75:22
105113
|
106114
LL | ${ignore($i) a b c}
107115
| ^
108116
|
109117
note: meta-variable expression must not have trailing tokens
110-
--> $DIR/syntax-errors.rs:70:22
118+
--> $DIR/syntax-errors.rs:75:22
111119
|
112120
LL | ${ignore($i) a b c}
113121
| ^
114122

115123
error: unexpected token: a
116-
--> $DIR/syntax-errors.rs:72:21
124+
--> $DIR/syntax-errors.rs:77:21
117125
|
118126
LL | ${ignore($i a b c)}
119127
| ^
120128
|
121129
note: meta-variable expression must not have trailing tokens
122-
--> $DIR/syntax-errors.rs:72:21
130+
--> $DIR/syntax-errors.rs:77:21
123131
|
124132
LL | ${ignore($i a b c)}
125133
| ^
126134

127135
error: unexpected token: a
128-
--> $DIR/syntax-errors.rs:75:19
136+
--> $DIR/syntax-errors.rs:80:19
129137
|
130138
LL | ${index() a b c}
131139
| ^
132140
|
133141
note: meta-variable expression must not have trailing tokens
134-
--> $DIR/syntax-errors.rs:75:19
142+
--> $DIR/syntax-errors.rs:80:19
135143
|
136144
LL | ${index() a b c}
137145
| ^
138146

139147
error: unexpected token: a
140-
--> $DIR/syntax-errors.rs:77:19
148+
--> $DIR/syntax-errors.rs:82:19
141149
|
142150
LL | ${index(1 a b c)}
143151
| ^
144152
|
145153
note: meta-variable expression must not have trailing tokens
146-
--> $DIR/syntax-errors.rs:77:19
154+
--> $DIR/syntax-errors.rs:82:19
147155
|
148156
LL | ${index(1 a b c)}
149157
| ^
150158

151159
error: unexpected token: a
152-
--> $DIR/syntax-errors.rs:80:19
160+
--> $DIR/syntax-errors.rs:85:19
153161
|
154162
LL | ${index() a b c}
155163
| ^
156164
|
157165
note: meta-variable expression must not have trailing tokens
158-
--> $DIR/syntax-errors.rs:80:19
166+
--> $DIR/syntax-errors.rs:85:19
159167
|
160168
LL | ${index() a b c}
161169
| ^
162170

163171
error: unexpected token: a
164-
--> $DIR/syntax-errors.rs:82:19
172+
--> $DIR/syntax-errors.rs:87:19
165173
|
166174
LL | ${index(1 a b c)}
167175
| ^
168176
|
169177
note: meta-variable expression must not have trailing tokens
170-
--> $DIR/syntax-errors.rs:82:19
178+
--> $DIR/syntax-errors.rs:87:19
171179
|
172180
LL | ${index(1 a b c)}
173181
| ^
174182

175183
error: meta-variable expression depth must be a literal
176-
--> $DIR/syntax-errors.rs:89:33
184+
--> $DIR/syntax-errors.rs:94:33
177185
|
178186
LL | ( $( $i:ident ),* ) => { ${ index(IDX) } };
179187
| ^^^^^
180188

181189
error: meta-variables within meta-variable expressions must be referenced using a dollar sign
182-
--> $DIR/syntax-errors.rs:95:11
190+
--> $DIR/syntax-errors.rs:100:11
183191
|
184192
LL | ${count(foo)}
185193
| ^^^^^
186194

187195
error: meta-variables within meta-variable expressions must be referenced using a dollar sign
188-
--> $DIR/syntax-errors.rs:102:11
196+
--> $DIR/syntax-errors.rs:107:11
189197
|
190198
LL | ${ignore(bar)}
191199
| ^^^^^^
192200

193201
error: unrecognized meta-variable expression
194-
--> $DIR/syntax-errors.rs:108:33
202+
--> $DIR/syntax-errors.rs:113:33
195203
|
196204
LL | ( $( $i:ident ),* ) => { ${ aaaaaaaaaaaaaa(i) } };
197205
| ^^^^^^^^^^^^^^ help: supported expressions are count, ignore, index and len
198206

199-
error: expected identifier or string literal
200-
--> $DIR/syntax-errors.rs:38:15
207+
error: expected an identifier
208+
--> $DIR/syntax-errors.rs:38:14
209+
|
210+
LL | () => { ${} };
211+
| ^^
201212
|
202-
LL | () => { ${} };
203-
| ^^
213+
= note: expected a metavariable expression name: `${expr( /* ... */ )}`
214+
= note: valid metavariable expressions are `count`, `ignore`, `index`, `len`, and `concat`
204215

205-
error: expected identifier, found `"hi"`
216+
error: expected an identifier
206217
--> $DIR/syntax-errors.rs:44:17
207218
|
208219
LL | () => { ${ "hi" } };
209-
| ^^^^ help: try removing `"hi"`
220+
| ^^^^ not a valid identifier
221+
|
222+
= note: expected a metavariable expression name: `${expr( /* ... */ )}`
223+
= note: valid metavariable expressions are `count`, `ignore`, `index`, `len`, and `concat`
210224

211-
error: expected identifier or string literal
212-
--> $DIR/syntax-errors.rs:55:33
225+
error: expected an identifier
226+
--> $DIR/syntax-errors.rs:60:33
213227
|
214228
LL | ( $( $i:ident ),* ) => { ${ {} } };
215-
| ^^
229+
| ^^ not a valid identifier
230+
|
231+
= note: expected a metavariable expression name: `${expr( /* ... */ )}`
232+
= note: valid metavariable expressions are `count`, `ignore`, `index`, `len`, and `concat`
216233

217-
error: aborting due to 24 previous errors
234+
error: aborting due to 25 previous errors
218235

0 commit comments

Comments
 (0)