Skip to content

Commit 372af98

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 #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 b522e7c commit 372af98

16 files changed

+77
-102
lines changed

‎compiler/rustc_ast/src/ast_traits.rs

-2
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

-1
Original file line numberDiff line numberDiff line change
@@ -905,7 +905,6 @@ fn visit_nonterminal<T: MutVisitor>(vis: &mut T, nt: &mut token::Nonterminal) {
905905
vis.flat_map_stmt(stmt).expect_one("expected visitor to produce exactly one item")
906906
})
907907
}),
908-
token::NtPat(pat) => vis.visit_pat(pat),
909908
token::NtExpr(expr) => vis.visit_expr(expr),
910909
token::NtLiteral(expr) => vis.visit_expr(expr),
911910
token::NtMeta(item) => {

‎compiler/rustc_ast/src/token.rs

-5
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,6 @@ impl Token {
659659
| NtExpr(..)
660660
| NtLiteral(..)
661661
| NtMeta(..)
662-
| NtPat(..)
663662
| NtPath(..)
664663
),
665664
OpenDelim(Delimiter::Invisible(InvisibleOrigin::MetaVar(
@@ -1075,7 +1074,6 @@ pub enum Nonterminal {
10751074
NtItem(P<ast::Item>),
10761075
NtBlock(P<ast::Block>),
10771076
NtStmt(P<ast::Stmt>),
1078-
NtPat(P<ast::Pat>),
10791077
NtExpr(P<ast::Expr>),
10801078
NtLiteral(P<ast::Expr>),
10811079
/// Stuff inside brackets for attributes
@@ -1172,7 +1170,6 @@ impl Nonterminal {
11721170
NtItem(item) => item.span,
11731171
NtBlock(block) => block.span,
11741172
NtStmt(stmt) => stmt.span,
1175-
NtPat(pat) => pat.span,
11761173
NtExpr(expr) | NtLiteral(expr) => expr.span,
11771174
NtMeta(attr_item) => attr_item.span(),
11781175
NtPath(path) => path.span,
@@ -1184,7 +1181,6 @@ impl Nonterminal {
11841181
NtItem(..) => "item",
11851182
NtBlock(..) => "block",
11861183
NtStmt(..) => "statement",
1187-
NtPat(..) => "pattern",
11881184
NtExpr(..) => "expression",
11891185
NtLiteral(..) => "literal",
11901186
NtMeta(..) => "attribute",
@@ -1209,7 +1205,6 @@ impl fmt::Debug for Nonterminal {
12091205
NtItem(..) => f.pad("NtItem(..)"),
12101206
NtBlock(..) => f.pad("NtBlock(..)"),
12111207
NtStmt(..) => f.pad("NtStmt(..)"),
1212-
NtPat(..) => f.pad("NtPat(..)"),
12131208
NtExpr(..) => f.pad("NtExpr(..)"),
12141209
NtLiteral(..) => f.pad("NtLiteral(..)"),
12151210
NtMeta(..) => f.pad("NtMeta(..)"),

‎compiler/rustc_ast/src/tokenstream.rs

-1
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

+14-4
Original file line numberDiff line numberDiff line change
@@ -279,9 +279,9 @@ pub(super) fn transcribe<'a>(
279279
if let Some(cur_matched) = lookup_cur_matched(ident, interp, &repeats) {
280280
// We wrap the tokens in invisible delimiters, unless they are already wrapped
281281
// in invisible delimiters with the same `MetaVarKind`. Because some proc
282-
// macros can't multiple layers of invisible delimiters of the same
282+
// macros can't handle multiple layers of invisible delimiters of the same
283283
// `MetaVarKind`. This loses some span info, though it hopefully won't matter.
284-
let mut mk_delimited = |mv_kind, mut stream: TokenStream| {
284+
let mut mk_delimited = |mk_span, mv_kind, mut stream: TokenStream| {
285285
if stream.len() == 1 {
286286
let tree = stream.iter().next().unwrap();
287287
if let TokenTree::Delimited(_, _, delim, inner) = tree
@@ -295,6 +295,7 @@ pub(super) fn transcribe<'a>(
295295
// Emit as a token stream within `Delimiter::Invisible` to maintain
296296
// parsing priorities.
297297
marker.visit_span(&mut sp);
298+
with_metavar_spans(|mspans| mspans.insert(mk_span, sp));
298299
// Both the open delim and close delim get the same span, which covers the
299300
// `$foo` in the decl macro RHS.
300301
TokenTree::Delimited(
@@ -322,12 +323,21 @@ pub(super) fn transcribe<'a>(
322323
let kind = token::NtLifetime(*ident, *is_raw);
323324
TokenTree::token_alone(kind, sp)
324325
}
326+
MatchedSingle(ParseNtResult::Pat(pat, pat_kind)) => mk_delimited(
327+
pat.span,
328+
MetaVarKind::Pat(*pat_kind),
329+
TokenStream::from_ast(pat),
330+
),
325331
MatchedSingle(ParseNtResult::Ty(ty)) => {
326332
let is_path = matches!(&ty.kind, TyKind::Path(None, _path));
327-
mk_delimited(MetaVarKind::Ty { is_path }, TokenStream::from_ast(ty))
333+
mk_delimited(
334+
ty.span,
335+
MetaVarKind::Ty { is_path },
336+
TokenStream::from_ast(ty),
337+
)
328338
}
329339
MatchedSingle(ParseNtResult::Vis(vis)) => {
330-
mk_delimited(MetaVarKind::Vis, TokenStream::from_ast(vis))
340+
mk_delimited(vis.span, MetaVarKind::Vis, TokenStream::from_ast(vis))
331341
}
332342
MatchedSingle(ParseNtResult::Nt(nt)) => {
333343
// Other variables are emitted into the output stream as groups with

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

+2-50
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,15 @@
11
use std::mem::take;
22
use std::ops::{Deref, DerefMut};
3-
use std::sync::Arc;
43

54
use ast::token::IdentIsRaw;
65
use rustc_ast as ast;
76
use rustc_ast::ptr::P;
87
use rustc_ast::token::{self, Delimiter, Lit, LitKind, Token, TokenKind};
9-
use rustc_ast::tokenstream::AttrTokenTree;
108
use rustc_ast::util::parser::AssocOp;
119
use rustc_ast::{
1210
AngleBracketedArg, AngleBracketedArgs, AnonConst, AttrVec, BinOpKind, BindingMode, Block,
13-
BlockCheckMode, Expr, ExprKind, GenericArg, Generics, HasTokens, Item, ItemKind, Param, Pat,
14-
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,
1513
};
1614
use rustc_ast_pretty::pprust;
1715
use rustc_data_structures::fx::FxHashSet;
@@ -2400,52 +2398,6 @@ impl<'a> Parser<'a> {
24002398
err.subdiagnostic(ExprParenthesesNeeded::surrounding(*sp));
24012399
}
24022400
err.span_label(span, "expected expression");
2403-
2404-
// Walk the chain of macro expansions for the current token to point at how the original
2405-
// code was interpreted. This helps the user realize when a macro argument of one type is
2406-
// later reinterpreted as a different type, like `$x:expr` being reinterpreted as `$x:pat`
2407-
// in a subsequent macro invocation (#71039).
2408-
let mut tok = self.token.clone();
2409-
let mut labels = vec![];
2410-
while let TokenKind::Interpolated(nt) = &tok.kind {
2411-
let tokens = nt.tokens();
2412-
labels.push(Arc::clone(nt));
2413-
if let Some(tokens) = tokens
2414-
&& let tokens = tokens.to_attr_token_stream()
2415-
&& let tokens = tokens.0.deref()
2416-
&& let [AttrTokenTree::Token(token, _)] = &tokens[..]
2417-
{
2418-
tok = token.clone();
2419-
} else {
2420-
break;
2421-
}
2422-
}
2423-
let mut iter = labels.into_iter().peekable();
2424-
let mut show_link = false;
2425-
while let Some(nt) = iter.next() {
2426-
let descr = nt.descr();
2427-
if let Some(next) = iter.peek() {
2428-
let next_descr = next.descr();
2429-
if next_descr != descr {
2430-
err.span_label(next.use_span(), format!("this is expected to be {next_descr}"));
2431-
err.span_label(
2432-
nt.use_span(),
2433-
format!(
2434-
"this is interpreted as {}, but it is expected to be {}",
2435-
next_descr, descr,
2436-
),
2437-
);
2438-
show_link = true;
2439-
}
2440-
}
2441-
}
2442-
if show_link {
2443-
err.note(
2444-
"when forwarding a matched fragment to another macro-by-example, matchers in the \
2445-
second macro will see an opaque AST of the fragment type, not the underlying \
2446-
tokens",
2447-
);
2448-
}
24492401
err
24502402
}
24512403

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

+5-3
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};
@@ -3071,8 +3071,10 @@ impl<'a> Parser<'a> {
30713071

30723072
fn is_named_param(&self) -> bool {
30733073
let offset = match &self.token.kind {
3074-
token::Interpolated(nt) => match &**nt {
3075-
token::NtPat(..) => return self.look_ahead(1, |t| t == &token::Colon),
3074+
token::OpenDelim(Delimiter::Invisible(origin)) => match origin {
3075+
InvisibleOrigin::MetaVar(MetaVarKind::Pat(_)) => {
3076+
return self.check_noexpect_past_close_delim(&token::Colon);
3077+
}
30763078
_ => 0,
30773079
},
30783080
token::BinOp(token::And) | token::AndAnd => 1,

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ pub use pat::{CommaRecoveryMode, RecoverColon, RecoverComma};
2424
use path::PathStyle;
2525
use rustc_ast::ptr::P;
2626
use rustc_ast::token::{
27-
self, Delimiter, IdentIsRaw, InvisibleOrigin, MetaVarKind, Nonterminal, Token, TokenKind,
27+
self, Delimiter, IdentIsRaw, InvisibleOrigin, MetaVarKind, Nonterminal, NtPatKind, Token,
28+
TokenKind,
2829
};
2930
use rustc_ast::tokenstream::{AttrsTarget, Spacing, TokenStream, TokenTree};
3031
use rustc_ast::util::case::Case;
@@ -1745,6 +1746,7 @@ pub enum ParseNtResult {
17451746
Tt(TokenTree),
17461747
Ident(Ident, IdentIsRaw),
17471748
Lifetime(Ident, IdentIsRaw),
1749+
Pat(P<ast::Pat>, NtPatKind),
17481750
Ty(P<ast::Ty>),
17491751
Vis(P<ast::Visibility>),
17501752

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

+13-11
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_no_top_guard(
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_no_top_guard(
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

+30-7
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ use std::ops::Bound;
22

33
use rustc_ast::mut_visit::{self, MutVisitor};
44
use rustc_ast::ptr::P;
5-
use rustc_ast::token::{self, BinOpToken, Delimiter, IdentIsRaw, Token};
5+
use rustc_ast::token::NtPatKind::*;
6+
use rustc_ast::token::{self, BinOpToken, Delimiter, IdentIsRaw, MetaVarKind, Token};
67
use rustc_ast::util::parser::ExprPrecedence;
78
use rustc_ast::visit::{self, Visitor};
89
use rustc_ast::{
@@ -30,7 +31,7 @@ use crate::errors::{
3031
UnexpectedVertVertInPattern, WrapInParens,
3132
};
3233
use crate::parser::expr::{DestructuredFloat, could_be_unclosed_char_literal};
33-
use crate::{exp, maybe_recover_from_interpolated_ty_qpath, maybe_whole};
34+
use crate::{exp, maybe_recover_from_interpolated_ty_qpath};
3435

3536
#[derive(PartialEq, Copy, Clone)]
3637
pub enum Expected {
@@ -689,6 +690,27 @@ impl<'a> Parser<'a> {
689690
PatVisitor { parser: self, stmt, arm: None, field: None }.visit_stmt(stmt);
690691
}
691692

693+
fn eat_metavar_pat(&mut self) -> Option<P<Pat>> {
694+
// Must try both kinds of pattern nonterminals.
695+
if let Some(pat) = self.eat_metavar_seq_with_matcher(
696+
|mv_kind| matches!(mv_kind, MetaVarKind::Pat(PatParam { .. })),
697+
|this| this.parse_pat_no_top_alt(None, None),
698+
) {
699+
Some(pat)
700+
} else if let Some(pat) = self.eat_metavar_seq(MetaVarKind::Pat(PatWithOr), |this| {
701+
this.parse_pat_no_top_guard(
702+
None,
703+
RecoverComma::No,
704+
RecoverColon::No,
705+
CommaRecoveryMode::EitherTupleOrPipe,
706+
)
707+
}) {
708+
Some(pat)
709+
} else {
710+
None
711+
}
712+
}
713+
692714
/// Parses a pattern, with a setting whether modern range patterns (e.g., `a..=b`, `a..b` are
693715
/// allowed).
694716
fn parse_pat_with_range_pat(
@@ -698,7 +720,10 @@ impl<'a> Parser<'a> {
698720
syntax_loc: Option<PatternLocation>,
699721
) -> PResult<'a, P<Pat>> {
700722
maybe_recover_from_interpolated_ty_qpath!(self, true);
701-
maybe_whole!(self, NtPat, |pat| pat);
723+
724+
if let Some(pat) = self.eat_metavar_pat() {
725+
return Ok(pat);
726+
}
702727

703728
let mut lo = self.token.span;
704729

@@ -1043,10 +1068,8 @@ impl<'a> Parser<'a> {
10431068
self.recover_additional_muts();
10441069

10451070
// Make sure we don't allow e.g. `let mut $p;` where `$p:pat`.
1046-
if let token::Interpolated(nt) = &self.token.kind {
1047-
if let token::NtPat(..) = &**nt {
1048-
self.expected_ident_found_err().emit();
1049-
}
1071+
if let Some(MetaVarKind::Pat(_)) = self.token.is_metavar_seq() {
1072+
self.expected_ident_found_err().emit();
10501073
}
10511074

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

‎tests/ui/macros/trace_faulty_macros.rs

+1-1
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

+3-10
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

+1-1
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

0 commit comments

Comments
 (0)