Skip to content

Commit

Permalink
Rollup merge of rust-lang#65838 - estebank:resilient-recovery, r=Centril
Browse files Browse the repository at this point in the history
Reduce amount of errors given unclosed delimiter

When in a file with a non-terminated item, catch the error and consume
the block instead of trying to recover it on a more granular way in order to
reduce the amount of unrelated errors that would be fixed after adding
the missing closing brace. Also point out the possible location of the
missing closing brace.

Fix rust-lang#63690.
  • Loading branch information
tmandry committed Nov 1, 2019
2 parents a5efbe3 + 454e2aa commit 6ea6cdc
Show file tree
Hide file tree
Showing 24 changed files with 293 additions and 153 deletions.
2 changes: 1 addition & 1 deletion src/librustc_metadata/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ impl cstore::CStore {
let source_file = sess.parse_sess.source_map().new_source_file(source_name, def.body);
let local_span = Span::with_root_ctxt(source_file.start_pos, source_file.end_pos);
let (body, mut errors) = source_file_to_stream(&sess.parse_sess, source_file, None);
emit_unclosed_delims(&mut errors, &sess.diagnostic());
emit_unclosed_delims(&mut errors, &sess.parse_sess);

// Mark the attrs as used
let attrs = data.get_item_attrs(id.index, sess);
Expand Down
9 changes: 8 additions & 1 deletion src/librustc_passes/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,14 @@ fn configure_main(tcx: TyCtxt<'_>, visitor: &EntryContext<'_, '_>) -> Option<(De
}

fn no_main_err(tcx: TyCtxt<'_>, visitor: &EntryContext<'_, '_>) {
let sp = tcx.hir().krate().span;
if *tcx.sess.parse_sess.reached_eof.borrow() {
// There's an unclosed brace that made the parser reach `Eof`, we shouldn't complain about
// the missing `fn main()` then as it might have been hidden inside an unclosed block.
tcx.sess.delay_span_bug(sp, "`main` not found, but expected unclosed brace error");
return;
}

// There is no main function.
let mut err = struct_err!(tcx.sess, E0601,
"`main` function not found in crate `{}`", tcx.crate_name(LOCAL_CRATE));
Expand All @@ -173,7 +181,6 @@ fn no_main_err(tcx: TyCtxt<'_>, visitor: &EntryContext<'_, '_>) {
} else {
String::from("consider adding a `main` function at the crate level")
};
let sp = tcx.hir().krate().span;
// The file may be empty, which leads to the diagnostic machinery not emitting this
// note. This is a relatively simple way to detect that case and emit a span-less
// note instead.
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/parse/lexer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ mod unicode_chars;
#[derive(Clone, Debug)]
pub struct UnmatchedBrace {
pub expected_delim: token::DelimToken,
pub found_delim: token::DelimToken,
pub found_delim: Option<token::DelimToken>,
pub found_span: Span,
pub unclosed_span: Option<Span>,
pub candidate_span: Option<Span>,
Expand Down
9 changes: 8 additions & 1 deletion src/libsyntax/parse/lexer/tokentrees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ impl<'a> TokenTreesReader<'a> {
.struct_span_err(self.token.span, msg);
for &(_, sp) in &self.open_braces {
err.span_label(sp, "un-closed delimiter");
self.unmatched_braces.push(UnmatchedBrace {
expected_delim: token::DelimToken::Brace,
found_delim: None,
found_span: self.token.span,
unclosed_span: Some(sp),
candidate_span: None,
});
}

if let Some((delim, _)) = self.open_braces.last() {
Expand Down Expand Up @@ -170,7 +177,7 @@ impl<'a> TokenTreesReader<'a> {
let (tok, _) = self.open_braces.pop().unwrap();
self.unmatched_braces.push(UnmatchedBrace {
expected_delim: tok,
found_delim: other,
found_delim: Some(other),
found_span: self.token.span,
unclosed_span: unclosed_delimiter,
candidate_span: candidate,
Expand Down
17 changes: 4 additions & 13 deletions src/libsyntax/parse/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! The main parser interface.

use crate::ast;
use crate::parse::parser::{Parser, emit_unclosed_delims};
use crate::parse::parser::{Parser, emit_unclosed_delims, make_unclosed_delims_error};
use crate::parse::token::Nonterminal;
use crate::tokenstream::{self, TokenStream, TokenTree};
use crate::print::pprust;
Expand Down Expand Up @@ -108,7 +108,7 @@ pub fn parse_stream_from_source_str(
sess.source_map().new_source_file(name, source),
override_span,
);
emit_unclosed_delims(&mut errors, &sess.span_diagnostic);
emit_unclosed_delims(&mut errors, &sess);
stream
}

Expand Down Expand Up @@ -242,18 +242,9 @@ pub fn maybe_file_to_stream(
err.buffer(&mut buffer);
// Not using `emit_unclosed_delims` to use `db.buffer`
for unmatched in unmatched_braces {
let mut db = sess.span_diagnostic.struct_span_err(unmatched.found_span, &format!(
"incorrect close delimiter: `{}`",
pprust::token_kind_to_string(&token::CloseDelim(unmatched.found_delim)),
));
db.span_label(unmatched.found_span, "incorrect close delimiter");
if let Some(sp) = unmatched.candidate_span {
db.span_label(sp, "close delimiter possibly meant for this");
if let Some(err) = make_unclosed_delims_error(unmatched, &sess) {
err.buffer(&mut buffer);
}
if let Some(sp) = unmatched.unclosed_span {
db.span_label(sp, "un-closed delimiter");
}
db.buffer(&mut buffer);
}
Err(buffer)
}
Expand Down
46 changes: 28 additions & 18 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::symbol::{kw, sym, Symbol};
use crate::tokenstream::{self, DelimSpan, TokenTree, TokenStream, TreeAndJoint};
use crate::ThinVec;

use errors::{Applicability, DiagnosticId, FatalError};
use errors::{Applicability, DiagnosticBuilder, DiagnosticId, FatalError};
use rustc_target::spec::abi::{self, Abi};
use syntax_pos::{Span, BytePos, DUMMY_SP, FileName};
use log::debug;
Expand Down Expand Up @@ -148,8 +148,7 @@ pub struct Parser<'a> {

impl<'a> Drop for Parser<'a> {
fn drop(&mut self) {
let diag = self.diagnostic();
emit_unclosed_delims(&mut self.unclosed_delims, diag);
emit_unclosed_delims(&mut self.unclosed_delims, &self.sess);
}
}

Expand Down Expand Up @@ -1370,20 +1369,31 @@ impl<'a> Parser<'a> {
}
}

pub fn emit_unclosed_delims(unclosed_delims: &mut Vec<UnmatchedBrace>, handler: &errors::Handler) {
for unmatched in unclosed_delims.iter() {
let mut err = handler.struct_span_err(unmatched.found_span, &format!(
"incorrect close delimiter: `{}`",
pprust::token_kind_to_string(&token::CloseDelim(unmatched.found_delim)),
));
err.span_label(unmatched.found_span, "incorrect close delimiter");
if let Some(sp) = unmatched.candidate_span {
err.span_label(sp, "close delimiter possibly meant for this");
}
if let Some(sp) = unmatched.unclosed_span {
err.span_label(sp, "un-closed delimiter");
}
err.emit();
crate fn make_unclosed_delims_error(
unmatched: UnmatchedBrace,
sess: &ParseSess,
) -> Option<DiagnosticBuilder<'_>> {
// `None` here means an `Eof` was found. We already emit those errors elsewhere, we add them to
// `unmatched_braces` only for error recovery in the `Parser`.
let found_delim = unmatched.found_delim?;
let mut err = sess.span_diagnostic.struct_span_err(unmatched.found_span, &format!(
"incorrect close delimiter: `{}`",
pprust::token_kind_to_string(&token::CloseDelim(found_delim)),
));
err.span_label(unmatched.found_span, "incorrect close delimiter");
if let Some(sp) = unmatched.candidate_span {
err.span_label(sp, "close delimiter possibly meant for this");
}
if let Some(sp) = unmatched.unclosed_span {
err.span_label(sp, "un-closed delimiter");
}
Some(err)
}

pub fn emit_unclosed_delims(unclosed_delims: &mut Vec<UnmatchedBrace>, sess: &ParseSess) {
*sess.reached_eof.borrow_mut() |= unclosed_delims.iter()
.any(|unmatched_delim| unmatched_delim.found_delim.is_none());
for unmatched in unclosed_delims.drain(..) {
make_unclosed_delims_error(unmatched, sess).map(|mut e| e.emit());
}
unclosed_delims.clear();
}
50 changes: 42 additions & 8 deletions src/libsyntax/parse/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,12 @@ impl RecoverQPath for Expr {
}
}

/// Control whether the closing delimiter should be consumed when calling `Parser::consume_block`.
crate enum ConsumeClosingDelim {
Yes,
No,
}

impl<'a> Parser<'a> {
pub fn fatal(&self, m: &str) -> DiagnosticBuilder<'a> {
self.span_fatal(self.token.span, m)
Expand Down Expand Up @@ -1105,8 +1111,8 @@ impl<'a> Parser<'a> {
Ok(x) => x,
Err(mut err) => {
err.emit();
// Recover from parse error.
self.consume_block(delim);
// Recover from parse error, callers expect the closing delim to be consumed.
self.consume_block(delim, ConsumeClosingDelim::Yes);
self.mk_expr(lo.to(self.prev_span), ExprKind::Err, ThinVec::new())
}
}
Expand Down Expand Up @@ -1135,6 +1141,11 @@ impl<'a> Parser<'a> {
// Don't attempt to recover from this unclosed delimiter more than once.
let unmatched = self.unclosed_delims.remove(pos);
let delim = TokenType::Token(token::CloseDelim(unmatched.expected_delim));
if unmatched.found_delim.is_none() {
// We encountered `Eof`, set this fact here to avoid complaining about missing
// `fn main()` when we found place to suggest the closing brace.
*self.sess.reached_eof.borrow_mut() = true;
}

// We want to suggest the inclusion of the closing delimiter where it makes
// the most sense, which is immediately after the last token:
Expand All @@ -1154,17 +1165,29 @@ impl<'a> Parser<'a> {
delim.to_string(),
Applicability::MaybeIncorrect,
);
err.emit();
self.expected_tokens.clear(); // reduce errors
Ok(true)
if unmatched.found_delim.is_none() {
// Encountered `Eof` when lexing blocks. Do not recover here to avoid knockdown
// errors which would be emitted elsewhere in the parser and let other error
// recovery consume the rest of the file.
Err(err)
} else {
err.emit();
self.expected_tokens.clear(); // Reduce the number of errors.
Ok(true)
}
}
_ => Err(err),
}
}

/// Recovers from `pub` keyword in places where it seems _reasonable_ but isn't valid.
pub(super) fn eat_bad_pub(&mut self) {
if self.token.is_keyword(kw::Pub) {
// When `unclosed_delims` is populated, it means that the code being parsed is already
// quite malformed, which might mean that, for example, a pub struct definition could be
// parsed as being a trait item, which is invalid and this error would trigger
// unconditionally, resulting in misleading diagnostics. Because of this, we only attempt
// this nice to have recovery for code that is otherwise well formed.
if self.token.is_keyword(kw::Pub) && self.unclosed_delims.is_empty() {
match self.parse_visibility(false) {
Ok(vis) => {
self.diagnostic()
Expand Down Expand Up @@ -1422,15 +1445,26 @@ impl<'a> Parser<'a> {
Ok(param)
}

pub(super) fn consume_block(&mut self, delim: token::DelimToken) {
pub(super) fn consume_block(
&mut self,
delim: token::DelimToken,
consume_close: ConsumeClosingDelim,
) {
let mut brace_depth = 0;
loop {
if self.eat(&token::OpenDelim(delim)) {
brace_depth += 1;
} else if self.eat(&token::CloseDelim(delim)) {
} else if self.check(&token::CloseDelim(delim)) {
if brace_depth == 0 {
if let ConsumeClosingDelim::Yes = consume_close {
// Some of the callers of this method expect to be able to parse the
// closing delimiter themselves, so we leave it alone. Otherwise we advance
// the parser.
self.bump();
}
return;
} else {
self.bump();
brace_depth -= 1;
continue;
}
Expand Down
17 changes: 10 additions & 7 deletions src/libsyntax/parse/parser/item.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::{Parser, PResult, PathStyle, SemiColonMode, BlockMode};
use super::diagnostics::{Error, dummy_arg};
use super::{Parser, PResult, PathStyle};
use super::diagnostics::{Error, dummy_arg, ConsumeClosingDelim};

use crate::maybe_whole;
use crate::ptr::P;
Expand Down Expand Up @@ -339,7 +339,7 @@ impl<'a> Parser<'a> {
let ident = self.parse_ident().unwrap();
self.bump(); // `(`
let kw_name = self.recover_first_param();
self.consume_block(token::Paren);
self.consume_block(token::Paren, ConsumeClosingDelim::Yes);
let (kw, kw_name, ambiguous) = if self.check(&token::RArrow) {
self.eat_to_tokens(&[&token::OpenDelim(token::Brace)]);
self.bump(); // `{`
Expand All @@ -357,7 +357,7 @@ impl<'a> Parser<'a> {
let msg = format!("missing `{}` for {} definition", kw, kw_name);
let mut err = self.diagnostic().struct_span_err(sp, &msg);
if !ambiguous {
self.consume_block(token::Brace);
self.consume_block(token::Brace, ConsumeClosingDelim::Yes);
let suggestion = format!("add `{}` here to parse `{}` as a public {}",
kw,
ident,
Expand Down Expand Up @@ -672,7 +672,8 @@ impl<'a> Parser<'a> {
Err(mut err) => {
err.emit();
if !at_end {
self.recover_stmt_(SemiColonMode::Break, BlockMode::Break);
self.consume_block(token::Brace, ConsumeClosingDelim::Yes);
break;
}
}
}
Expand Down Expand Up @@ -861,7 +862,8 @@ impl<'a> Parser<'a> {
Err(mut e) => {
e.emit();
if !at_end {
self.recover_stmt_(SemiColonMode::Break, BlockMode::Break);
self.consume_block(token::Brace, ConsumeClosingDelim::Yes);
break;
}
}
}
Expand Down Expand Up @@ -1520,14 +1522,15 @@ impl<'a> Parser<'a> {
if self.eat(&token::OpenDelim(token::Brace)) {
while self.token != token::CloseDelim(token::Brace) {
let field = self.parse_struct_decl_field().map_err(|e| {
self.recover_stmt();
self.consume_block(token::Brace, ConsumeClosingDelim::No);
recovered = true;
e
});
match field {
Ok(field) => fields.push(field),
Err(mut err) => {
err.emit();
break;
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/libsyntax/sess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ pub struct ParseSess {
pub ambiguous_block_expr_parse: Lock<FxHashMap<Span, Span>>,
pub injected_crate_name: Once<Symbol>,
crate gated_spans: GatedSpans,
/// The parser has reached `Eof` due to an unclosed brace. Used to silence unnecessary errors.
pub reached_eof: Lock<bool>,
}

impl ParseSess {
Expand Down Expand Up @@ -101,6 +103,7 @@ impl ParseSess {
ambiguous_block_expr_parse: Lock::new(FxHashMap::default()),
injected_crate_name: Once::new(),
gated_spans: GatedSpans::default(),
reached_eof: Lock::new(false),
}
}

Expand Down
28 changes: 21 additions & 7 deletions src/test/ui/did_you_mean/issue-40006.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,30 @@
impl dyn X { //~ ERROR cannot be made into an object
//~^ ERROR missing
impl dyn A { //~ ERROR missing
Y
}

struct S;

trait X { //~ ERROR missing
X() {}
fn xxx() { ### } //~ ERROR missing
//~^ ERROR expected
L = M; //~ ERROR missing
Z = { 2 + 3 }; //~ ERROR expected one of
fn xxx() { ### }
L = M;
Z = { 2 + 3 };
::Y ();
}

trait A { //~ ERROR missing
X() {}
}
trait B {
fn xxx() { ### } //~ ERROR expected
}
trait C { //~ ERROR missing `fn`, `type`, or `const` for trait-item declaration
L = M;
}
trait D { //~ ERROR missing `fn`, `type`, or `const` for trait-item declaration
Z = { 2 + 3 };
}
trait E {
::Y (); //~ ERROR expected one of
}

Expand All @@ -21,5 +35,5 @@ impl S {
}

fn main() {
S.hello_method();
S.hello_method(); //~ no method named `hello_method` found for type `S` in the current scope
}
Loading

0 comments on commit 6ea6cdc

Please sign in to comment.