From 3ee841c3351326a7bea83b689f54d9fee27e6e85 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Fri, 18 Mar 2016 15:49:12 +1300 Subject: [PATCH 1/3] Don't loop forever on error recovery with EOF closes #31804 --- src/libsyntax/parse/parser.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 6839f11cd70..66912abb6f5 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3824,7 +3824,9 @@ impl<'a> Parser<'a> { fn recover_stmt_(&mut self, break_on_semi: SemiColonMode) { let mut brace_depth = 0; let mut bracket_depth = 0; + debug!("recover_stmt_ enter loop"); loop { + debug!("recover_stmt_ loop {:?}", self.token); match self.token { token::OpenDelim(token::DelimToken::Brace) => { brace_depth += 1; @@ -3836,6 +3838,7 @@ impl<'a> Parser<'a> { } token::CloseDelim(token::DelimToken::Brace) => { if brace_depth == 0 { + debug!("recover_stmt_ return - close delim {:?}", self.token); return; } brace_depth -= 1; @@ -3848,12 +3851,16 @@ impl<'a> Parser<'a> { } self.bump(); } - token::Eof => return, + token::Eof => { + debug!("recover_stmt_ return - Eof"); + return; + } token::Semi => { self.bump(); if break_on_semi == SemiColonMode::Break && brace_depth == 0 && bracket_depth == 0 { + debug!("recover_stmt_ return - Semi"); return; } } @@ -4042,6 +4049,8 @@ impl<'a> Parser<'a> { while !self.eat(&token::CloseDelim(token::Brace)) { let Spanned {node, span} = if let Some(s) = self.parse_stmt_() { s + } else if self.token == token::Eof { + break; } else { // Found only `;` or `}`. continue; From 2731dc169c3e35707049575829cb106e2bdc9801 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Tue, 22 Mar 2016 16:02:09 +1300 Subject: [PATCH 2/3] Error recovery in the tokeniser Closes #31994 --- src/libsyntax/parse/parser.rs | 83 ++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 25 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 66912abb6f5..3010c040914 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -268,8 +268,8 @@ pub struct Parser<'a> { /// Used to determine the path to externally loaded source files pub filename: Option, pub mod_path_stack: Vec, - /// Stack of spans of open delimiters. Used for error message. - pub open_braces: Vec, + /// Stack of open delimiters and their spans. Used for error message. + pub open_braces: Vec<(token::DelimToken, Span)>, /// Flag if this parser "owns" the directory that it is currently parsing /// in. This will affect how nested files are looked up. pub owns_directory: bool, @@ -895,7 +895,7 @@ impl<'a> Parser<'a> { sep: SeqSep, f: F) -> Vec - where F: FnMut(&mut Parser<'a>) -> PResult<'a, T>, + where F: FnMut(&mut Parser<'a>) -> PResult<'a, T> { self.parse_seq_to_before_tokens(&[ket], sep, f, |mut e| e.emit()) } @@ -2755,8 +2755,8 @@ impl<'a> Parser<'a> { let mut err: DiagnosticBuilder<'a> = self.diagnostic().struct_span_err(self.span, "this file contains an un-closed delimiter"); - for sp in &self.open_braces { - err.span_help(*sp, "did you mean to close this delimiter?"); + for &(_, sp) in &self.open_braces { + err.span_help(sp, "did you mean to close this delimiter?"); } Err(err) @@ -2766,23 +2766,66 @@ impl<'a> Parser<'a> { let pre_span = self.span; // Parse the open delimiter. - self.open_braces.push(self.span); + self.open_braces.push((delim, self.span)); let open_span = self.span; self.bump(); - // Parse the token trees within the delimiters - let tts = self.parse_seq_to_before_end(&token::CloseDelim(delim), - SeqSep::none(), - |p| p.parse_token_tree()); + // Parse the token trees within the delimiters. + // We stop at any delimiter so we can try to recover if the user + // uses an incorrect delimiter. + let tts = self.parse_seq_to_before_tokens(&[&token::CloseDelim(token::Brace), + &token::CloseDelim(token::Paren), + &token::CloseDelim(token::Bracket)], + SeqSep::none(), + |p| p.parse_token_tree(), + |mut e| e.emit()); - // Parse the close delimiter. let close_span = self.span; - self.bump(); - self.open_braces.pop().unwrap(); - // Expand to cover the entire delimited token tree let span = Span { hi: close_span.hi, ..pre_span }; + match self.token { + // Correct delmiter. + token::CloseDelim(d) if d == delim => { + self.open_braces.pop().unwrap(); + + // Parse the close delimiter. + self.bump(); + } + // Incorect delimiter. + token::CloseDelim(other) => { + let token_str = self.this_token_to_string(); + let mut err = self.diagnostic().struct_span_err(self.span, + &format!("incorrect close delimiter: `{}`", token_str)); + // This is a conservative error: only report the last unclosed delimiter. + // The previous unclosed delimiters could actually be closed! The parser + // just hasn't gotten to them yet. + if let Some(&(_, sp)) = self.open_braces.last() { + err.span_note(sp, "unclosed delimiter"); + }; + err.emit(); + + self.open_braces.pop().unwrap(); + + // If the incorrect delimter matches an earlier opening + // delimiter, then don't consume it (it can be used to + // close the earlier one)Otherwise, consume it. + // E.g., we try to recover from: + // fn foo() { + // bar(baz( + // } // Incorrect delimiter but matches the earlier `{` + if !self.open_braces.iter().any(|&(b, _)| b == other) { + self.bump(); + } + } + token::Eof => { + // Silently recover, the EOF token will be seen again + // and an error emitted then. Thus we don't pop from + // self.open_braces here. + }, + _ => unreachable!(), + } + Ok(TokenTree::Delimited(span, Rc::new(Delimited { delim: delim, open_span: open_span, @@ -2798,17 +2841,7 @@ impl<'a> Parser<'a> { maybe_whole!(deref self, NtTT); match self.token { token::CloseDelim(_) => { - let token_str = self.this_token_to_string(); - let mut err = self.diagnostic().struct_span_err(self.span, - &format!("incorrect close delimiter: `{}`", token_str)); - // This is a conservative error: only report the last unclosed delimiter. - // The previous unclosed delimiters could actually be closed! The parser - // just hasn't gotten to them yet. - if let Some(&sp) = self.open_braces.last() { - err.span_note(sp, "unclosed delimiter"); - }; - - Err(err) + panic!("should have been caught above"); }, /* we ought to allow different depths of unquotation */ token::Dollar | token::SubstNt(..) if self.quote_depth > 0 => { From 180d6b55ca19c63347664f0622b6ccc37fb101f5 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Wed, 23 Mar 2016 09:24:54 +1300 Subject: [PATCH 3/3] Tests --- src/libsyntax/parse/parser.rs | 7 +++- .../issue-10636-2.rs | 8 ++--- src/test/compile-fail/issue-31804.rs | 16 +++++++++ .../compile-fail/token-error-correct-2.rs | 17 ++++++++++ .../compile-fail/token-error-correct-3.rs | 33 +++++++++++++++++++ src/test/compile-fail/token-error-correct.rs | 20 +++++++++++ src/test/parse-fail/issue-2354-1.rs | 2 +- .../macro-mismatched-delim-paren-brace.rs | 2 +- 8 files changed, 97 insertions(+), 8 deletions(-) rename src/test/{parse-fail => compile-fail}/issue-10636-2.rs (75%) create mode 100644 src/test/compile-fail/issue-31804.rs create mode 100644 src/test/compile-fail/token-error-correct-2.rs create mode 100644 src/test/compile-fail/token-error-correct-3.rs create mode 100644 src/test/compile-fail/token-error-correct.rs diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 3010c040914..29ef105eccb 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2841,7 +2841,12 @@ impl<'a> Parser<'a> { maybe_whole!(deref self, NtTT); match self.token { token::CloseDelim(_) => { - panic!("should have been caught above"); + // An unexpected closing delimiter (i.e., there is no + // matching opening delimiter). + let token_str = self.this_token_to_string(); + let err = self.diagnostic().struct_span_err(self.span, + &format!("unexpected close delimiter: `{}`", token_str)); + Err(err) }, /* we ought to allow different depths of unquotation */ token::Dollar | token::SubstNt(..) if self.quote_depth > 0 => { diff --git a/src/test/parse-fail/issue-10636-2.rs b/src/test/compile-fail/issue-10636-2.rs similarity index 75% rename from src/test/parse-fail/issue-10636-2.rs rename to src/test/compile-fail/issue-10636-2.rs index 41a3b06e655..eaccaf3cdbd 100644 --- a/src/test/parse-fail/issue-10636-2.rs +++ b/src/test/compile-fail/issue-10636-2.rs @@ -1,4 +1,4 @@ -// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// Copyright 2013-2016 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -11,11 +11,9 @@ // FIXME(31528) we emit a bunch of silly errors here due to continuing past the // first one. This would be easy-ish to address by better recovery in tokenisation. -// compile-flags: -Z parse-only - -pub fn trace_option(option: Option) { //~ HELP did you mean to close this delimiter? +pub fn trace_option(option: Option) { option.map(|some| 42; //~ NOTE: unclosed delimiter //~^ ERROR: expected one of + //~^^ ERROR: mismatched types } //~ ERROR: incorrect close delimiter //~^ ERROR: expected one of -//~ ERROR: this file contains an un-closed delimiter diff --git a/src/test/compile-fail/issue-31804.rs b/src/test/compile-fail/issue-31804.rs new file mode 100644 index 00000000000..b6a04bee85d --- /dev/null +++ b/src/test/compile-fail/issue-31804.rs @@ -0,0 +1,16 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that error recovery in the parser to an EOF does not give an infinite +// spew of errors. + +fn main() { + let +} //~ ERROR unexpected token: `}` diff --git a/src/test/compile-fail/token-error-correct-2.rs b/src/test/compile-fail/token-error-correct-2.rs new file mode 100644 index 00000000000..ab429ab8780 --- /dev/null +++ b/src/test/compile-fail/token-error-correct-2.rs @@ -0,0 +1,17 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that we do some basic error correcton in the tokeniser (and don't ICE). + +fn main() { + if foo { //~ NOTE: unclosed delimiter + //~^ ERROR: unresolved name `foo` + ) //~ ERROR: incorrect close delimiter: `)` +} diff --git a/src/test/compile-fail/token-error-correct-3.rs b/src/test/compile-fail/token-error-correct-3.rs new file mode 100644 index 00000000000..fe8c9f69013 --- /dev/null +++ b/src/test/compile-fail/token-error-correct-3.rs @@ -0,0 +1,33 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that we do some basic error correcton in the tokeniser (and don't spew +// too many bogus errors). + +pub mod raw { + use std::{io, fs}; + use std::path::Path; + + pub fn ensure_dir_exists, F: FnOnce(&Path)>(path: P, + callback: F) + -> io::Result { + if !is_directory(path.as_ref()) { //~ ERROR: unresolved name `is_directory` + callback(path.as_ref(); //~ NOTE: unclosed delimiter + //~^ ERROR: expected one of + fs::create_dir_all(path.as_ref()).map(|()| true) //~ ERROR: expected one of + } else { //~ ERROR: incorrect close delimiter: `}` + Ok(false); + } + + panic!(); + } +} + +fn main() {} diff --git a/src/test/compile-fail/token-error-correct.rs b/src/test/compile-fail/token-error-correct.rs new file mode 100644 index 00000000000..6c54acd7bdb --- /dev/null +++ b/src/test/compile-fail/token-error-correct.rs @@ -0,0 +1,20 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that we do some basic error correcton in the tokeniser. + +fn main() { + foo(bar(; //~ NOTE: unclosed delimiter + //~^ NOTE: unclosed delimiter + //~^^ ERROR: unexpected token: `;` + //~^^^ ERROR: unresolved name `bar` + //~^^^^ ERROR: unresolved name `foo` +} //~ ERROR: incorrect close delimiter: `}` +//~^ ERROR: incorrect close delimiter: `}` diff --git a/src/test/parse-fail/issue-2354-1.rs b/src/test/parse-fail/issue-2354-1.rs index e65f95780bb..f24c5440735 100644 --- a/src/test/parse-fail/issue-2354-1.rs +++ b/src/test/parse-fail/issue-2354-1.rs @@ -10,4 +10,4 @@ // compile-flags: -Z parse-only -static foo: isize = 2; } //~ ERROR incorrect close delimiter: +static foo: isize = 2; } //~ ERROR unexpected close delimiter: diff --git a/src/test/parse-fail/macro-mismatched-delim-paren-brace.rs b/src/test/parse-fail/macro-mismatched-delim-paren-brace.rs index 84094ab6ca8..cbc0ed0ccdb 100644 --- a/src/test/parse-fail/macro-mismatched-delim-paren-brace.rs +++ b/src/test/parse-fail/macro-mismatched-delim-paren-brace.rs @@ -14,4 +14,4 @@ fn main() { foo! ( bar, "baz", 1, 2.0 } //~ ERROR incorrect close delimiter -} +} //~ ERROR unexpected close delimiter: `}`