From 00942381575bc6c081835dbb38f68ce9603c032f Mon Sep 17 00:00:00 2001 From: sjwang05 <63834813+sjwang05@users.noreply.github.com> Date: Thu, 9 Nov 2023 18:46:45 -0800 Subject: [PATCH 1/5] Catch stray { in let-chains --- compiler/rustc_parse/src/lexer/tokentrees.rs | 34 +++++++++++++++- tests/ui/parser/brace-in-let-chain.rs | 28 +++++++++++++ tests/ui/parser/brace-in-let-chain.stderr | 42 ++++++++++++++++++++ 3 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 tests/ui/parser/brace-in-let-chain.rs create mode 100644 tests/ui/parser/brace-in-let-chain.stderr diff --git a/compiler/rustc_parse/src/lexer/tokentrees.rs b/compiler/rustc_parse/src/lexer/tokentrees.rs index 31d91fe80bd..7aa4ac7c4cb 100644 --- a/compiler/rustc_parse/src/lexer/tokentrees.rs +++ b/compiler/rustc_parse/src/lexer/tokentrees.rs @@ -5,7 +5,8 @@ use super::{StringReader, UnmatchedDelim}; use rustc_ast::token::{self, Delimiter, Token}; use rustc_ast::tokenstream::{DelimSpan, Spacing, TokenStream, TokenTree}; use rustc_ast_pretty::pprust::token_to_string; -use rustc_errors::PErr; +use rustc_errors::{Applicability, PErr}; +use rustc_span::symbol::kw; pub(super) struct TokenTreesReader<'a> { string_reader: StringReader<'a>, @@ -121,9 +122,40 @@ impl<'a> TokenTreesReader<'a> { // out instead of complaining about the unclosed delims. let mut parser = crate::stream_to_parser(self.string_reader.sess, tts, None); let mut diff_errs = vec![]; + // Suggest removing a `{` we think appears in an `if`/`while` condition + // We want to suggest removing a `{` only if we think we're in an `if`/`while` condition, but + // we have no way of tracking this in the lexer itself, so we piggyback on the parser + let mut in_cond = false; while parser.token != token::Eof { if let Err(diff_err) = parser.err_diff_marker() { diff_errs.push(diff_err); + } else if parser.token.is_keyword(kw::If) { + in_cond = true; + } else if parser.token == token::CloseDelim(Delimiter::Brace) { + in_cond = false; + } else if in_cond && parser.token == token::OpenDelim(Delimiter::Brace) { + // Store the `&&` and `let` to use their spans later when creating the diagnostic + let maybe_andand = parser.look_ahead(1, |t| t.clone()); + let maybe_let = parser.look_ahead(2, |t| t.clone()); + if maybe_andand == token::OpenDelim(Delimiter::Brace) { + // This might be the beginning of the `if`/`while` body (i.e., the end of the condition) + in_cond = false; + } else if maybe_andand == token::AndAnd && maybe_let.is_keyword(kw::Let) { + let mut err = parser.struct_span_err( + parser.token.span, + "found a `{` in the middle of a let-chain", + ); + err.span_suggestion( + parser.token.span, + "consider removing this brace to parse the `let` as part of the same chain", + "", Applicability::MachineApplicable + ); + err.span_note( + maybe_andand.span.to(maybe_let.span), + "you might have meant to continue the let-chain here", + ); + errs.push(err); + } } parser.bump(); } diff --git a/tests/ui/parser/brace-in-let-chain.rs b/tests/ui/parser/brace-in-let-chain.rs new file mode 100644 index 00000000000..4dc13fb3847 --- /dev/null +++ b/tests/ui/parser/brace-in-let-chain.rs @@ -0,0 +1,28 @@ +// issue #117766 + +#![feature(let_chains)] +fn main() { + if let () = () + && let () = () { //~ERROR: found a `{` in the middle of a let-chain + && let () = () + { + } +} + +fn foo() { + { + && let () = () +} + +fn bar() { + if false {} + { + && let () = () +} + +fn baz() { + if false { + { + && let () = () + } +} //~ERROR: this file contains an unclosed delimiter diff --git a/tests/ui/parser/brace-in-let-chain.stderr b/tests/ui/parser/brace-in-let-chain.stderr new file mode 100644 index 00000000000..7550d5c43cf --- /dev/null +++ b/tests/ui/parser/brace-in-let-chain.stderr @@ -0,0 +1,42 @@ +error: this file contains an unclosed delimiter + --> $DIR/brace-in-let-chain.rs:28:54 + | +LL | fn main() { + | - unclosed delimiter +... +LL | fn foo() { + | - unclosed delimiter +... +LL | fn bar() { + | - unclosed delimiter +... +LL | fn baz() { + | - unclosed delimiter +LL | if false { +LL | { + | - this delimiter might not be properly closed... +LL | && let () = () +LL | } + | - ...as it matches this but it has different indentation +LL | } + | ^ + +error: found a `{` in the middle of a let-chain + --> $DIR/brace-in-let-chain.rs:6:24 + | +LL | && let () = () { + | ^ + | +note: you might have meant to continue the let-chain here + --> $DIR/brace-in-let-chain.rs:7:9 + | +LL | && let () = () + | ^^^^^^ +help: consider removing this brace to parse the `let` as part of the same chain + | +LL - && let () = () { +LL + && let () = () + | + +error: aborting due to 2 previous errors + From 9455259450f0186df991a14d960bb3759e7eac43 Mon Sep 17 00:00:00 2001 From: sjwang05 <63834813+sjwang05@users.noreply.github.com> Date: Thu, 9 Nov 2023 20:04:55 -0800 Subject: [PATCH 2/5] Catch an edge case --- compiler/rustc_parse/src/lexer/tokentrees.rs | 6 +++++- tests/ui/parser/brace-in-let-chain.rs | 9 +++++++++ tests/ui/parser/brace-in-let-chain.stderr | 5 ++++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_parse/src/lexer/tokentrees.rs b/compiler/rustc_parse/src/lexer/tokentrees.rs index 7aa4ac7c4cb..e7d2c678824 100644 --- a/compiler/rustc_parse/src/lexer/tokentrees.rs +++ b/compiler/rustc_parse/src/lexer/tokentrees.rs @@ -131,7 +131,11 @@ impl<'a> TokenTreesReader<'a> { diff_errs.push(diff_err); } else if parser.token.is_keyword(kw::If) { in_cond = true; - } else if parser.token == token::CloseDelim(Delimiter::Brace) { + } else if matches!( + parser.token.kind, + token::CloseDelim(Delimiter::Brace) | token::FatArrow + ) { + // end of the `if`/`while` body, or the end of a `match` guard in_cond = false; } else if in_cond && parser.token == token::OpenDelim(Delimiter::Brace) { // Store the `&&` and `let` to use their spans later when creating the diagnostic diff --git a/tests/ui/parser/brace-in-let-chain.rs b/tests/ui/parser/brace-in-let-chain.rs index 4dc13fb3847..78060e238d4 100644 --- a/tests/ui/parser/brace-in-let-chain.rs +++ b/tests/ui/parser/brace-in-let-chain.rs @@ -9,6 +9,15 @@ fn main() { } } +fn qux() { + let foo = false; + match foo { + _ if foo => { + && let () = () + _ => {} + } +} + fn foo() { { && let () = () diff --git a/tests/ui/parser/brace-in-let-chain.stderr b/tests/ui/parser/brace-in-let-chain.stderr index 7550d5c43cf..8e20cc43421 100644 --- a/tests/ui/parser/brace-in-let-chain.stderr +++ b/tests/ui/parser/brace-in-let-chain.stderr @@ -1,9 +1,12 @@ error: this file contains an unclosed delimiter - --> $DIR/brace-in-let-chain.rs:28:54 + --> $DIR/brace-in-let-chain.rs:37:54 | LL | fn main() { | - unclosed delimiter ... +LL | fn qux() { + | - unclosed delimiter +... LL | fn foo() { | - unclosed delimiter ... From a49368f00b66409ae9fcf42d52e4f8246c73c266 Mon Sep 17 00:00:00 2001 From: sjwang05 <63834813+sjwang05@users.noreply.github.com> Date: Fri, 10 Nov 2023 12:13:53 -0800 Subject: [PATCH 3/5] Correctly handle while-let-chains --- compiler/rustc_parse/src/lexer/tokentrees.rs | 2 +- compiler/rustc_parse/src/parser/mod.rs | 2 +- tests/ui/parser/brace-in-let-chain.rs | 21 ++++++++++++++ tests/ui/parser/brace-in-let-chain.stderr | 30 ++++++++++++++++++-- 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_parse/src/lexer/tokentrees.rs b/compiler/rustc_parse/src/lexer/tokentrees.rs index e7d2c678824..41f4d0055aa 100644 --- a/compiler/rustc_parse/src/lexer/tokentrees.rs +++ b/compiler/rustc_parse/src/lexer/tokentrees.rs @@ -129,7 +129,7 @@ impl<'a> TokenTreesReader<'a> { while parser.token != token::Eof { if let Err(diff_err) = parser.err_diff_marker() { diff_errs.push(diff_err); - } else if parser.token.is_keyword(kw::If) { + } else if parser.is_keyword_ahead(0, &[kw::If, kw::While]) { in_cond = true; } else if matches!( parser.token.kind, diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 1a7ae406911..6eab140117e 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -1115,7 +1115,7 @@ impl<'a> Parser<'a> { } /// Returns whether any of the given keywords are `dist` tokens ahead of the current one. - fn is_keyword_ahead(&self, dist: usize, kws: &[Symbol]) -> bool { + pub fn is_keyword_ahead(&self, dist: usize, kws: &[Symbol]) -> bool { self.look_ahead(dist, |t| kws.iter().any(|&kw| t.is_keyword(kw))) } diff --git a/tests/ui/parser/brace-in-let-chain.rs b/tests/ui/parser/brace-in-let-chain.rs index 78060e238d4..1f34c73a2c3 100644 --- a/tests/ui/parser/brace-in-let-chain.rs +++ b/tests/ui/parser/brace-in-let-chain.rs @@ -9,6 +9,27 @@ fn main() { } } +fn quux() { + while let () = () + && let () = () { //~ERROR: found a `{` in the middle of a let-chain + && let () = () + { + } +} + +fn foobar() { + while false {} + { + && let () = () +} + +fn fubar() { + while false { + { + && let () = () + } +} + fn qux() { let foo = false; match foo { diff --git a/tests/ui/parser/brace-in-let-chain.stderr b/tests/ui/parser/brace-in-let-chain.stderr index 8e20cc43421..c43310f4736 100644 --- a/tests/ui/parser/brace-in-let-chain.stderr +++ b/tests/ui/parser/brace-in-let-chain.stderr @@ -1,9 +1,18 @@ error: this file contains an unclosed delimiter - --> $DIR/brace-in-let-chain.rs:37:54 + --> $DIR/brace-in-let-chain.rs:58:54 | LL | fn main() { | - unclosed delimiter ... +LL | fn quux() { + | - unclosed delimiter +... +LL | fn foobar() { + | - unclosed delimiter +... +LL | fn fubar() { + | - unclosed delimiter +... LL | fn qux() { | - unclosed delimiter ... @@ -24,6 +33,23 @@ LL | } LL | } | ^ +error: found a `{` in the middle of a let-chain + --> $DIR/brace-in-let-chain.rs:14:24 + | +LL | && let () = () { + | ^ + | +note: you might have meant to continue the let-chain here + --> $DIR/brace-in-let-chain.rs:15:9 + | +LL | && let () = () + | ^^^^^^ +help: consider removing this brace to parse the `let` as part of the same chain + | +LL - && let () = () { +LL + && let () = () + | + error: found a `{` in the middle of a let-chain --> $DIR/brace-in-let-chain.rs:6:24 | @@ -41,5 +67,5 @@ LL - && let () = () { LL + && let () = () | -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors From f88cf0206faa5fb169deebbbd1024d9a0381ecd3 Mon Sep 17 00:00:00 2001 From: sjwang05 <63834813+sjwang05@users.noreply.github.com> Date: Sat, 11 Nov 2023 13:39:08 -0800 Subject: [PATCH 4/5] Move unclosed delim errors to separate function --- compiler/rustc_parse/src/lexer/tokentrees.rs | 111 ++++++++++--------- tests/ui/parser/brace-in-let-chain.stderr | 14 +-- 2 files changed, 62 insertions(+), 63 deletions(-) diff --git a/compiler/rustc_parse/src/lexer/tokentrees.rs b/compiler/rustc_parse/src/lexer/tokentrees.rs index 41f4d0055aa..db795ce9f72 100644 --- a/compiler/rustc_parse/src/lexer/tokentrees.rs +++ b/compiler/rustc_parse/src/lexer/tokentrees.rs @@ -117,59 +117,8 @@ impl<'a> TokenTreesReader<'a> { // We stop at any delimiter so we can try to recover if the user // uses an incorrect delimiter. let (tts, res) = self.parse_token_trees(/* is_delimited */ true); - if let Err(mut errs) = res { - // If there are unclosed delims, see if there are diff markers and if so, point them - // out instead of complaining about the unclosed delims. - let mut parser = crate::stream_to_parser(self.string_reader.sess, tts, None); - let mut diff_errs = vec![]; - // Suggest removing a `{` we think appears in an `if`/`while` condition - // We want to suggest removing a `{` only if we think we're in an `if`/`while` condition, but - // we have no way of tracking this in the lexer itself, so we piggyback on the parser - let mut in_cond = false; - while parser.token != token::Eof { - if let Err(diff_err) = parser.err_diff_marker() { - diff_errs.push(diff_err); - } else if parser.is_keyword_ahead(0, &[kw::If, kw::While]) { - in_cond = true; - } else if matches!( - parser.token.kind, - token::CloseDelim(Delimiter::Brace) | token::FatArrow - ) { - // end of the `if`/`while` body, or the end of a `match` guard - in_cond = false; - } else if in_cond && parser.token == token::OpenDelim(Delimiter::Brace) { - // Store the `&&` and `let` to use their spans later when creating the diagnostic - let maybe_andand = parser.look_ahead(1, |t| t.clone()); - let maybe_let = parser.look_ahead(2, |t| t.clone()); - if maybe_andand == token::OpenDelim(Delimiter::Brace) { - // This might be the beginning of the `if`/`while` body (i.e., the end of the condition) - in_cond = false; - } else if maybe_andand == token::AndAnd && maybe_let.is_keyword(kw::Let) { - let mut err = parser.struct_span_err( - parser.token.span, - "found a `{` in the middle of a let-chain", - ); - err.span_suggestion( - parser.token.span, - "consider removing this brace to parse the `let` as part of the same chain", - "", Applicability::MachineApplicable - ); - err.span_note( - maybe_andand.span.to(maybe_let.span), - "you might have meant to continue the let-chain here", - ); - errs.push(err); - } - } - parser.bump(); - } - if !diff_errs.is_empty() { - errs.iter_mut().for_each(|err| { - err.delay_as_bug(); - }); - return Err(diff_errs); - } - return Err(errs); + if let Err(errs) = res { + return Err(self.unclosed_delim_err(tts, errs)); } // Expand to cover the entire delimited token tree @@ -256,6 +205,62 @@ impl<'a> TokenTreesReader<'a> { Ok(TokenTree::Delimited(delim_span, open_delim, tts)) } + fn unclosed_delim_err(&mut self, tts: TokenStream, mut errs: Vec>) -> Vec> { + // If there are unclosed delims, see if there are diff markers and if so, point them + // out instead of complaining about the unclosed delims. + let mut parser = crate::stream_to_parser(self.string_reader.sess, tts, None); + let mut diff_errs = vec![]; + // Suggest removing a `{` we think appears in an `if`/`while` condition + // We want to suggest removing a `{` only if we think we're in an `if`/`while` condition, but + // we have no way of tracking this in the lexer itself, so we piggyback on the parser + let mut in_cond = false; + while parser.token != token::Eof { + if let Err(diff_err) = parser.err_diff_marker() { + diff_errs.push(diff_err); + } else if parser.is_keyword_ahead(0, &[kw::If, kw::While]) { + in_cond = true; + } else if matches!( + parser.token.kind, + token::CloseDelim(Delimiter::Brace) | token::FatArrow + ) { + // end of the `if`/`while` body, or the end of a `match` guard + in_cond = false; + } else if in_cond && parser.token == token::OpenDelim(Delimiter::Brace) { + // Store the `&&` and `let` to use their spans later when creating the diagnostic + let maybe_andand = parser.look_ahead(1, |t| t.clone()); + let maybe_let = parser.look_ahead(2, |t| t.clone()); + if maybe_andand == token::OpenDelim(Delimiter::Brace) { + // This might be the beginning of the `if`/`while` body (i.e., the end of the condition) + in_cond = false; + } else if maybe_andand == token::AndAnd && maybe_let.is_keyword(kw::Let) { + let mut err = parser.struct_span_err( + parser.token.span, + "found a `{` in the middle of a let-chain", + ); + err.span_suggestion( + parser.token.span, + "consider removing this brace to parse the `let` as part of the same chain", + "", + Applicability::MachineApplicable, + ); + err.span_label( + maybe_andand.span.to(maybe_let.span), + "you might have meant to continue the let-chain here", + ); + errs.push(err); + } + } + parser.bump(); + } + if !diff_errs.is_empty() { + errs.iter_mut().for_each(|err| { + err.delay_as_bug(); + }); + return diff_errs; + } + return errs; + } + fn close_delim_err(&mut self, delim: Delimiter) -> PErr<'a> { // An unexpected closing delimiter (i.e., there is no // matching opening delimiter). diff --git a/tests/ui/parser/brace-in-let-chain.stderr b/tests/ui/parser/brace-in-let-chain.stderr index c43310f4736..7182d86d001 100644 --- a/tests/ui/parser/brace-in-let-chain.stderr +++ b/tests/ui/parser/brace-in-let-chain.stderr @@ -38,12 +38,9 @@ error: found a `{` in the middle of a let-chain | LL | && let () = () { | ^ - | -note: you might have meant to continue the let-chain here - --> $DIR/brace-in-let-chain.rs:15:9 - | LL | && let () = () - | ^^^^^^ + | ------ you might have meant to continue the let-chain here + | help: consider removing this brace to parse the `let` as part of the same chain | LL - && let () = () { @@ -55,12 +52,9 @@ error: found a `{` in the middle of a let-chain | LL | && let () = () { | ^ - | -note: you might have meant to continue the let-chain here - --> $DIR/brace-in-let-chain.rs:7:9 - | LL | && let () = () - | ^^^^^^ + | ------ you might have meant to continue the let-chain here + | help: consider removing this brace to parse the `let` as part of the same chain | LL - && let () = () { From 274824b917c290d6ac9227e1c263a3358ba6e6f2 Mon Sep 17 00:00:00 2001 From: sjwang05 <63834813+sjwang05@users.noreply.github.com> Date: Sun, 12 Nov 2023 14:46:01 -0800 Subject: [PATCH 5/5] Fix `is_keyword_ahead` visibility Co-authored-by: Takayuki Maeda --- compiler/rustc_parse/src/parser/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 6eab140117e..76f3f21a516 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -1115,7 +1115,7 @@ impl<'a> Parser<'a> { } /// Returns whether any of the given keywords are `dist` tokens ahead of the current one. - pub fn is_keyword_ahead(&self, dist: usize, kws: &[Symbol]) -> bool { + pub(crate) fn is_keyword_ahead(&self, dist: usize, kws: &[Symbol]) -> bool { self.look_ahead(dist, |t| kws.iter().any(|&kw| t.is_keyword(kw))) }