Skip to content

Commit 93ed397

Browse files
tgross35tgross35
committed
mbe: Refactor diagnostics for invalid metavar expression syntax
Give a more user-friendly diagnostic about the following: * Trailing tokens within braces, e.g. `${foo() extra}` * Missing parentheses, e.g. `${foo}` * Incorrect number of arguments, with a hint about correct usage.
1 parent 837c5dd commit 93ed397

File tree

5 files changed

+234
-122
lines changed

5 files changed

+234
-122
lines changed

‎compiler/rustc_expand/messages.ftl

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,27 @@ expand_module_multiple_candidates =
133133
expand_must_repeat_once =
134134
this must repeat at least once
135135
136+
expand_mve_extra_tokens =
137+
unexpected trailing tokens
138+
.label = for this metavariable expression
139+
.range = the `{$name}` metavariable expression takes between {$min_or_exact_args} and {$max_args} arguments
140+
.exact = the `{$name}` metavariable expression takes {$min_or_exact_args ->
141+
[zero] no arguments
142+
[one] a single argument
143+
*[other] {$min_or_exact_args} arguments
144+
}
145+
.suggestion = try removing {$extra_count ->
146+
[one] this token
147+
*[other] these tokens
148+
}
149+
150+
expand_mve_missing_paren =
151+
expected `(`
152+
.label = for this this metavariable expression
153+
.unexpected = unexpected token
154+
.note = metavariable expressions use function-like parentheses syntax
155+
.suggestion = try adding parentheses
156+
136157
expand_mve_unrecognized_var =
137158
variable `{$key}` is not recognized in meta-variable expression
138159

‎compiler/rustc_expand/src/errors.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,40 @@ pub(crate) use metavar_exprs::*;
496496
mod metavar_exprs {
497497
use super::*;
498498

499+
#[derive(Diagnostic, Default)]
500+
#[diag(expand_mve_extra_tokens)]
501+
pub(crate) struct MveExtraTokens {
502+
#[primary_span]
503+
#[suggestion(code = "", applicability = "machine-applicable")]
504+
pub span: Span,
505+
#[label]
506+
pub ident_span: Span,
507+
pub extra_count: usize,
508+
509+
// The rest is only used for specific diagnostics and can be default if neither
510+
// `note` is `Some`.
511+
#[note(expand_exact)]
512+
pub exact_args_note: Option<()>,
513+
#[note(expand_range)]
514+
pub range_args_note: Option<()>,
515+
pub min_or_exact_args: usize,
516+
pub max_args: usize,
517+
pub name: String,
518+
}
519+
520+
#[derive(Diagnostic)]
521+
#[note]
522+
#[diag(expand_mve_missing_paren)]
523+
pub(crate) struct MveMissingParen {
524+
#[primary_span]
525+
#[label]
526+
pub ident_span: Span,
527+
#[label(expand_unexpected)]
528+
pub unexpected_span: Option<Span>,
529+
#[suggestion(code = "( /* ... */ )", applicability = "has-placeholders")]
530+
pub insert_span: Option<Span>,
531+
}
532+
499533
#[derive(Diagnostic)]
500534
#[diag(expand_mve_unrecognized_var)]
501535
pub(crate) struct MveUnrecognizedVar {

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

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

10+
use crate::errors;
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

@@ -40,11 +42,32 @@ impl MetaVarExpr {
4042
) -> PResult<'psess, MetaVarExpr> {
4143
let mut iter = input.iter();
4244
let ident = parse_ident(&mut iter, psess, outer_span)?;
43-
let Some(TokenTree::Delimited(.., Delimiter::Parenthesis, args)) = iter.next() else {
44-
let msg = "meta-variable expression parameter must be wrapped in parentheses";
45-
return Err(psess.dcx().struct_span_err(ident.span, msg));
45+
let next = iter.next();
46+
let Some(TokenTree::Delimited(.., Delimiter::Parenthesis, args)) = next else {
47+
// No `()`; wrong or no delimiters. Point at a problematic span or a place to
48+
// add parens if it makes sense.
49+
let (unexpected_span, insert_span) = match next {
50+
Some(TokenTree::Delimited(..)) => (None, None),
51+
Some(tt) => (Some(tt.span()), None),
52+
None => (None, Some(ident.span.shrink_to_hi())),
53+
};
54+
let err =
55+
errors::MveMissingParen { ident_span: ident.span, unexpected_span, insert_span };
56+
return Err(psess.dcx().create_err(err));
4657
};
47-
check_trailing_token(&mut iter, psess)?;
58+
59+
// Ensure there are no trailing tokens in the braces, e.g. `${foo() extra}`
60+
if iter.peek().is_some() {
61+
let span = iter_span(&iter).expect("checked is_some above");
62+
let err = errors::MveExtraTokens {
63+
span,
64+
ident_span: ident.span,
65+
extra_count: iter.count(),
66+
..Default::default()
67+
};
68+
return Err(psess.dcx().create_err(err));
69+
}
70+
4871
let mut iter = args.iter();
4972
let rslt = match ident.as_str() {
5073
"concat" => parse_concat(&mut iter, psess, outer_span, ident.span)?,
@@ -67,7 +90,7 @@ impl MetaVarExpr {
6790
return Err(err);
6891
}
6992
};
70-
check_trailing_token(&mut iter, psess)?;
93+
check_trailing_tokens(&mut iter, psess, ident)?;
7194
Ok(rslt)
7295
}
7396

@@ -87,20 +110,51 @@ impl MetaVarExpr {
87110
}
88111
}
89112

90-
// Checks if there are any remaining tokens. For example, `${ignore(ident ... a b c ...)}`
91-
fn check_trailing_token<'psess>(
113+
/// Checks if there are any remaining tokens (for example, `${ignore($valid, extra)}`) and create
114+
/// a diag with the correct arg count if so.
115+
fn check_trailing_tokens<'psess>(
92116
iter: &mut TokenStreamIter<'_>,
93117
psess: &'psess ParseSess,
118+
ident: Ident,
94119
) -> PResult<'psess, ()> {
95-
if let Some(tt) = iter.next() {
96-
let mut diag = psess
97-
.dcx()
98-
.struct_span_err(tt.span(), format!("unexpected token: {}", pprust::tt_to_string(tt)));
99-
diag.span_note(tt.span(), "meta-variable expression must not have trailing tokens");
100-
Err(diag)
101-
} else {
102-
Ok(())
120+
if iter.peek().is_none() {
121+
// All tokens consumed, as expected
122+
return Ok(());
103123
}
124+
125+
// `None` for max indicates the arg count must be exact, `Some` indicates a range is accepted.
126+
let (min_or_exact_args, max_args) = match ident.as_str() {
127+
"concat" => panic!("concat takes unlimited tokens but didn't eat them all"),
128+
"ignore" => (1, None),
129+
// 1 or 2 args
130+
"count" => (1, Some(2)),
131+
// 0 or 1 arg
132+
"index" => (0, Some(1)),
133+
"len" => (0, Some(1)),
134+
other => unreachable!("unknown MVEs should be rejected earlier (got `{other}`)"),
135+
};
136+
137+
let err = errors::MveExtraTokens {
138+
span: iter_span(iter).expect("checked is_none above"),
139+
ident_span: ident.span,
140+
extra_count: iter.count(),
141+
142+
exact_args_note: if max_args.is_some() { None } else { Some(()) },
143+
range_args_note: if max_args.is_some() { Some(()) } else { None },
144+
min_or_exact_args,
145+
max_args: max_args.unwrap_or_default(),
146+
name: ident.to_string(),
147+
};
148+
Err(psess.dcx().create_err(err))
149+
}
150+
151+
/// Returns a span encompassing all tokens in the iterator if there is at least one item.
152+
fn iter_span(iter: &TokenStreamIter<'_>) -> Option<Span> {
153+
let mut iter = iter.clone(); // cloning is cheap
154+
let first_sp = iter.next()?.span();
155+
let last_sp = iter.last().map(TokenTree::span).unwrap_or(first_sp);
156+
let span = first_sp.with_hi(last_sp.hi());
157+
Some(span)
104158
}
105159

