From 2247f9a0a0633b4728a853fe496b1a71521a70f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Tue, 26 Jan 2021 10:00:29 +0300 Subject: [PATCH 1/3] Add a parse recovery in array type syntax When user types [x, y] in a type context instead of [x; y] we now try to recover. In particular this fixes confusing "unmatched angle bracket" errors in code like fn main() { drop::<[(), 0]>([]); } which would previously generate --> src/main.rs:2:11 | 2 | drop::<[(), 0]>([]); | ^ help: remove extra angle bracket error: expected one of `;` or `]`, found `,` --> src/main.rs:2:15 | 2 | drop::<[(), 0]>([]); | ^ expected one of `;` or `]` With the recovery we now only generate the second error. Fixes #81097 Note that we only try this recovery in edition 2018 and later. In earlier editions functions without bodies are now allowed to have named arguments, so having this recovery is causing problems in cases like trait Foo { fn foo([a, b]: [i32; 2]) {} } where the `[a, b]` part is an error, and assuming `;` in place of `,` there makes the error message worse, not better. A test for this is in `ui/issues/issue-50571.rs`. --- compiler/rustc_parse/src/parser/ty.rs | 95 ++++++++++++++++++++++++--- src/test/ui/parser/issue-81097.rs | 3 + src/test/ui/parser/issue-81097.stderr | 13 ++++ 3 files changed, 102 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/parser/issue-81097.rs create mode 100644 src/test/ui/parser/issue-81097.stderr diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index 9553f5d09e83b..efdbb20d29048 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -60,6 +60,27 @@ pub(super) enum RecoverReturnSign { No, } +/// Signals whether to recover from invalid separator in array syntax `[; ]` (the +/// semicolon) +/// +/// We don't want this recovery when parsing pre-2018-edition trait method arguments, which don't +/// allow names or patterns in the arguments. Example: +/// +/// ``` +/// trait T { +/// fn foo([a, b]: [i32; 2]) {} +/// } +/// ``` +/// +/// Here `[a, b]` is parsed as a type and we need to fail without recovery/errors so that we can +/// check (after type parsing fails) whteher it looks like a pattern, and give a more helpful +/// message if it is. ui/issues/issue-50571.rs tests this. +#[derive(Copy, Clone, PartialEq)] +enum RecoverArrayType { + Yes, + No, +} + impl RecoverReturnSign { /// [RecoverReturnSign::Yes] allows for recovering `fn foo() => u8` and `fn foo(): u8`, /// [RecoverReturnSign::OnlyFatArrow] allows for recovering only `fn foo() => u8` (recovering @@ -98,6 +119,7 @@ impl<'a> Parser<'a> { AllowCVariadic::No, RecoverQPath::Yes, RecoverReturnSign::Yes, + RecoverArrayType::Yes, ) } @@ -110,6 +132,7 @@ impl<'a> Parser<'a> { AllowCVariadic::Yes, RecoverQPath::Yes, RecoverReturnSign::Yes, + RecoverArrayType::No, // see type documentation ) } @@ -125,6 +148,7 @@ impl<'a> Parser<'a> { AllowCVariadic::No, RecoverQPath::Yes, RecoverReturnSign::Yes, + RecoverArrayType::Yes, ) } @@ -135,6 +159,7 @@ impl<'a> Parser<'a> { AllowCVariadic::Yes, RecoverQPath::Yes, RecoverReturnSign::OnlyFatArrow, + RecoverArrayType::Yes, ) } @@ -152,6 +177,7 @@ impl<'a> Parser<'a> { AllowCVariadic::No, recover_qpath, recover_return_sign, + RecoverArrayType::Yes, )?; FnRetTy::Ty(ty) } else if recover_return_sign.can_recover(&self.token.kind) { @@ -171,6 +197,7 @@ impl<'a> Parser<'a> { AllowCVariadic::No, recover_qpath, recover_return_sign, + RecoverArrayType::Yes, )?; FnRetTy::Ty(ty) } else { @@ -184,6 +211,7 @@ impl<'a> Parser<'a> { allow_c_variadic: AllowCVariadic, recover_qpath: RecoverQPath, recover_return_sign: RecoverReturnSign, + recover_array_type: RecoverArrayType, ) -> PResult<'a, P> { let allow_qpath_recovery = recover_qpath == RecoverQPath::Yes; maybe_recover_from_interpolated_ty_qpath!(self, allow_qpath_recovery); @@ -199,7 +227,7 @@ impl<'a> Parser<'a> { } else if self.eat(&token::BinOp(token::Star)) { self.parse_ty_ptr()? } else if self.eat(&token::OpenDelim(token::Bracket)) { - self.parse_array_or_slice_ty()? + self.parse_array_or_slice_ty(recover_array_type)? } else if self.check(&token::BinOp(token::And)) || self.check(&token::AndAnd) { // Reference self.expect_and()?; @@ -346,7 +374,10 @@ impl<'a> Parser<'a> { /// Parses an array (`[TYPE; EXPR]`) or slice (`[TYPE]`) type. /// The opening `[` bracket is already eaten. - fn parse_array_or_slice_ty(&mut self) -> PResult<'a, TyKind> { + fn parse_array_or_slice_ty( + &mut self, + recover_array_type: RecoverArrayType, + ) -> PResult<'a, TyKind> { let elt_ty = match self.parse_ty() { Ok(ty) => ty, Err(mut err) @@ -360,13 +391,59 @@ impl<'a> Parser<'a> { } Err(err) => return Err(err), }; - let ty = if self.eat(&token::Semi) { - TyKind::Array(elt_ty, self.parse_anon_const_expr()?) - } else { - TyKind::Slice(elt_ty) - }; - self.expect(&token::CloseDelim(token::Bracket))?; - Ok(ty) + + match self.token.kind { + token::Semi => { + self.bump(); // consume ';' + let size_const = self.parse_anon_const_expr()?; + self.expect(&token::CloseDelim(token::Bracket))?; + Ok(TyKind::Array(elt_ty, size_const)) + } + token::CloseDelim(token::Bracket) => { + self.bump(); // consume ']' + Ok(TyKind::Slice(elt_ty)) + } + token::Comma if recover_array_type == RecoverArrayType::Yes => { + // Possibly used ',' instead of ';' in slice type syntax, try to recover + let comma_span = self.token.span; + self.bump(); // consume ',' + let snapshot = self.clone(); + match self.parse_anon_const_expr() { + Ok(array_size) => { + // Recovery successful, generate help at the position of `,` + let mut comma_err = + self.struct_span_err(comma_span, "expected `;`, found `,`"); + comma_err.span_suggestion_short( + comma_span, + "use `;` to separate the array length from the type", + ";".to_string(), + Applicability::MachineApplicable, + ); + comma_err.emit(); + self.expect(&token::CloseDelim(token::Bracket))?; + Ok(TyKind::Array(elt_ty, array_size)) + } + Err(mut const_expr_err) => { + // Recovery failed, restore parser state, emit an error for the ',' + const_expr_err.cancel(); + *self = snapshot; + let comma_err = self + .struct_span_err(comma_span, "expected one of `;` or `]`, found `,`"); + Err(comma_err) + } + } + } + _ => { + let err = self.struct_span_err( + self.token.span, + &format!( + "expected one of ';' or ']', found {}", + super::token_descr(&self.token) + ), + ); + Err(err) + } + } } fn parse_borrowed_pointee(&mut self) -> PResult<'a, TyKind> { diff --git a/src/test/ui/parser/issue-81097.rs b/src/test/ui/parser/issue-81097.rs new file mode 100644 index 0000000000000..70663f9a354ae --- /dev/null +++ b/src/test/ui/parser/issue-81097.rs @@ -0,0 +1,3 @@ +fn main() { + drop::<[(), 0]>([]); //~ ERROR: expected `;`, found `,` +} diff --git a/src/test/ui/parser/issue-81097.stderr b/src/test/ui/parser/issue-81097.stderr new file mode 100644 index 0000000000000..0a2d5bbd0bf75 --- /dev/null +++ b/src/test/ui/parser/issue-81097.stderr @@ -0,0 +1,13 @@ +error: expected `;`, found `,` + --> $DIR/issue-81097.rs:2:15 + | +LL | drop::<[(), 0]>([]); + | ^ + | +help: use `;` to separate the array length from the type + | +LL | drop::<[(); 0]>([]); + | ^ + +error: aborting due to previous error + From 261be3a6a2b67ba2be19838bc6d9b7bccb159c35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Sun, 14 Feb 2021 09:43:11 +0300 Subject: [PATCH 2/3] Update quotes in err msgs, update two tests --- compiler/rustc_parse/src/parser/ty.rs | 2 +- src/test/ui/parser/better-expected.rs | 2 +- src/test/ui/parser/better-expected.stderr | 4 ++-- src/test/ui/parser/removed-syntax-fixed-vec.rs | 2 +- src/test/ui/parser/removed-syntax-fixed-vec.stderr | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index efdbb20d29048..eb190287c8a8e 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -437,7 +437,7 @@ impl<'a> Parser<'a> { let err = self.struct_span_err( self.token.span, &format!( - "expected one of ';' or ']', found {}", + "expected one of `;` or `]`, found {}", super::token_descr(&self.token) ), ); diff --git a/src/test/ui/parser/better-expected.rs b/src/test/ui/parser/better-expected.rs index 16b61caa4dffc..b3b6c92f61f32 100644 --- a/src/test/ui/parser/better-expected.rs +++ b/src/test/ui/parser/better-expected.rs @@ -1,3 +1,3 @@ fn main() { - let x: [isize 3]; //~ ERROR expected one of `!`, `(`, `+`, `::`, `;`, `<`, or `]`, found `3` + let x: [isize 3]; //~ ERROR expected one of `;` or `]`, found `3` } diff --git a/src/test/ui/parser/better-expected.stderr b/src/test/ui/parser/better-expected.stderr index 21bf8d19a721e..395aa4d94b5a6 100644 --- a/src/test/ui/parser/better-expected.stderr +++ b/src/test/ui/parser/better-expected.stderr @@ -1,8 +1,8 @@ -error: expected one of `!`, `(`, `+`, `::`, `;`, `<`, or `]`, found `3` +error: expected one of `;` or `]`, found `3` --> $DIR/better-expected.rs:2:19 | LL | let x: [isize 3]; - | - ^ expected one of 7 possible tokens + | - ^ | | | while parsing the type for `x` diff --git a/src/test/ui/parser/removed-syntax-fixed-vec.rs b/src/test/ui/parser/removed-syntax-fixed-vec.rs index 560efecb91cfc..f0cd27e2ab61c 100644 --- a/src/test/ui/parser/removed-syntax-fixed-vec.rs +++ b/src/test/ui/parser/removed-syntax-fixed-vec.rs @@ -1 +1 @@ -type v = [isize * 3]; //~ ERROR expected one of `!`, `(`, `+`, `::`, `;`, `<`, or `]`, found `*` +type v = [isize * 3]; //~ ERROR expected one of `;` or `]`, found `*` diff --git a/src/test/ui/parser/removed-syntax-fixed-vec.stderr b/src/test/ui/parser/removed-syntax-fixed-vec.stderr index a2b97544f9e55..c9cb397571624 100644 --- a/src/test/ui/parser/removed-syntax-fixed-vec.stderr +++ b/src/test/ui/parser/removed-syntax-fixed-vec.stderr @@ -1,8 +1,8 @@ -error: expected one of `!`, `(`, `+`, `::`, `;`, `<`, or `]`, found `*` +error: expected one of `;` or `]`, found `*` --> $DIR/removed-syntax-fixed-vec.rs:1:17 | LL | type v = [isize * 3]; - | ^ expected one of 7 possible tokens + | ^ error: aborting due to previous error From 1288f17d5ee26c3b2f13d153aedc03ab983a3168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Sat, 27 Feb 2021 06:52:53 +0300 Subject: [PATCH 3/3] Revert to previous version --- compiler/rustc_parse/src/parser/ty.rs | 55 ++++++------------- src/test/ui/parser/better-expected.rs | 2 +- src/test/ui/parser/better-expected.stderr | 4 +- src/test/ui/parser/issue-81097.rs | 2 +- src/test/ui/parser/issue-81097.stderr | 9 +-- .../ui/parser/removed-syntax-fixed-vec.rs | 2 +- .../ui/parser/removed-syntax-fixed-vec.stderr | 4 +- 7 files changed, 26 insertions(+), 52 deletions(-) diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index eb190287c8a8e..ae8c4c8f493cf 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -392,57 +392,36 @@ impl<'a> Parser<'a> { Err(err) => return Err(err), }; - match self.token.kind { - token::Semi => { - self.bump(); // consume ';' - let size_const = self.parse_anon_const_expr()?; - self.expect(&token::CloseDelim(token::Bracket))?; - Ok(TyKind::Array(elt_ty, size_const)) - } - token::CloseDelim(token::Bracket) => { - self.bump(); // consume ']' - Ok(TyKind::Slice(elt_ty)) - } - token::Comma if recover_array_type == RecoverArrayType::Yes => { + let size_const = + if self.eat(&token::Semi) { Some(self.parse_anon_const_expr()?) } else { None }; + + match self.expect(&token::CloseDelim(token::Bracket)) { + Ok(_) => Ok(match size_const { + Some(size_const) => TyKind::Array(elt_ty, size_const), + None => TyKind::Slice(elt_ty), + }), + Err(mut bracket_err) + if recover_array_type == RecoverArrayType::Yes + && self.token.kind == token::Comma + && size_const.is_none() => + { // Possibly used ',' instead of ';' in slice type syntax, try to recover - let comma_span = self.token.span; - self.bump(); // consume ',' + self.bump(); let snapshot = self.clone(); match self.parse_anon_const_expr() { Ok(array_size) => { - // Recovery successful, generate help at the position of `,` - let mut comma_err = - self.struct_span_err(comma_span, "expected `;`, found `,`"); - comma_err.span_suggestion_short( - comma_span, - "use `;` to separate the array length from the type", - ";".to_string(), - Applicability::MachineApplicable, - ); - comma_err.emit(); + bracket_err.emit(); self.expect(&token::CloseDelim(token::Bracket))?; Ok(TyKind::Array(elt_ty, array_size)) } Err(mut const_expr_err) => { - // Recovery failed, restore parser state, emit an error for the ',' const_expr_err.cancel(); *self = snapshot; - let comma_err = self - .struct_span_err(comma_span, "expected one of `;` or `]`, found `,`"); - Err(comma_err) + Err(bracket_err) } } } - _ => { - let err = self.struct_span_err( - self.token.span, - &format!( - "expected one of `;` or `]`, found {}", - super::token_descr(&self.token) - ), - ); - Err(err) - } + Err(bracket_err) => Err(bracket_err), } } diff --git a/src/test/ui/parser/better-expected.rs b/src/test/ui/parser/better-expected.rs index b3b6c92f61f32..16b61caa4dffc 100644 --- a/src/test/ui/parser/better-expected.rs +++ b/src/test/ui/parser/better-expected.rs @@ -1,3 +1,3 @@ fn main() { - let x: [isize 3]; //~ ERROR expected one of `;` or `]`, found `3` + let x: [isize 3]; //~ ERROR expected one of `!`, `(`, `+`, `::`, `;`, `<`, or `]`, found `3` } diff --git a/src/test/ui/parser/better-expected.stderr b/src/test/ui/parser/better-expected.stderr index 395aa4d94b5a6..21bf8d19a721e 100644 --- a/src/test/ui/parser/better-expected.stderr +++ b/src/test/ui/parser/better-expected.stderr @@ -1,8 +1,8 @@ -error: expected one of `;` or `]`, found `3` +error: expected one of `!`, `(`, `+`, `::`, `;`, `<`, or `]`, found `3` --> $DIR/better-expected.rs:2:19 | LL | let x: [isize 3]; - | - ^ + | - ^ expected one of 7 possible tokens | | | while parsing the type for `x` diff --git a/src/test/ui/parser/issue-81097.rs b/src/test/ui/parser/issue-81097.rs index 70663f9a354ae..c1f8072431d85 100644 --- a/src/test/ui/parser/issue-81097.rs +++ b/src/test/ui/parser/issue-81097.rs @@ -1,3 +1,3 @@ fn main() { - drop::<[(), 0]>([]); //~ ERROR: expected `;`, found `,` + drop::<[(), 0]>([]); //~ ERROR: expected one of `;` or `]`, found `,` } diff --git a/src/test/ui/parser/issue-81097.stderr b/src/test/ui/parser/issue-81097.stderr index 0a2d5bbd0bf75..a486f5a079f0c 100644 --- a/src/test/ui/parser/issue-81097.stderr +++ b/src/test/ui/parser/issue-81097.stderr @@ -1,13 +1,8 @@ -error: expected `;`, found `,` +error: expected one of `;` or `]`, found `,` --> $DIR/issue-81097.rs:2:15 | LL | drop::<[(), 0]>([]); - | ^ - | -help: use `;` to separate the array length from the type - | -LL | drop::<[(); 0]>([]); - | ^ + | ^ expected one of `;` or `]` error: aborting due to previous error diff --git a/src/test/ui/parser/removed-syntax-fixed-vec.rs b/src/test/ui/parser/removed-syntax-fixed-vec.rs index f0cd27e2ab61c..560efecb91cfc 100644 --- a/src/test/ui/parser/removed-syntax-fixed-vec.rs +++ b/src/test/ui/parser/removed-syntax-fixed-vec.rs @@ -1 +1 @@ -type v = [isize * 3]; //~ ERROR expected one of `;` or `]`, found `*` +type v = [isize * 3]; //~ ERROR expected one of `!`, `(`, `+`, `::`, `;`, `<`, or `]`, found `*` diff --git a/src/test/ui/parser/removed-syntax-fixed-vec.stderr b/src/test/ui/parser/removed-syntax-fixed-vec.stderr index c9cb397571624..a2b97544f9e55 100644 --- a/src/test/ui/parser/removed-syntax-fixed-vec.stderr +++ b/src/test/ui/parser/removed-syntax-fixed-vec.stderr @@ -1,8 +1,8 @@ -error: expected one of `;` or `]`, found `*` +error: expected one of `!`, `(`, `+`, `::`, `;`, `<`, or `]`, found `*` --> $DIR/removed-syntax-fixed-vec.rs:1:17 | LL | type v = [isize * 3]; - | ^ + | ^ expected one of 7 possible tokens error: aborting due to previous error