Skip to content

Commit 727bf84

Browse files
nnethercotennethercote
committed
Remove NtPat.
The one notable test change is `tests/ui/macros/trace_faulty_macros.rs`. This commit removes the complicated `Interpolated` handling in `expected_expression_found` that results in a longer error message. But I think the new, shorter message is actually an improvement. The original complaint was in rust-lang#71039, when the error message started with "error: expected expression, found `1 + 1`". That was confusing because `1 + 1` is an expression. Other than that, the reporter said "the whole error message is not too bad if you ignore the first line". Subsequently, extra complexity and wording was added to the error message. But I don't think the extra wording actually helps all that much. In particular, it still says of the `1+1` that "this is expected to be expression". This repeats the problem from the original complaint! This commit removes the extra complexity, reverting to a simpler error message. This is primarily because the traversal a pain without `Interpolated` tokens. Nonetheless, I think the error message is *improved*. It now starts with "expected expression, found `pat` metavariable", which is much clearer and the real problem. It also doesn't say anything specific about `1+1`, which is good, because the `1+1` isn't really relevant to the error -- it's the `$e:pat` that's important.
1 parent 5ec7dc4 commit 727bf84

16 files changed

+65
-97
lines changed

‎compiler/rustc_ast/src/ast_traits.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,6 @@ impl HasTokens for Nonterminal {
201201
Nonterminal::NtItem(item) => item.tokens(),
202202
Nonterminal::NtStmt(stmt) => stmt.tokens(),
203203
Nonterminal::NtExpr(expr) | Nonterminal::NtLiteral(expr) => expr.tokens(),
204-
Nonterminal::NtPat(pat) => pat.tokens(),
205204
Nonterminal::NtMeta(attr_item) => attr_item.tokens(),
206205
Nonterminal::NtPath(path) => path.tokens(),
207206
Nonterminal::NtBlock(block) => block.tokens(),
@@ -212,7 +211,6 @@ impl HasTokens for Nonterminal {
212211
Nonterminal::NtItem(item) => item.tokens_mut(),
213212
Nonterminal::NtStmt(stmt) => stmt.tokens_mut(),
214213
Nonterminal::NtExpr(expr) | Nonterminal::NtLiteral(expr) => expr.tokens_mut(),
215-
Nonterminal::NtPat(pat) => pat.tokens_mut(),
216214
Nonterminal::NtMeta(attr_item) => attr_item.tokens_mut(),
217215
Nonterminal::NtPath(path) => path.tokens_mut(),
218216
Nonterminal::NtBlock(block) => block.tokens_mut(),

‎compiler/rustc_ast/src/mut_visit.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -810,7 +810,6 @@ fn visit_nonterminal<T: MutVisitor>(nt: &mut token::Nonterminal, vis: &mut T) {
810810
vis.flat_map_stmt(stmt).expect_one("expected visitor to produce exactly one item")
811811
})
812812
}),
813-
token::NtPat(pat) => vis.visit_pat(pat),
814813
token::NtExpr(expr) => vis.visit_expr(expr),
815814
token::NtLiteral(expr) => vis.visit_expr(expr),
816815
token::NtMeta(item) => {

‎compiler/rustc_ast/src/token.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,6 @@ impl Token {
637637
| Lt | BinOp(Shl) // associated path
638638
| PathSep => true, // global path
639639
Interpolated(ref nt) => matches!(&**nt, NtLiteral(..) |
640-
NtPat(..) |
641640
NtBlock(..) |
642641
NtPath(..)),
643642
OpenDelim(Delimiter::Invisible(InvisibleOrigin::MetaVar(
@@ -1042,7 +1041,6 @@ pub enum Nonterminal {
10421041
NtItem(P<ast::Item>),
10431042
NtBlock(P<ast::Block>),
10441043
NtStmt(P<ast::Stmt>),
1045-
NtPat(P<ast::Pat>),
10461044
NtExpr(P<ast::Expr>),
10471045
NtLiteral(P<ast::Expr>),
10481046
/// Stuff inside brackets for attributes
@@ -1139,7 +1137,6 @@ impl Nonterminal {
11391137
NtItem(item) => item.span,
11401138
NtBlock(block) => block.span,
11411139
NtStmt(stmt) => stmt.span,
1142-
NtPat(pat) => pat.span,
11431140
NtExpr(expr) | NtLiteral(expr) => expr.span,
11441141
NtMeta(attr_item) => attr_item.span(),
11451142
NtPath(path) => path.span,
@@ -1151,7 +1148,6 @@ impl Nonterminal {
11511148
NtItem(..) => "item",
11521149
NtBlock(..) => "block",
11531150
NtStmt(..) => "statement",
1154-
NtPat(..) => "pattern",
11551151
NtExpr(..) => "expression",
11561152
NtLiteral(..) => "literal",
11571153
NtMeta(..) => "attribute",
@@ -1176,7 +1172,6 @@ impl fmt::Debug for Nonterminal {
11761172
NtItem(..) => f.pad("NtItem(..)"),
11771173
NtBlock(..) => f.pad("NtBlock(..)"),
11781174
NtStmt(..) => f.pad("NtStmt(..)"),
1179-
NtPat(..) => f.pad("NtPat(..)"),
11801175
NtExpr(..) => f.pad("NtExpr(..)"),
11811176
NtLiteral(..) => f.pad("NtLiteral(..)"),
11821177
NtMeta(..) => f.pad("NtMeta(..)"),

‎compiler/rustc_ast/src/tokenstream.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,6 @@ impl TokenStream {
468468
TokenStream::token_alone(token::Semi, stmt.span)
469469
}
470470
Nonterminal::NtStmt(stmt) => TokenStream::from_ast(stmt),
471-
Nonterminal::NtPat(pat) => TokenStream::from_ast(pat),
472471
Nonterminal::NtMeta(attr) => TokenStream::from_ast(attr),
473472
Nonterminal::NtPath(path) => TokenStream::from_ast(path),
474473
Nonterminal::NtExpr(expr) | Nonterminal::NtLiteral(expr) => TokenStream::from_ast(expr),

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,9 @@ pub(super) fn transcribe<'a>(
299299
let kind = token::NtLifetime(*ident);
300300
TokenTree::token_alone(kind, sp)
301301
}
302+
MatchedSingle(ParseNtResult::Pat(pat, pat_kind)) => {
303+
mk_delimited(MetaVarKind::Pat(*pat_kind), TokenStream::from_ast(pat))
304+
}
302305
MatchedSingle(ParseNtResult::Ty(ty)) => {
303306
mk_delimited(MetaVarKind::Ty, TokenStream::from_ast(ty))
304307
}

‎compiler/rustc_parse/src/parser/diagnostics.rs

Lines changed: 2 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,11 @@ use ast::token::IdentIsRaw;
2424
use rustc_ast as ast;
2525
use rustc_ast::ptr::P;
2626
use rustc_ast::token::{self, Delimiter, Lit, LitKind, Token, TokenKind};
27-
use rustc_ast::tokenstream::AttrTokenTree;
2827
use rustc_ast::util::parser::AssocOp;
2928
use rustc_ast::{
3029
AngleBracketedArg, AngleBracketedArgs, AnonConst, AttrVec, BinOpKind, BindingMode, Block,
31-
BlockCheckMode, Expr, ExprKind, GenericArg, Generics, HasTokens, Item, ItemKind, Param, Pat,
32-
PatKind, Path, PathSegment, QSelf, Recovered, Ty, TyKind,
30+
BlockCheckMode, Expr, ExprKind, GenericArg, Generics, Item, ItemKind, Param, Pat, PatKind,
31+
Path, PathSegment, QSelf, Recovered, Ty, TyKind,
3332
};
3433
use rustc_ast_pretty::pprust;
3534
use rustc_data_structures::fx::FxHashSet;
@@ -2348,52 +2347,6 @@ impl<'a> Parser<'a> {
23482347
err.subdiagnostic(ExprParenthesesNeeded::surrounding(*sp));
23492348
}
23502349
err.span_label(span, "expected expression");
2351-
2352-
// Walk the chain of macro expansions for the current token to point at how the original
2353-
// code was interpreted. This helps the user realize when a macro argument of one type is
2354-
// later reinterpreted as a different type, like `$x:expr` being reinterpreted as `$x:pat`
2355-
// in a subsequent macro invocation (#71039).
2356-
let mut tok = self.token.clone();
2357-
let mut labels = vec![];
2358-
while let TokenKind::Interpolated(nt) = &tok.kind {
2359-
let tokens = nt.tokens();
2360-
labels.push(nt.clone());
2361-
if let Some(tokens) = tokens
2362-
&& let tokens = tokens.to_attr_token_stream()
2363-
&& let tokens = tokens.0.deref()
2364-
&& let [AttrTokenTree::Token(token, _)] = &tokens[..]
2365-
{
2366-
tok = token.clone();
2367-
} else {
2368-
break;
2369-
}
2370-
}
2371-
let mut iter = labels.into_iter().peekable();
2372-
let mut show_link = false;
2373-
while let Some(nt) = iter.next() {
2374-
let descr = nt.descr();
2375-
if let Some(next) = iter.peek() {
2376-
let next_descr = next.descr();
2377-
if next_descr != descr {
2378-
err.span_label(next.use_span(), format!("this is expected to be {next_descr}"));
2379-
err.span_label(
2380-
nt.use_span(),
2381-
format!(
2382-
"this is interpreted as {}, but it is expected to be {}",
2383-
next_descr, descr,
2384-
),
2385-
);
2386-
show_link = true;
2387-
}
2388-
}
2389-
}
2390-
if show_link {
2391-
err.note(
2392-
"when forwarding a matched fragment to another macro-by-example, matchers in the \
2393-
second macro will see an opaque AST of the fragment type, not the underlying \
2394-
tokens",
2395-
);
2396-
}
23972350
err
23982351
}
23992352

‎compiler/rustc_parse/src/parser/item.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::maybe_whole;
77
use ast::token::IdentIsRaw;
88
use rustc_ast::ast::*;
99
use rustc_ast::ptr::P;
10-
use rustc_ast::token::{self, Delimiter, TokenKind};
10+
use rustc_ast::token::{self, Delimiter, InvisibleOrigin, MetaVarKind, TokenKind};
1111
use rustc_ast::tokenstream::{DelimSpan, TokenStream, TokenTree};
1212
use rustc_ast::util::case::Case;
1313
use rustc_ast::{self as ast};
@@ -2963,8 +2963,10 @@ impl<'a> Parser<'a> {
29632963

29642964
fn is_named_param(&self) -> bool {
29652965
let offset = match &self.token.kind {
2966-
token::Interpolated(nt) => match &**nt {
2967-
token::NtPat(..) => return self.look_ahead(1, |t| t == &token::Colon),
2966+
token::OpenDelim(Delimiter::Invisible(origin)) => match origin {
2967+
InvisibleOrigin::MetaVar(MetaVarKind::Pat(_)) => {
2968+
return self.check_noexpect_past_close_delim(&token::Colon);
2969+
}
29682970
_ => 0,
29692971
},
29702972
token::BinOp(token::And) | token::AndAnd => 1,

‎compiler/rustc_parse/src/parser/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ use path::PathStyle;
2020

2121
use rustc_ast::ptr::P;
2222
use rustc_ast::token::{
23-
self, Delimiter, IdentIsRaw, InvisibleOrigin, MetaVarKind, Nonterminal, Token, TokenKind,
23+
self, Delimiter, IdentIsRaw, InvisibleOrigin, MetaVarKind, Nonterminal, NtPatKind, Token,
24+
TokenKind,
2425
};
2526
use rustc_ast::tokenstream::{AttrsTarget, DelimSpacing, DelimSpan, Spacing};
2627
use rustc_ast::tokenstream::{TokenStream, TokenTree, TokenTreeCursor};
@@ -1693,6 +1694,7 @@ pub enum ParseNtResult {
16931694
Tt(TokenTree),
16941695
Ident(Ident, IdentIsRaw),
16951696
Lifetime(Ident),
1697+
Pat(P<ast::Pat>, NtPatKind),
16961698
Ty(P<ast::Ty>),
16971699
Vis(P<ast::Visibility>),
16981700

‎compiler/rustc_parse/src/parser/nonterminal.rs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ impl<'a> Parser<'a> {
4747
fn nt_may_be_ident(nt: &Nonterminal) -> bool {
4848
match nt {
4949
NtStmt(_)
50-
| NtPat(_)
5150
| NtExpr(_)
5251
| NtLiteral(_) // `true`, `false`
5352
| NtMeta(_)
@@ -88,7 +87,7 @@ impl<'a> Parser<'a> {
8887
token::NtLifetime(..) => true,
8988
token::Interpolated(nt) => match &**nt {
9089
NtBlock(_) | NtStmt(_) | NtExpr(_) | NtLiteral(_) => true,
91-
NtItem(_) | NtPat(_) | NtMeta(_) | NtPath(_) => false,
90+
NtItem(_) | NtMeta(_) | NtPath(_) => false,
9291
},
9392
token::OpenDelim(Delimiter::Invisible(InvisibleOrigin::MetaVar(k))) => match k {
9493
MetaVarKind::Block
@@ -180,15 +179,18 @@ impl<'a> Parser<'a> {
180179
}
181180
},
182181
NonterminalKind::Pat(pat_kind) => {
183-
NtPat(self.collect_tokens_no_attrs(|this| match pat_kind {
184-
PatParam { .. } => this.parse_pat_no_top_alt(None, None),
185-
PatWithOr => this.parse_pat_allow_top_alt(
186-
None,
187-
RecoverComma::No,
188-
RecoverColon::No,
189-
CommaRecoveryMode::EitherTupleOrPipe,
190-
),
191-
})?)
182+
return Ok(ParseNtResult::Pat(
183+
self.collect_tokens_no_attrs(|this| match pat_kind {
184+
PatParam { .. } => this.parse_pat_no_top_alt(None, None),
185+
PatWithOr => this.parse_pat_allow_top_alt(
186+
None,
187+
RecoverComma::No,
188+
RecoverColon::No,
189+
CommaRecoveryMode::EitherTupleOrPipe,
190+
),
191+
})?,
192+
pat_kind,
193+
));
192194
}
193195
NonterminalKind::Expr(_) => NtExpr(self.parse_expr_force_collect()?),
194196
NonterminalKind::Literal => {

‎compiler/rustc_parse/src/parser/pat.rs

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ use crate::errors::{
1010
UnexpectedParenInRangePat, UnexpectedParenInRangePatSugg,
1111
UnexpectedVertVertBeforeFunctionParam, UnexpectedVertVertInPattern, WrapInParens,
1212
};
13+
use crate::maybe_recover_from_interpolated_ty_qpath;
1314
use crate::parser::expr::{could_be_unclosed_char_literal, LhsExpr};
14-
use crate::{maybe_recover_from_interpolated_ty_qpath, maybe_whole};
1515
use rustc_ast::mut_visit::{noop_visit_pat, MutVisitor};
1616
use rustc_ast::ptr::P;
17-
use rustc_ast::token::{self, BinOpToken, Delimiter, Token};
17+
use rustc_ast::token::{self, BinOpToken, Delimiter, MetaVarKind, NtPatKind::*, Token};
1818
use rustc_ast::{
1919
self as ast, AttrVec, BindingMode, ByRef, Expr, ExprKind, MacCall, Mutability, Pat, PatField,
2020
PatFieldsRest, PatKind, Path, QSelf, RangeEnd, RangeSyntax,
@@ -429,6 +429,27 @@ impl<'a> Parser<'a> {
429429
None
430430
}
431431

432+
fn eat_metavar_pat(&mut self) -> Option<P<Pat>> {
433+
// Must try both kinds of pattern nonterminals.
434+
if let Some(pat) = self.eat_metavar_seq_with_matcher(
435+
|mv_kind| matches!(mv_kind, MetaVarKind::Pat(PatParam { .. })),
436+
|this| this.parse_pat_no_top_alt(None, None),
437+
) {
438+
Some(pat)
439+
} else if let Some(pat) = self.eat_metavar_seq(MetaVarKind::Pat(PatWithOr), |this| {
440+
this.parse_pat_allow_top_alt(
441+
None,
442+
RecoverComma::No,
443+
RecoverColon::No,
444+
CommaRecoveryMode::EitherTupleOrPipe,
445+
)
446+
}) {
447+
Some(pat)
448+
} else {
449+
None
450+
}
451+
}
452+
432453
/// Parses a pattern, with a setting whether modern range patterns (e.g., `a..=b`, `a..b` are
433454
/// allowed).
434455
fn parse_pat_with_range_pat(
@@ -438,7 +459,10 @@ impl<'a> Parser<'a> {
438459
syntax_loc: Option<PatternLocation>,
439460
) -> PResult<'a, P<Pat>> {
440461
maybe_recover_from_interpolated_ty_qpath!(self, true);
441-
maybe_whole!(self, NtPat, |pat| pat);
462+
463+
if let Some(pat) = self.eat_metavar_pat() {
464+
return Ok(pat);
465+
}
442466

443467
let mut lo = self.token.span;
444468

@@ -770,10 +794,8 @@ impl<'a> Parser<'a> {
770794
self.recover_additional_muts();
771795

772796
// Make sure we don't allow e.g. `let mut $p;` where `$p:pat`.
773-
if let token::Interpolated(nt) = &self.token.kind {
774-
if let token::NtPat(..) = &**nt {
775-
self.expected_ident_found_err().emit();
776-
}
797+
if let Some(MetaVarKind::Pat(_)) = self.token.is_metavar_seq() {
798+
self.expected_ident_found_err().emit();
777799
}
778800

779801
// Parse the pattern we hope to be an identifier.

‎tests/ui/macros/trace_faulty_macros.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ macro_rules! test {
4646
(let $p:pat = $e:expr) => {test!(($p,$e))};
4747
// this should be expr
4848
// vvv
49-
(($p:pat, $e:pat)) => {let $p = $e;}; //~ ERROR expected expression, found pattern `1+1`
49+
(($p:pat, $e:pat)) => {let $p = $e;}; //~ ERROR expected expression, found `pat` metavariable
5050
}
5151

5252
fn foo() {

‎tests/ui/macros/trace_faulty_macros.stderr

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ LL | my_recursive_macro!();
5050
= note: expanding `my_recursive_macro! { }`
5151
= note: to `my_recursive_macro! ();`
5252

53-
error: expected expression, found pattern `A { a : a, b : 0, c : _, .. }`
53+
error: expected expression, found `pat` metavariable
5454
--> $DIR/trace_faulty_macros.rs:16:9
5555
|
5656
LL | $a
@@ -69,22 +69,15 @@ LL | #[derive(Debug)]
6969
LL | fn use_derive_macro_as_attr() {}
7070
| -------------------------------- not a `struct`, `enum` or `union`
7171

72-
error: expected expression, found pattern `1+1`
72+
error: expected expression, found `pat` metavariable
7373
--> $DIR/trace_faulty_macros.rs:49:37
7474
|
75-
LL | (let $p:pat = $e:expr) => {test!(($p,$e))};
76-
| -- this is interpreted as expression, but it is expected to be pattern
77-
...
7875
LL | (($p:pat, $e:pat)) => {let $p = $e;};
7976
| ^^ expected expression
8077
...
8178
LL | test!(let x = 1+1);
82-
| ------------------
83-
| | |
84-
| | this is expected to be expression
85-
| in this macro invocation
79+
| ------------------ in this macro invocation
8680
|
87-
= note: when forwarding a matched fragment to another macro-by-example, matchers in the second macro will see an opaque AST of the fragment type, not the underlying tokens
8881
= note: this error originates in the macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)
8982

9083
note: trace_macro

‎tests/ui/parser/issues/issue-65122-mac-invoc-in-mut-patterns.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ macro_rules! mac2 {
1212
($eval:pat) => {
1313
let mut $eval = ();
1414
//~^ ERROR `mut` must be followed by a named binding
15-
//~| ERROR expected identifier, found `does_not_exist!()`
15+
//~| ERROR expected identifier, found metavariable
1616
};
1717
}
1818

‎tests/ui/parser/issues/issue-65122-mac-invoc-in-mut-patterns.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ LL - let mut $eval = ();
1515
LL + let $eval = ();
1616
|
1717

18-
error: expected identifier, found `does_not_exist!()`
18+
error: expected identifier, found metavariable
1919
--> $DIR/issue-65122-mac-invoc-in-mut-patterns.rs:13:17
2020
|
2121
LL | let mut $eval = ();
22-
| ^^^^^ expected identifier
22+
| ^^^^^ expected identifier, found metavariable
2323
...
2424
LL | mac2! { does_not_exist!() }
2525
| --------------------------- in this macro invocation

‎tests/ui/parser/mut-patterns.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ pub fn main() {
4141
// Make sure we don't accidentally allow `mut $p` where `$p:pat`.
4242
macro_rules! foo {
4343
($p:pat) => {
44-
let mut $p = 0; //~ ERROR expected identifier, found `x`
44+
let mut $p = 0; //~ ERROR expected identifier, found metavariable
4545
}
4646
}
4747
foo!(x);

‎tests/ui/parser/mut-patterns.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,11 @@ help: add `mut` to each binding
142142
LL | let W(mut a, W(mut b, W(ref c, W(mut d, B { box mut f }))))
143143
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
144144

145-
error: expected identifier, found `x`
145+
error: expected identifier, found metavariable
146146
--> $DIR/mut-patterns.rs:44:21
147147
|
148148
LL | let mut $p = 0;
149-
| ^^ expected identifier
149+
| ^^ expected identifier, found metavariable
150150
...
151151
LL | foo!(x);
152152
| ------- in this macro invocation

0 commit comments

Comments
 (0)