Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch stray { in let-chains #117770

Merged
merged 5 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
79 changes: 60 additions & 19 deletions compiler/rustc_parse/src/lexer/tokentrees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -116,24 +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![];
while parser.token != token::Eof {
if let Err(diff_err) = parser.err_diff_marker() {
diff_errs.push(diff_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
Expand Down Expand Up @@ -220,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<PErr<'a>>) -> Vec<PErr<'a>> {
// 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).
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
sjwang05 marked this conversation as resolved.
Show resolved Hide resolved
self.look_ahead(dist, |t| kws.iter().any(|&kw| t.is_keyword(kw)))
}

Expand Down
58 changes: 58 additions & 0 deletions tests/ui/parser/brace-in-let-chain.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// issue #117766

#![feature(let_chains)]
fn main() {
if let () = ()
&& let () = () { //~ERROR: found a `{` in the middle of a let-chain
&& let () = ()
{
}
}

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 {
_ if foo => {
&& let () = ()
_ => {}
}
}

fn foo() {
{
&& let () = ()
}

fn bar() {
if false {}
{
&& let () = ()
}

fn baz() {
if false {
{
&& let () = ()
}
} //~ERROR: this file contains an unclosed delimiter
65 changes: 65 additions & 0 deletions tests/ui/parser/brace-in-let-chain.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
error: this file contains an unclosed delimiter
--> $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
...
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 | }
| ^
Comment on lines +1 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to decide if it's better in general to silence this error (like we do for diff markers) or not is best for these cases... I would leave it as is for now, because I don't know if I can come up with a case where doing so would be detrimental, but suspect we can silence it.


error: found a `{` in the middle of a let-chain
--> $DIR/brace-in-let-chain.rs:14:24
|
LL | && let () = () {
| ^
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 () = () {
LL + && let () = ()
|

error: found a `{` in the middle of a let-chain
--> $DIR/brace-in-let-chain.rs:6:24
|
LL | && let () = () {
| ^
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 () = () {
LL + && let () = ()
|

error: aborting due to 3 previous errors