106160
/// Indicates what is placed in a `concat` parameter. For example, literals

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

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ macro_rules! metavar_with_literal_suffix {
3030

3131
macro_rules! mve_without_parens {
3232
( $( $i:ident ),* ) => { ${ count } };
33-
//~^ ERROR meta-variable expression parameter must be wrapped in parentheses
33+
//~^ ERROR expected `(`
3434
}
3535

3636
#[rustfmt::skip]
@@ -45,9 +45,14 @@ macro_rules! open_brackets_with_lit {
4545
//~^ ERROR expected identifier
4646
}
4747

48+
macro_rules! mvs_missing_paren {
49+
( $( $i:ident ),* ) => { ${ count $i ($i) } };
50+
//~^ ERROR expected `(`
51+
}
52+
4853
macro_rules! mve_wrong_delim {
4954
( $( $i:ident ),* ) => { ${ count{i} } };
50-
//~^ ERROR meta-variable expression parameter must be wrapped in parentheses
55+
//~^ ERROR expected `(`
5156
}
5257

5358
macro_rules! invalid_metavar {
@@ -64,28 +69,30 @@ macro_rules! open_brackets_with_group {
6469
macro_rules! extra_garbage_after_metavar {
6570
( $( $i:ident ),* ) => {
6671
${count() a b c}
67-
//~^ ERROR unexpected token: a
72+
//~^ ERROR unexpected trailing tokens
6873
${count($i a b c)}
69-
//~^ ERROR unexpected token: a
74+
//~^ ERROR unexpected trailing tokens
7075
${count($i, 1 a b c)}
71-
//~^ ERROR unexpected token: a
76+
//~^ ERROR unexpected trailing tokens
7277
${count($i) a b c}
73-
//~^ ERROR unexpected token: a
78+
//~^ ERROR unexpected trailing tokens
7479

7580
${ignore($i) a b c}
76-
//~^ ERROR unexpected token: a
81+
//~^ ERROR unexpected trailing tokens
7782
${ignore($i a b c)}
78-
//~^ ERROR unexpected token: a
83+
//~^ ERROR unexpected trailing tokens
7984

8085
${index() a b c}
81-
//~^ ERROR unexpected token: a
86+
//~^ ERROR unexpected trailing tokens
8287
${index(1 a b c)}
83-
//~^ ERROR unexpected token: a
88+
//~^ ERROR unexpected trailing tokens
8489

8590
${index() a b c}
86-
//~^ ERROR unexpected token: a
91+
//~^ ERROR unexpected trailing tokens
8792
${index(1 a b c)}
88-
//~^ ERROR unexpected token: a
93+
//~^ ERROR unexpected trailing tokens
94+
${index(1, a b c)}
95+
//~^ ERROR unexpected trailing tokens
8996
};
9097
}
9198

0 commit comments

Comments
 (0)