From 690addc6ecc575f6b9df5e2fc09bd119e51e6194 Mon Sep 17 00:00:00 2001 From: Yutaro Ohno Date: Sat, 3 Dec 2022 23:24:49 +0900 Subject: [PATCH 1/2] parser: fix ICE with invalid variable declaration in macro call Fix ICE on parsing an invalid variable declaration as a statement like: ``` macro_rules! m { ($s:stmt) => {} } m! { var x } ``` --- compiler/rustc_parse/src/parser/stmt.rs | 8 ++--- src/test/ui/macros/issue-103529.rs | 13 +++++++++ src/test/ui/macros/issue-103529.stderr | 39 +++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/macros/issue-103529.rs create mode 100644 src/test/ui/macros/issue-103529.stderr diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index ff1ddfd97dfc6..b481325054758 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -72,12 +72,12 @@ impl<'a> Parser<'a> { Ok(Some(if self.token.is_keyword(kw::Let) { self.parse_local_mk(lo, attrs, capture_semi, force_collect)? - } else if self.is_kw_followed_by_ident(kw::Mut) { + } else if self.is_kw_followed_by_ident(kw::Mut) && self.may_recover() { self.recover_stmt_local(lo, attrs, InvalidVariableDeclarationSub::MissingLet)? - } else if self.is_kw_followed_by_ident(kw::Auto) { + } else if self.is_kw_followed_by_ident(kw::Auto) && self.may_recover() { self.bump(); // `auto` self.recover_stmt_local(lo, attrs, InvalidVariableDeclarationSub::UseLetNotAuto)? - } else if self.is_kw_followed_by_ident(sym::var) { + } else if self.is_kw_followed_by_ident(sym::var) && self.may_recover() { self.bump(); // `var` self.recover_stmt_local(lo, attrs, InvalidVariableDeclarationSub::UseLetNotVar)? } else if self.check_path() && !self.token.is_qpath_start() && !self.is_path_start_item() { @@ -244,7 +244,7 @@ impl<'a> Parser<'a> { } fn recover_local_after_let(&mut self, lo: Span, attrs: AttrWrapper) -> PResult<'a, Stmt> { - self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| { + self.collect_tokens_trailing_token(attrs, ForceCollect::Yes, |this, attrs| { let local = this.parse_local(attrs)?; // FIXME - maybe capture semicolon in recovery? Ok(( diff --git a/src/test/ui/macros/issue-103529.rs b/src/test/ui/macros/issue-103529.rs new file mode 100644 index 0000000000000..fa05baed7fc8e --- /dev/null +++ b/src/test/ui/macros/issue-103529.rs @@ -0,0 +1,13 @@ +macro_rules! m { + ($s:stmt) => {} +} + +m! { mut x } +//~^ ERROR expected expression, found keyword `mut` +//~| ERROR expected a statement +m! { auto x } +//~^ ERROR invalid variable declaration +m! { var x } +//~^ ERROR invalid variable declaration + +fn main() {} diff --git a/src/test/ui/macros/issue-103529.stderr b/src/test/ui/macros/issue-103529.stderr new file mode 100644 index 0000000000000..61e322afc7709 --- /dev/null +++ b/src/test/ui/macros/issue-103529.stderr @@ -0,0 +1,39 @@ +error: expected expression, found keyword `mut` + --> $DIR/issue-103529.rs:5:6 + | +LL | m! { mut x } + | ^^^ expected expression + +error: expected a statement + --> $DIR/issue-103529.rs:5:10 + | +LL | ($s:stmt) => {} + | ------- while parsing argument for this `stmt` macro fragment +... +LL | m! { mut x } + | ^ + +error: invalid variable declaration + --> $DIR/issue-103529.rs:8:6 + | +LL | m! { auto x } + | ^^^^ + | +help: write `let` instead of `auto` to introduce a new variable + | +LL | m! { let x } + | ~~~ + +error: invalid variable declaration + --> $DIR/issue-103529.rs:10:6 + | +LL | m! { var x } + | ^^^ + | +help: write `let` instead of `var` to introduce a new variable + | +LL | m! { let x } + | ~~~ + +error: aborting due to 4 previous errors + From e4812583c70db2259b952829a7412dea96c5afdd Mon Sep 17 00:00:00 2001 From: Yutaro Ohno Date: Sat, 3 Dec 2022 23:37:23 +0900 Subject: [PATCH 2/2] parser: refactoring on recovery from invalid variable declarations Previously, the `recover_local_after_let` function was called from the body of the `recover_stmt_local` function. Unifying these two functions make it more simple and more readable. --- compiler/rustc_parse/src/parser/stmt.rs | 37 ++++++++++++++----------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index b481325054758..42197e6379749 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -73,13 +73,21 @@ impl<'a> Parser<'a> { Ok(Some(if self.token.is_keyword(kw::Let) { self.parse_local_mk(lo, attrs, capture_semi, force_collect)? } else if self.is_kw_followed_by_ident(kw::Mut) && self.may_recover() { - self.recover_stmt_local(lo, attrs, InvalidVariableDeclarationSub::MissingLet)? + self.recover_stmt_local_after_let(lo, attrs, InvalidVariableDeclarationSub::MissingLet)? } else if self.is_kw_followed_by_ident(kw::Auto) && self.may_recover() { self.bump(); // `auto` - self.recover_stmt_local(lo, attrs, InvalidVariableDeclarationSub::UseLetNotAuto)? + self.recover_stmt_local_after_let( + lo, + attrs, + InvalidVariableDeclarationSub::UseLetNotAuto, + )? } else if self.is_kw_followed_by_ident(sym::var) && self.may_recover() { self.bump(); // `var` - self.recover_stmt_local(lo, attrs, InvalidVariableDeclarationSub::UseLetNotVar)? + self.recover_stmt_local_after_let( + lo, + attrs, + InvalidVariableDeclarationSub::UseLetNotVar, + )? } else if self.check_path() && !self.token.is_qpath_start() && !self.is_path_start_item() { // We have avoided contextual keywords like `union`, items with `crate` visibility, // or `auto trait` items. We aim to parse an arbitrary path `a::b` but not something @@ -213,13 +221,21 @@ impl<'a> Parser<'a> { } } - fn recover_stmt_local( + fn recover_stmt_local_after_let( &mut self, lo: Span, attrs: AttrWrapper, subdiagnostic: fn(Span) -> InvalidVariableDeclarationSub, ) -> PResult<'a, Stmt> { - let stmt = self.recover_local_after_let(lo, attrs)?; + let stmt = + self.collect_tokens_trailing_token(attrs, ForceCollect::Yes, |this, attrs| { + let local = this.parse_local(attrs)?; + // FIXME - maybe capture semicolon in recovery? + Ok(( + this.mk_stmt(lo.to(this.prev_token.span), StmtKind::Local(local)), + TrailingToken::None, + )) + })?; self.sess.emit_err(InvalidVariableDeclaration { span: lo, sub: subdiagnostic(lo) }); Ok(stmt) } @@ -243,17 +259,6 @@ impl<'a> Parser<'a> { }) } - fn recover_local_after_let(&mut self, lo: Span, attrs: AttrWrapper) -> PResult<'a, Stmt> { - self.collect_tokens_trailing_token(attrs, ForceCollect::Yes, |this, attrs| { - let local = this.parse_local(attrs)?; - // FIXME - maybe capture semicolon in recovery? - Ok(( - this.mk_stmt(lo.to(this.prev_token.span), StmtKind::Local(local)), - TrailingToken::None, - )) - }) - } - /// Parses a local variable declaration. fn parse_local(&mut self, attrs: AttrVec) -> PResult<'a, P> { let lo = self.prev_token.span;