From 981e8b46c5bbb78bf39207bbfb8fe6f8d6ba853e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 22 Jan 2024 01:18:16 +0000 Subject: [PATCH 1/3] Check that a token can begin a nonterminal kind before parsing it as a macro arg in rustfmt --- src/tools/rustfmt/src/parse/macros/mod.rs | 41 +++++++++++-------- .../tests/source/macros/rewrite-const-item.rs | 1 + .../tests/target/macros/rewrite-const-item.rs | 3 ++ 3 files changed, 28 insertions(+), 17 deletions(-) create mode 100644 src/tools/rustfmt/tests/source/macros/rewrite-const-item.rs create mode 100644 src/tools/rustfmt/tests/target/macros/rewrite-const-item.rs diff --git a/src/tools/rustfmt/src/parse/macros/mod.rs b/src/tools/rustfmt/src/parse/macros/mod.rs index 2dd2622174f81..36e3972a4633c 100644 --- a/src/tools/rustfmt/src/parse/macros/mod.rs +++ b/src/tools/rustfmt/src/parse/macros/mod.rs @@ -1,7 +1,7 @@ use rustc_ast::token::{Delimiter, TokenKind}; use rustc_ast::tokenstream::TokenStream; use rustc_ast::{ast, ptr}; -use rustc_parse::parser::{ForceCollect, Parser}; +use rustc_parse::parser::{ForceCollect, Parser, Recovery}; use rustc_parse::{stream_to_parser, MACRO_ARGUMENTS}; use rustc_session::parse::ParseSess; use rustc_span::symbol::{self, kw}; @@ -24,45 +24,52 @@ fn build_parser<'a>(context: &RewriteContext<'a>, tokens: TokenStream) -> Parser fn parse_macro_arg<'a, 'b: 'a>(parser: &'a mut Parser<'b>) -> Option { macro_rules! parse_macro_arg { - ($macro_arg:ident, $parser:expr, $f:expr) => { + ($macro_arg:ident, $can_begin:expr, $try_parse:expr, $then:expr) => { let mut cloned_parser = (*parser).clone(); - match $parser(&mut cloned_parser) { - Ok(x) => { - if parser.sess.dcx.has_errors().is_some() { + if $can_begin(&mut cloned_parser) { + match $try_parse(&mut cloned_parser) { + Ok(x) => { + if parser.sess.dcx.has_errors().is_some() { + parser.sess.dcx.reset_err_count(); + } else { + // Parsing succeeded. + *parser = cloned_parser; + return Some(MacroArg::$macro_arg($then(x)?)); + } + } + Err(e) => { + e.cancel(); parser.sess.dcx.reset_err_count(); - } else { - // Parsing succeeded. - *parser = cloned_parser; - return Some(MacroArg::$macro_arg($f(x)?)); } } - Err(e) => { - e.cancel(); - parser.sess.dcx.reset_err_count(); - } } }; } parse_macro_arg!( Expr, - |parser: &mut rustc_parse::parser::Parser<'b>| parser.parse_expr(), + |parser: &mut Parser<'b>| parser.token.can_begin_expr(), + |parser: &mut Parser<'b>| parser.parse_expr(), |x: ptr::P| Some(x) ); parse_macro_arg!( Ty, - |parser: &mut rustc_parse::parser::Parser<'b>| parser.parse_ty(), + |parser: &mut Parser<'b>| parser.token.can_begin_type(), + |parser: &mut Parser<'b>| parser.parse_ty(), |x: ptr::P| Some(x) ); parse_macro_arg!( Pat, - |parser: &mut rustc_parse::parser::Parser<'b>| parser.parse_pat_no_top_alt(None, None), + // FIXME: This isn't right + |_| true, + |parser: &mut Parser<'b>| parser.parse_pat_no_top_alt(None, None), |x: ptr::P| Some(x) ); // `parse_item` returns `Option>`. parse_macro_arg!( Item, - |parser: &mut rustc_parse::parser::Parser<'b>| parser.parse_item(ForceCollect::No), + |_| true, + |parser: &mut Parser<'b>| parser.parse_item(ForceCollect::No), |x: Option>| x ); diff --git a/src/tools/rustfmt/tests/source/macros/rewrite-const-item.rs b/src/tools/rustfmt/tests/source/macros/rewrite-const-item.rs new file mode 100644 index 0000000000000..3db2c26ab5aa7 --- /dev/null +++ b/src/tools/rustfmt/tests/source/macros/rewrite-const-item.rs @@ -0,0 +1 @@ +m!(const N: usize = 0;); diff --git a/src/tools/rustfmt/tests/target/macros/rewrite-const-item.rs b/src/tools/rustfmt/tests/target/macros/rewrite-const-item.rs new file mode 100644 index 0000000000000..f7ebaf7827726 --- /dev/null +++ b/src/tools/rustfmt/tests/target/macros/rewrite-const-item.rs @@ -0,0 +1,3 @@ +m!( + const N: usize = 0; +); From b93ae21441eeb466f300efff41f97f1c8b9e0508 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 22 Jan 2024 00:56:58 +0000 Subject: [PATCH 2/3] Do not eagerly recover malformed AST in rustfmt --- src/tools/rustfmt/src/parse/macros/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/rustfmt/src/parse/macros/mod.rs b/src/tools/rustfmt/src/parse/macros/mod.rs index 36e3972a4633c..c6ee96ebb8aa6 100644 --- a/src/tools/rustfmt/src/parse/macros/mod.rs +++ b/src/tools/rustfmt/src/parse/macros/mod.rs @@ -15,7 +15,7 @@ pub(crate) mod cfg_if; pub(crate) mod lazy_static; fn build_stream_parser<'a>(sess: &'a ParseSess, tokens: TokenStream) -> Parser<'a> { - stream_to_parser(sess, tokens, MACRO_ARGUMENTS) + stream_to_parser(sess, tokens, MACRO_ARGUMENTS).recovery(Recovery::Forbidden) } fn build_parser<'a>(context: &RewriteContext<'a>, tokens: TokenStream) -> Parser<'a> { From 5297433747e5cd2c3794fd9eb88917797e5758ae Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 22 Jan 2024 01:55:49 +0000 Subject: [PATCH 3/3] Actually, just use nonterminal_may_begin_with --- src/tools/rustfmt/src/parse/macros/mod.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/tools/rustfmt/src/parse/macros/mod.rs b/src/tools/rustfmt/src/parse/macros/mod.rs index c6ee96ebb8aa6..a9f9ea1826ae5 100644 --- a/src/tools/rustfmt/src/parse/macros/mod.rs +++ b/src/tools/rustfmt/src/parse/macros/mod.rs @@ -1,4 +1,4 @@ -use rustc_ast::token::{Delimiter, TokenKind}; +use rustc_ast::token::{Delimiter, NonterminalKind, TokenKind}; use rustc_ast::tokenstream::TokenStream; use rustc_ast::{ast, ptr}; use rustc_parse::parser::{ForceCollect, Parser, Recovery}; @@ -24,9 +24,9 @@ fn build_parser<'a>(context: &RewriteContext<'a>, tokens: TokenStream) -> Parser fn parse_macro_arg<'a, 'b: 'a>(parser: &'a mut Parser<'b>) -> Option { macro_rules! parse_macro_arg { - ($macro_arg:ident, $can_begin:expr, $try_parse:expr, $then:expr) => { + ($macro_arg:ident, $nt_kind:expr, $try_parse:expr, $then:expr) => { let mut cloned_parser = (*parser).clone(); - if $can_begin(&mut cloned_parser) { + if Parser::nonterminal_may_begin_with($nt_kind, &cloned_parser.token) { match $try_parse(&mut cloned_parser) { Ok(x) => { if parser.sess.dcx.has_errors().is_some() { @@ -48,27 +48,26 @@ fn parse_macro_arg<'a, 'b: 'a>(parser: &'a mut Parser<'b>) -> Option { parse_macro_arg!( Expr, - |parser: &mut Parser<'b>| parser.token.can_begin_expr(), + NonterminalKind::Expr, |parser: &mut Parser<'b>| parser.parse_expr(), |x: ptr::P| Some(x) ); parse_macro_arg!( Ty, - |parser: &mut Parser<'b>| parser.token.can_begin_type(), + NonterminalKind::Ty, |parser: &mut Parser<'b>| parser.parse_ty(), |x: ptr::P| Some(x) ); parse_macro_arg!( Pat, - // FIXME: This isn't right - |_| true, + NonterminalKind::PatParam { inferred: false }, |parser: &mut Parser<'b>| parser.parse_pat_no_top_alt(None, None), |x: ptr::P| Some(x) ); // `parse_item` returns `Option>`. parse_macro_arg!( Item, - |_| true, + NonterminalKind::Item, |parser: &mut Parser<'b>| parser.parse_item(ForceCollect::No), |x: Option>| x );