Skip to content

Commit f5f2a57

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 8b93c18 commit f5f2a57

16 files changed

+67
-97
lines changed

‎compiler/rustc_ast/src/ast_traits.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,6 @@ impl HasTokens for Nonterminal {
234234
Nonterminal::NtItem(item) => item.tokens(),
235235
Nonterminal::NtStmt(stmt) => stmt.tokens(),
236236
Nonterminal::NtExpr(expr) | Nonterminal::NtLiteral(expr) => expr.tokens(),
237-
Nonterminal::NtPat(pat) => pat.tokens(),
238237
Nonterminal::NtMeta(attr_item) => attr_item.tokens(),
239238
Nonterminal::NtPath(path) => path.tokens(),
240239
Nonterminal::NtBlock(block) => block.tokens(),
@@ -245,7 +244,6 @@ impl HasTokens for Nonterminal {
245244
Nonterminal::NtItem(item) => item.tokens_mut(),
246245
Nonterminal::NtStmt(stmt) => stmt.tokens_mut(),
247246
Nonterminal::NtExpr(expr) | Nonterminal::NtLiteral(expr) => expr.tokens_mut(),
248-
Nonterminal::NtPat(pat) => pat.tokens_mut(),
249247
Nonterminal::NtMeta(attr_item) => attr_item.tokens_mut(),
250248
Nonterminal::NtPath(path) => path.tokens_mut(),
251249
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
@@ -832,7 +832,6 @@ fn visit_nonterminal<T: MutVisitor>(nt: &mut token::Nonterminal, vis: &mut T) {
832832
vis.flat_map_stmt(stmt).expect_one("expected visitor to produce exactly one item")
833833
})
834834
}),
835-
token::NtPat(pat) => vis.visit_pat(pat),
836835
token::NtExpr(expr) => vis.visit_expr(expr),
837836
token::NtLiteral(expr) => vis.visit_expr(expr),
838837
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(
@@ -1043,7 +1042,6 @@ pub enum Nonterminal {
10431042
NtItem(P<ast::Item>),
10441043
NtBlock(P<ast::Block>),
10451044
NtStmt(P<ast::Stmt>),
1046-
NtPat(P<ast::Pat>),
10471045
NtExpr(P<ast::Expr>),
10481046
NtLiteral(P<ast::Expr>),
10491047
/// Stuff inside brackets for attributes
@@ -1140,7 +1138,6 @@ impl Nonterminal {
11401138
NtItem(item) => item.span,
11411139
NtBlock(block) => block.span,
11421140
NtStmt(stmt) => stmt.span,
1143-
NtPat(pat) => pat.span,
11441141
NtExpr(expr) | NtLiteral(expr) => expr.span,
11451142
NtMeta(attr_item) => attr_item.span(),
11461143
NtPath(path) => path.span,
@@ -1152,7 +1149,6 @@ impl Nonterminal {
11521149
NtItem(..) => "item",
11531150
NtBlock(..) => "block",
11541151
NtStmt(..) => "statement",
1155-
NtPat(..) => "pattern",
11561152
NtExpr(..) => "expression",
11571153
NtLiteral(..) => "literal",
11581154
NtMeta(..) => "attribute",
@@ -1177,7 +1173,6 @@ impl fmt::Debug for Nonterminal {
11771173
NtItem(..) => f.pad("NtItem(..)"),
11781174
NtBlock(..) => f.pad("NtBlock(..)"),
11791175
NtStmt(..) => f.pad("NtStmt(..)"),
1180-
NtPat(..) => f.pad("NtPat(..)"),
11811176
NtExpr(..) => f.pad("NtExpr(..)"),
11821177
NtLiteral(..) => f.pad("NtLiteral(..)"),
11831178
NtMeta(..) => f.pad("NtMeta(..)"),

‎compiler/rustc_ast/src/tokenstream.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,6 @@ impl TokenStream {
473473
TokenStream::token_alone(token::Semi, stmt.span)
474474
}
475475
Nonterminal::NtStmt(stmt) => TokenStream::from_ast(stmt),
476-
Nonterminal::NtPat(pat) => TokenStream::from_ast(pat),
477476
Nonterminal::NtMeta(attr) => TokenStream::from_ast(attr),
478477
Nonterminal::NtPath(path) => TokenStream::from_ast(path),
479478
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
@@ -296,6 +296,9 @@ pub(super) fn transcribe<'a>(
296296
let kind = token::NtLifetime(*ident);
297297
TokenTree::token_alone(kind, sp)
298298
}
299+
MatchedSingle(ParseNtResult::Pat(pat, pat_kind)) => {
300+
mk_delimited(MetaVarKind::Pat(*pat_kind), TokenStream::from_ast(pat))
301+
}
299302
MatchedSingle(ParseNtResult::Ty(ty)) => {
300303
mk_delimited(MetaVarKind::Ty, TokenStream::from_ast(ty))
301304
}

‎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;
@@ -2360,52 +2359,6 @@ impl<'a> Parser<'a> {
23602359
err.subdiagnostic(ExprParenthesesNeeded::surrounding(*sp));
23612360
}
23622361
err.span_label(span, "expected expression");
2363-
2364-
// Walk the chain of macro expansions for the current token to point at how the original
2365-
// code was interpreted. This helps the user realize when a macro argument of one type is
2366-
// later reinterpreted as a different type, like `$x:expr` being reinterpreted as `$x:pat`
2367-
// in a subsequent macro invocation (#71039).
2368-
let mut tok = self.token.clone();
2369-
let mut labels = vec![];
2370-
while let TokenKind::Interpolated(nt) = &tok.kind {
2371-
let tokens = nt.tokens();
2372-
labels.push(nt.clone());
2373-
if let Some(tokens) = tokens
2374-
&& let tokens = tokens.to_attr_token_stream()
2375-
&& let tokens = tokens.0.deref()
2376-
&& let [AttrTokenTree::Token(token, _)] = &tokens[..]
2377-
{
2378-
tok = token.clone();
2379-
} else {
2380-
break;
2381-
}
2382-
}
2383-
let mut iter = labels.into_iter().peekable();
2384-
let mut show_link = false;
2385-
while let Some(nt) = iter.next() {
2386-
let descr = nt.descr();
2387-
if let Some(next) = iter.peek() {
2388-
let next_descr = next.descr();
2389-
if next_descr != descr {
2390-
err.span_label(next.use_span(), format!("this is expected to be {next_descr}"));
2391-
err.span_label(
2392-
nt.use_span(),
2393-
format!(
2394-
"this is interpreted as {}, but it is expected to be {}",
2395-
next_descr, descr,
2396-
),
2397-
);
2398-
show_link = true;
2399-
}
2400-
}
2401-
}
2402-
if show_link {
2403-
err.note(
2404-
"when forwarding a matched fragment to another macro-by-example, matchers in the \
2405-
second macro will see an opaque AST of the fragment type, not the underlying \
2406-
tokens",
2407-
);
2408-
}
24092362
err
24102363
}
24112364

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

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

28962896
fn is_named_param(&self) -> bool {
28972897
let offset = match &self.token.kind {
2898-
token::Interpolated(nt) => match &**nt {
2899-
token::NtPat(..) => return self.look_ahead(1, |t| t == &token::Colon),
2898+
token::OpenDelim(Delimiter::Invisible(origin)) => match origin {
2899+
InvisibleOrigin::MetaVar(MetaVarKind::Pat(_)) => {
2900+
return self.check_noexpect_past_close_delim(&token::Colon);
2901+
}
29002902
_ => 0,
29012903
},
29022904
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::{AttributesData, DelimSpacing, DelimSpan, Spacing};
2627
use rustc_ast::tokenstream::{TokenStream, TokenTree, TokenTreeCursor};
@@ -1701,6 +1702,7 @@ pub enum ParseNtResult {
17011702
Tt(TokenTree),
17021703
Ident(Ident, IdentIsRaw),
17031704
Lifetime(Ident),
1705+
Pat(P<ast::Pat>, NtPatKind),
17041706
Ty(P<ast::Ty>),
17051707
Vis(P<ast::Visibility>),
17061708

‎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: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ use crate::errors::{
1010
UnexpectedParenInRangePatSugg, UnexpectedVertVertBeforeFunctionParam,
1111
UnexpectedVertVertInPattern,
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,
@@ -425,6 +425,29 @@ impl<'a> Parser<'a> {
425425
None
426426
}
427427

428+
fn eat_metavar_pat(&mut self) -> Option<P<Pat>> {
429+
// Must try both kinds of pattern nonterminals.
430+
if let Some(pat) = self.eat_metavar_seq_with_matcher(
431+
|mv_kind| matches!(mv_kind, MetaVarKind::Pat(PatParam { .. })),
432+
|this| this.collect_tokens_no_attrs(|this| this.parse_pat_no_top_alt(None, None)),
433+
) {
434+
Some(pat)
435+
} else if let Some(pat) = self.eat_metavar_seq(MetaVarKind::Pat(PatWithOr), |this| {
436+
this.collect_tokens_no_attrs(|this| {
437+
this.parse_pat_allow_top_alt(
438+
None,
439+
RecoverComma::No,
440+
RecoverColon::No,
441+
CommaRecoveryMode::EitherTupleOrPipe,
442+
)
443+
})
444+
}) {
445+
Some(pat)
446+
} else {
447+
None
448+
}
449+
}
450+
428451
/// Parses a pattern, with a setting whether modern range patterns (e.g., `a..=b`, `a..b` are
429452
/// allowed).
430453
fn parse_pat_with_range_pat(
@@ -434,7 +457,10 @@ impl<'a> Parser<'a> {
434457
syntax_loc: Option<PatternLocation>,
435458
) -> PResult<'a, P<Pat>> {
436459
maybe_recover_from_interpolated_ty_qpath!(self, true);
437-
maybe_whole!(self, NtPat, |pat| pat);
460+
461+
if let Some(pat) = self.eat_metavar_pat() {
462+
return Ok(pat);
463+
}
438464

439465
let mut lo = self.token.span;
440466

@@ -755,10 +781,8 @@ impl<'a> Parser<'a> {
755781
self.recover_additional_muts();
756782

757783
// Make sure we don't allow e.g. `let mut $p;` where `$p:pat`.
758-
if let token::Interpolated(nt) = &self.token.kind {
759-
if let token::NtPat(..) = &**nt {
760-
self.expected_ident_found_err().emit();
761-
}
784+
if let Some(MetaVarKind::Pat(_)) = self.token.is_metavar_seq() {
785+
self.expected_ident_found_err().emit();
762786
}
763787

764788
// 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
@@ -10,11 +10,11 @@ LL | mac1! { does_not_exist!() }
1010
= note: `mut` may be followed by `variable` and `variable @ pattern`
1111
= note: this error originates in the macro `mac1` (in Nightly builds, run with -Z macro-backtrace for more info)
1212

13-
error: expected identifier, found `does_not_exist!()`
13+
error: expected identifier, found metavariable
1414
--> $DIR/issue-65122-mac-invoc-in-mut-patterns.rs:13:17
1515
|
1616
LL | let mut $eval = ();
17-
| ^^^^^ expected identifier
17+
| ^^^^^ expected identifier, found metavariable
1818
...
1919
LL | mac2! { does_not_exist!() }
2020
| --------------------------- 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
@@ -99,11 +99,11 @@ LL | let mut W(mut a, W(b, W(ref c, W(d, B { box f }))))
9999
|
100100
= note: `mut` may be followed by `variable` and `variable @ pattern`
101101

102-
error: expected identifier, found `x`
102+
error: expected identifier, found metavariable
103103
--> $DIR/mut-patterns.rs:44:21
104104
|
105105
LL | let mut $p = 0;
106-
| ^^ expected identifier
106+
| ^^ expected identifier, found metavariable
107107
...
108108
LL | foo!(x);
109109
| ------- in this macro invocation

0 commit comments

Comments
 (0)