Skip to content

Commit 4eb0e75

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 is 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 2a78ecb commit 4eb0e75

16 files changed

+66
-98
lines changed

‎compiler/rustc_ast/src/ast_traits.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,6 @@ impl HasTokens for Nonterminal {
202202
Nonterminal::NtItem(item) => item.tokens(),
203203
Nonterminal::NtStmt(stmt) => stmt.tokens(),
204204
Nonterminal::NtExpr(expr) | Nonterminal::NtLiteral(expr) => expr.tokens(),
205-
Nonterminal::NtPat(pat) => pat.tokens(),
206205
Nonterminal::NtMeta(attr_item) => attr_item.tokens(),
207206
Nonterminal::NtPath(path) => path.tokens(),
208207
Nonterminal::NtBlock(block) => block.tokens(),
@@ -213,7 +212,6 @@ impl HasTokens for Nonterminal {
213212
Nonterminal::NtItem(item) => item.tokens_mut(),
214213
Nonterminal::NtStmt(stmt) => stmt.tokens_mut(),
215214
Nonterminal::NtExpr(expr) | Nonterminal::NtLiteral(expr) => expr.tokens_mut(),
216-
Nonterminal::NtPat(pat) => pat.tokens_mut(),
217215
Nonterminal::NtMeta(attr_item) => attr_item.tokens_mut(),
218216
Nonterminal::NtPath(path) => path.tokens_mut(),
219217
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
@@ -809,7 +809,6 @@ fn visit_nonterminal<T: MutVisitor>(vis: &mut T, nt: &mut token::Nonterminal) {
809809
vis.flat_map_stmt(stmt).expect_one("expected visitor to produce exactly one item")
810810
})
811811
}),
812-
token::NtPat(pat) => vis.visit_pat(pat),
813812
token::NtExpr(expr) => vis.visit_expr(expr),
814813
token::NtLiteral(expr) => vis.visit_expr(expr),
815814
token::NtMeta(item) => {

‎compiler/rustc_ast/src/token.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,6 @@ impl Token {
658658
| NtExpr(..)
659659
| NtLiteral(..)
660660
| NtMeta(..)
661-
| NtPat(..)
662661
| NtPath(..)
663662
),
664663
OpenDelim(Delimiter::Invisible(InvisibleOrigin::MetaVar(
@@ -1067,7 +1066,6 @@ pub enum Nonterminal {
10671066
NtItem(P<ast::Item>),
10681067
NtBlock(P<ast::Block>),
10691068
NtStmt(P<ast::Stmt>),
1070-
NtPat(P<ast::Pat>),
10711069
NtExpr(P<ast::Expr>),
10721070
NtLiteral(P<ast::Expr>),
10731071
/// Stuff inside brackets for attributes
@@ -1164,7 +1162,6 @@ impl Nonterminal {
11641162
NtItem(item) => item.span,
11651163
NtBlock(block) => block.span,
11661164
NtStmt(stmt) => stmt.span,
1167-
NtPat(pat) => pat.span,
11681165
NtExpr(expr) | NtLiteral(expr) => expr.span,
11691166
NtMeta(attr_item) => attr_item.span(),
11701167
NtPath(path) => path.span,
@@ -1176,7 +1173,6 @@ impl Nonterminal {
11761173
NtItem(..) => "item",
11771174
NtBlock(..) => "block",
11781175
NtStmt(..) => "statement",
1179-
NtPat(..) => "pattern",
11801176
NtExpr(..) => "expression",
11811177
NtLiteral(..) => "literal",
11821178
NtMeta(..) => "attribute",
@@ -1201,7 +1197,6 @@ impl fmt::Debug for Nonterminal {
12011197
NtItem(..) => f.pad("NtItem(..)"),
12021198
NtBlock(..) => f.pad("NtBlock(..)"),
12031199
NtStmt(..) => f.pad("NtStmt(..)"),
1204-
NtPat(..) => f.pad("NtPat(..)"),
12051200
NtExpr(..) => f.pad("NtExpr(..)"),
12061201
NtLiteral(..) => f.pad("NtLiteral(..)"),
12071202
NtMeta(..) => f.pad("NtMeta(..)"),

‎compiler/rustc_ast/src/tokenstream.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,6 @@ impl TokenStream {
467467
TokenStream::token_alone(token::Semi, stmt.span)
468468
}
469469
Nonterminal::NtStmt(stmt) => TokenStream::from_ast(stmt),
470-
Nonterminal::NtPat(pat) => TokenStream::from_ast(pat),
471470
Nonterminal::NtMeta(attr) => TokenStream::from_ast(attr),
472471
Nonterminal::NtPath(path) => TokenStream::from_ast(path),
473472
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
@@ -318,6 +318,9 @@ pub(super) fn transcribe<'a>(
318318
let kind = token::NtLifetime(*ident, *is_raw);
319319
TokenTree::token_alone(kind, sp)
320320
}
321+
MatchedSingle(ParseNtResult::Pat(pat, pat_kind)) => {
322+
mk_delimited(MetaVarKind::Pat(*pat_kind), TokenStream::from_ast(pat))
323+
}
321324
MatchedSingle(ParseNtResult::Ty(ty)) => {
322325
mk_delimited(MetaVarKind::Ty, TokenStream::from_ast(ty))
323326
}

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

Lines changed: 2 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,14 @@ use ast::token::IdentIsRaw;
55
use rustc_ast as ast;
66
use rustc_ast::ptr::P;
77
use rustc_ast::token::{self, Delimiter, Lit, LitKind, Token, TokenKind};
8-
use rustc_ast::tokenstream::AttrTokenTree;
98
use rustc_ast::util::parser::AssocOp;
109
use rustc_ast::{
1110
AngleBracketedArg, AngleBracketedArgs, AnonConst, AttrVec, BinOpKind, BindingMode, Block,
12-
BlockCheckMode, Expr, ExprKind, GenericArg, Generics, HasTokens, Item, ItemKind, Param, Pat,
13-
PatKind, Path, PathSegment, QSelf, Recovered, Ty, TyKind,
11+
BlockCheckMode, Expr, ExprKind, GenericArg, Generics, Item, ItemKind, Param, Pat, PatKind,
12+
Path, PathSegment, QSelf, Recovered, Ty, TyKind,
1413
};
1514
use rustc_ast_pretty::pprust;
1615
use rustc_data_structures::fx::FxHashSet;
17-
use rustc_data_structures::sync::Lrc;
1816
use rustc_errors::{
1917
Applicability, Diag, DiagCtxtHandle, ErrorGuaranteed, FatalError, PErr, PResult, Subdiagnostic,
2018
Suggestions, pluralize,
@@ -2429,52 +2427,6 @@ impl<'a> Parser<'a> {
24292427
err.subdiagnostic(ExprParenthesesNeeded::surrounding(*sp));
24302428
}
24312429
err.span_label(span, "expected expression");
2432-
2433-
// Walk the chain of macro expansions for the current token to point at how the original
2434-
// code was interpreted. This helps the user realize when a macro argument of one type is
2435-
// later reinterpreted as a different type, like `$x:expr` being reinterpreted as `$x:pat`
2436-
// in a subsequent macro invocation (#71039).
2437-
let mut tok = self.token.clone();
2438-
let mut labels = vec![];
2439-
while let TokenKind::Interpolated(nt) = &tok.kind {
2440-
let tokens = nt.tokens();
2441-
labels.push(Lrc::clone(nt));
2442-
if let Some(tokens) = tokens
2443-
&& let tokens = tokens.to_attr_token_stream()
2444-
&& let tokens = tokens.0.deref()
2445-
&& let [AttrTokenTree::Token(token, _)] = &tokens[..]
2446-
{
2447-
tok = token.clone();
2448-
} else {
2449-
break;
2450-
}
2451-
}
2452-
let mut iter = labels.into_iter().peekable();
2453-
let mut show_link = false;
2454-
while let Some(nt) = iter.next() {
2455-
let descr = nt.descr();
2456-
if let Some(next) = iter.peek() {
2457-
let next_descr = next.descr();
2458-
if next_descr != descr {
2459-
err.span_label(next.use_span(), format!("this is expected to be {next_descr}"));
2460-
err.span_label(
2461-
nt.use_span(),
2462-
format!(
2463-
"this is interpreted as {}, but it is expected to be {}",
2464-
next_descr, descr,
2465-
),
2466-
);
2467-
show_link = true;
2468-
}
2469-
}
2470-
}
2471-
if show_link {
2472-
err.note(
2473-
"when forwarding a matched fragment to another macro-by-example, matchers in the \
2474-
second macro will see an opaque AST of the fragment type, not the underlying \
2475-
tokens",
2476-
);
2477-
}
24782430
err
24792431
}
24802432

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::mem;
44
use ast::token::IdentIsRaw;
55
use rustc_ast::ast::*;
66
use rustc_ast::ptr::P;
7-
use rustc_ast::token::{self, Delimiter, TokenKind};
7+
use rustc_ast::token::{self, Delimiter, InvisibleOrigin, MetaVarKind, TokenKind};
88
use rustc_ast::tokenstream::{DelimSpan, TokenStream, TokenTree};
99
use rustc_ast::util::case::Case;
1010
use rustc_ast::{self as ast};
@@ -2976,8 +2976,10 @@ impl<'a> Parser<'a> {
29762976

29772977
fn is_named_param(&self) -> bool {
29782978
let offset = match &self.token.kind {
2979-
token::Interpolated(nt) => match &**nt {
2980-
token::NtPat(..) => return self.look_ahead(1, |t| t == &token::Colon),
2979+
token::OpenDelim(Delimiter::Invisible(origin)) => match origin {
2980+
InvisibleOrigin::MetaVar(MetaVarKind::Pat(_)) => {
2981+
return self.check_noexpect_past_close_delim(&token::Colon);
2982+
}
29812983
_ => 0,
29822984
},
29832985
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
@@ -22,7 +22,8 @@ pub use pat::{CommaRecoveryMode, RecoverColon, RecoverComma};
2222
use path::PathStyle;
2323
use rustc_ast::ptr::P;
2424
use rustc_ast::token::{
25-
self, Delimiter, IdentIsRaw, InvisibleOrigin, MetaVarKind, Nonterminal, Token, TokenKind,
25+
self, Delimiter, IdentIsRaw, InvisibleOrigin, MetaVarKind, Nonterminal, NtPatKind, Token,
26+
TokenKind,
2627
};
2728
use rustc_ast::tokenstream::{
2829
AttrsTarget, DelimSpacing, DelimSpan, Spacing, TokenStream, TokenTree, TokenTreeCursor,
@@ -1763,6 +1764,7 @@ pub enum ParseNtResult {
17631764
Tt(TokenTree),
17641765
Ident(Ident, IdentIsRaw),
17651766
Lifetime(Ident, IdentIsRaw),
1767+
Pat(P<ast::Pat>, NtPatKind),
17661768
Ty(P<ast::Ty>),
17671769
Vis(P<ast::Visibility>),
17681770

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

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ impl<'a> Parser<'a> {
4949
fn nt_may_be_ident(nt: &Nonterminal) -> bool {
5050
match nt {
5151
NtStmt(_)
52-
| NtPat(_)
5352
| NtExpr(_)
5453
| NtLiteral(_) // `true`, `false`
5554
| NtMeta(_)
@@ -99,7 +98,7 @@ impl<'a> Parser<'a> {
9998
token::NtLifetime(..) => true,
10099
token::Interpolated(nt) => match &**nt {
101100
NtBlock(_) | NtStmt(_) | NtExpr(_) | NtLiteral(_) => true,
102-
NtItem(_) | NtPat(_) | NtMeta(_) | NtPath(_) => false,
101+
NtItem(_) | NtMeta(_) | NtPath(_) => false,
103102
},
104103
token::OpenDelim(Delimiter::Invisible(InvisibleOrigin::MetaVar(k))) => match k {
105104
MetaVarKind::Block
@@ -170,15 +169,18 @@ impl<'a> Parser<'a> {
170169
}
171170
},
172171
NonterminalKind::Pat(pat_kind) => {
173-
NtPat(self.collect_tokens_no_attrs(|this| match pat_kind {
174-
PatParam { .. } => this.parse_pat_no_top_alt(None, None),
175-
PatWithOr => this.parse_pat_allow_top_alt(
176-
None,
177-
RecoverComma::No,
178-
RecoverColon::No,
179-
CommaRecoveryMode::EitherTupleOrPipe,
180-
),
181-
})?)
172+
return Ok(ParseNtResult::Pat(
173+
self.collect_tokens_no_attrs(|this| match pat_kind {
174+
PatParam { .. } => this.parse_pat_no_top_alt(None, None),
175+
PatWithOr => this.parse_pat_allow_top_alt(
176+
None,
177+
RecoverComma::No,
178+
RecoverColon::No,
179+
CommaRecoveryMode::EitherTupleOrPipe,
180+
),
181+
})?,
182+
pat_kind,
183+
));
182184
}
183185
NonterminalKind::Expr(_) => NtExpr(self.parse_expr_force_collect()?),
184186
NonterminalKind::Literal => {

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

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use rustc_ast::mut_visit::{self, MutVisitor};
22
use rustc_ast::ptr::P;
3-
use rustc_ast::token::{self, BinOpToken, Delimiter, IdentIsRaw, Token};
3+
use rustc_ast::token::NtPatKind::*;
4+
use rustc_ast::token::{self, BinOpToken, Delimiter, IdentIsRaw, MetaVarKind, Token};
45
use rustc_ast::visit::{self, Visitor};
56
use rustc_ast::{
67
self as ast, Arm, AttrVec, BinOpKind, BindingMode, ByRef, Expr, ExprKind, ExprPrecedence,
@@ -28,8 +29,8 @@ use crate::errors::{
2829
UnexpectedParenInRangePatSugg, UnexpectedVertVertBeforeFunctionParam,
2930
UnexpectedVertVertInPattern, WrapInParens,
3031
};
32+
use crate::maybe_recover_from_interpolated_ty_qpath;
3133
use crate::parser::expr::{DestructuredFloat, could_be_unclosed_char_literal};
32-
use crate::{maybe_recover_from_interpolated_ty_qpath, maybe_whole};
3334

3435
#[derive(PartialEq, Copy, Clone)]
3536
pub enum Expected {
@@ -664,6 +665,27 @@ impl<'a> Parser<'a> {
664665
PatVisitor { parser: self, stmt, arm: None, field: None }.visit_stmt(stmt);
665666
}
666667

668+
fn eat_metavar_pat(&mut self) -> Option<P<Pat>> {
669+
// Must try both kinds of pattern nonterminals.
670+
if let Some(pat) = self.eat_metavar_seq_with_matcher(
671+
|mv_kind| matches!(mv_kind, MetaVarKind::Pat(PatParam { .. })),
672+
|this| this.parse_pat_no_top_alt(None, None),
673+
) {
674+
Some(pat)
675+
} else if let Some(pat) = self.eat_metavar_seq(MetaVarKind::Pat(PatWithOr), |this| {
676+
this.parse_pat_allow_top_alt(
677+
None,
678+
RecoverComma::No,
679+
RecoverColon::No,
680+
CommaRecoveryMode::EitherTupleOrPipe,
681+
)
682+
}) {
683+
Some(pat)
684+
} else {
685+
None
686+
}
687+
}
688+
667689
/// Parses a pattern, with a setting whether modern range patterns (e.g., `a..=b`, `a..b` are
668690
/// allowed).
669691
fn parse_pat_with_range_pat(
@@ -673,7 +695,10 @@ impl<'a> Parser<'a> {
673695
syntax_loc: Option<PatternLocation>,
674696
) -> PResult<'a, P<Pat>> {
675697
maybe_recover_from_interpolated_ty_qpath!(self, true);
676-
maybe_whole!(self, NtPat, |pat| pat);
698+
699+
if let Some(pat) = self.eat_metavar_pat() {
700+
return Ok(pat);
701+
}
677702

678703
let mut lo = self.token.span;
679704

@@ -1015,10 +1040,8 @@ impl<'a> Parser<'a> {
10151040
self.recover_additional_muts();
10161041

10171042
// Make sure we don't allow e.g. `let mut $p;` where `$p:pat`.
1018-
if let token::Interpolated(nt) = &self.token.kind {
1019-
if let token::NtPat(..) = &**nt {
1020-
self.expected_ident_found_err().emit();
1021-
}
1043+
if let Some(MetaVarKind::Pat(_)) = self.token.is_metavar_seq() {
1044+
self.expected_ident_found_err().emit();
10221045
}
10231046

10241047
// 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)