Skip to content

Commit 053a095

Browse files
committed
Reduce ammount 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 more granularly 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.
1 parent d3d28a4 commit 053a095

17 files changed

+302
-96
lines changed

src/libsyntax/parse/lexer/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ mod unicode_chars;
2323
#[derive(Clone, Debug)]
2424
pub struct UnmatchedBrace {
2525
pub expected_delim: token::DelimToken,
26-
pub found_delim: token::DelimToken,
26+
pub found_delim: Option<token::DelimToken>,
2727
pub found_span: Span,
2828
pub unclosed_span: Option<Span>,
2929
pub candidate_span: Option<Span>,

src/libsyntax/parse/lexer/tokentrees.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ impl<'a> TokenTreesReader<'a> {
8080
.struct_span_err(self.token.span, msg);
8181
for &(_, sp) in &self.open_braces {
8282
err.span_label(sp, "un-closed delimiter");
83+
self.unmatched_braces.push(UnmatchedBrace {
84+
expected_delim: token::DelimToken::Brace,
85+
found_delim: None,
86+
found_span: self.token.span,
87+
unclosed_span: Some(sp),
88+
candidate_span: None,
89+
});
8390
}
8491

8592
if let Some((delim, _)) = self.open_braces.last() {
@@ -170,7 +177,7 @@ impl<'a> TokenTreesReader<'a> {
170177
let (tok, _) = self.open_braces.pop().unwrap();
171178
self.unmatched_braces.push(UnmatchedBrace {
172179
expected_delim: tok,
173-
found_delim: other,
180+
found_delim: Some(other),
174181
found_span: self.token.span,
175182
unclosed_span: unclosed_delimiter,
176183
candidate_span: candidate,

src/libsyntax/parse/mod.rs

+3-12
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! The main parser interface.
22
33
use crate::ast;
4-
use crate::parse::parser::{Parser, emit_unclosed_delims};
4+
use crate::parse::parser::{Parser, emit_unclosed_delims, make_unclosed_delims_error};
55
use crate::parse::token::Nonterminal;
66
use crate::tokenstream::{self, TokenStream, TokenTree};
77
use crate::print::pprust;
@@ -242,18 +242,9 @@ pub fn maybe_file_to_stream(
242242
err.buffer(&mut buffer);
243243
// Not using `emit_unclosed_delims` to use `db.buffer`
244244
for unmatched in unmatched_braces {
245-
let mut db = sess.span_diagnostic.struct_span_err(unmatched.found_span, &format!(
246-
"incorrect close delimiter: `{}`",
247-
pprust::token_kind_to_string(&token::CloseDelim(unmatched.found_delim)),
248-
));
249-
db.span_label(unmatched.found_span, "incorrect close delimiter");
250-
if let Some(sp) = unmatched.candidate_span {
251-
db.span_label(sp, "close delimiter possibly meant for this");
245+
if let Some(err) = make_unclosed_delims_error(unmatched, &sess.span_diagnostic) {
246+
err.buffer(&mut buffer);
252247
}
253-
if let Some(sp) = unmatched.unclosed_span {
254-
db.span_label(sp, "un-closed delimiter");
255-
}
256-
db.buffer(&mut buffer);
257248
}
258249
Err(buffer)
259250
}

src/libsyntax/parse/parser.rs

+24-15
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use crate::symbol::{kw, sym, Symbol};
2727
use crate::tokenstream::{self, DelimSpan, TokenTree, TokenStream, TreeAndJoint};
2828
use crate::ThinVec;
2929

30-
use errors::{Applicability, DiagnosticId, FatalError};
30+
use errors::{Applicability, DiagnosticBuilder, DiagnosticId, FatalError};
3131
use rustc_target::spec::abi::{self, Abi};
3232
use syntax_pos::{Span, BytePos, DUMMY_SP, FileName};
3333
use log::debug;
@@ -1370,20 +1370,29 @@ impl<'a> Parser<'a> {
13701370
}
13711371
}
13721372

1373+
crate fn make_unclosed_delims_error(
1374+
unmatched: UnmatchedBrace,
1375+
handler: &errors::Handler,
1376+
) -> Option<DiagnosticBuilder<'_>> {
1377+
// `None` here means an `Eof` was found. We already emit those errors elsewhere, we add them to
1378+
// `unmatched_braces` only for error recovery in the `Parser`.
1379+
let found_delim = unmatched.found_delim?;
1380+
let mut err = handler.struct_span_err(unmatched.found_span, &format!(
1381+
"incorrect close delimiter: `{}`",
1382+
pprust::token_kind_to_string(&token::CloseDelim(found_delim)),
1383+
));
1384+
err.span_label(unmatched.found_span, "incorrect close delimiter");
1385+
if let Some(sp) = unmatched.candidate_span {
1386+
err.span_label(sp, "close delimiter possibly meant for this");
1387+
}
1388+
if let Some(sp) = unmatched.unclosed_span {
1389+
err.span_label(sp, "un-closed delimiter");
1390+
}
1391+
Some(err)
1392+
}
1393+
13731394
pub fn emit_unclosed_delims(unclosed_delims: &mut Vec<UnmatchedBrace>, handler: &errors::Handler) {
1374-
for unmatched in unclosed_delims.iter() {
1375-
let mut err = handler.struct_span_err(unmatched.found_span, &format!(
1376-
"incorrect close delimiter: `{}`",
1377-
pprust::token_kind_to_string(&token::CloseDelim(unmatched.found_delim)),
1378-
));
1379-
err.span_label(unmatched.found_span, "incorrect close delimiter");
1380-
if let Some(sp) = unmatched.candidate_span {
1381-
err.span_label(sp, "close delimiter possibly meant for this");
1382-
}
1383-
if let Some(sp) = unmatched.unclosed_span {
1384-
err.span_label(sp, "un-closed delimiter");
1385-
}
1386-
err.emit();
1395+
for unmatched in unclosed_delims.drain(..) {
1396+
make_unclosed_delims_error(unmatched, handler).map(|mut e| e.emit());
13871397
}
1388-
unclosed_delims.clear();
13891398
}

src/libsyntax/parse/parser/diagnostics.rs

+37-8
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,12 @@ impl RecoverQPath for Expr {
171171
}
172172
}
173173

174+
/// Control whether the closing delimiter should be consumed when calling `Parser::consume_block`.
175+
crate enum ConsumeClosingDelim {
176+
Yes,
177+
No,
178+
}
179+
174180
impl<'a> Parser<'a> {
175181
pub fn fatal(&self, m: &str) -> DiagnosticBuilder<'a> {
176182
self.span_fatal(self.token.span, m)
@@ -1105,8 +1111,8 @@ impl<'a> Parser<'a> {
11051111
Ok(x) => x,
11061112
Err(mut err) => {
11071113
err.emit();
1108-
// Recover from parse error.
1109-
self.consume_block(delim);
1114+
// Recover from parse error, callers expect the closing delim to be consumed.
1115+
self.consume_block(delim, ConsumeClosingDelim::Yes);
11101116
self.mk_expr(lo.to(self.prev_span), ExprKind::Err, ThinVec::new())
11111117
}
11121118
}
@@ -1154,17 +1160,29 @@ impl<'a> Parser<'a> {
11541160
delim.to_string(),
11551161
Applicability::MaybeIncorrect,
11561162
);
1157-
err.emit();
1158-
self.expected_tokens.clear(); // reduce errors
1159-
Ok(true)
1163+
if unmatched.found_delim.is_none() {
1164+
// Encountered `Eof` when lexing blocks. Do not recover here to avoid knockdown
1165+
// errors which would be emitted elsewhere in the parser and let other error
1166+
// recovery consume the rest of the file.
1167+
Err(err)
1168+
} else {
1169+
err.emit();
1170+
self.expected_tokens.clear(); // Reduce the number of errors.
1171+
Ok(true)
1172+
}
11601173
}
11611174
_ => Err(err),
11621175
}
11631176
}
11641177

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

1425-
pub(super) fn consume_block(&mut self, delim: token::DelimToken) {
1443+
pub(super) fn consume_block(
1444+
&mut self,
1445+
delim: token::DelimToken,
1446+
consume_close: ConsumeClosingDelim,
1447+
) {
14261448
let mut brace_depth = 0;
14271449
loop {
14281450
if self.eat(&token::OpenDelim(delim)) {
14291451
brace_depth += 1;
1430-
} else if self.eat(&token::CloseDelim(delim)) {
1452+
} else if self.check(&token::CloseDelim(delim)) {
14311453
if brace_depth == 0 {
1454+
if let ConsumeClosingDelim::Yes = consume_close {
1455+
// Some of the callers of this method expect to be able to parse the
1456+
// closing delimiter themselves, so we leave it alone. Otherwise we advance
1457+
// the parser.
1458+
self.bump();
1459+
}
14321460
return;
14331461
} else {
1462+
self.bump();
14341463
brace_depth -= 1;
14351464
continue;
14361465
}

src/libsyntax/parse/parser/item.rs

+10-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
use super::{Parser, PResult, PathStyle, SemiColonMode, BlockMode};
2-
use super::diagnostics::{Error, dummy_arg};
1+
use super::{Parser, PResult, PathStyle};
2+
use super::diagnostics::{Error, dummy_arg, ConsumeClosingDelim};
33

44
use crate::maybe_whole;
55
use crate::ptr::P;
@@ -339,7 +339,7 @@ impl<'a> Parser<'a> {
339339
let ident = self.parse_ident().unwrap();
340340
self.bump(); // `(`
341341
let kw_name = self.recover_first_param();
342-
self.consume_block(token::Paren);
342+
self.consume_block(token::Paren, ConsumeClosingDelim::Yes);
343343
let (kw, kw_name, ambiguous) = if self.check(&token::RArrow) {
344344
self.eat_to_tokens(&[&token::OpenDelim(token::Brace)]);
345345
self.bump(); // `{`
@@ -357,7 +357,7 @@ impl<'a> Parser<'a> {
357357
let msg = format!("missing `{}` for {} definition", kw, kw_name);
358358
let mut err = self.diagnostic().struct_span_err(sp, &msg);
359359
if !ambiguous {
360-
self.consume_block(token::Brace);
360+
self.consume_block(token::Brace, ConsumeClosingDelim::Yes);
361361
let suggestion = format!("add `{}` here to parse `{}` as a public {}",
362362
kw,
363363
ident,
@@ -672,7 +672,8 @@ impl<'a> Parser<'a> {
672672
Err(mut err) => {
673673
err.emit();
674674
if !at_end {
675-
self.recover_stmt_(SemiColonMode::Break, BlockMode::Break);
675+
self.consume_block(token::Brace, ConsumeClosingDelim::Yes);
676+
break;
676677
}
677678
}
678679
}
@@ -861,7 +862,8 @@ impl<'a> Parser<'a> {
861862
Err(mut e) => {
862863
e.emit();
863864
if !at_end {
864-
self.recover_stmt_(SemiColonMode::Break, BlockMode::Break);
865+
self.consume_block(token::Brace, ConsumeClosingDelim::Yes);
866+
break;
865867
}
866868
}
867869
}
@@ -1520,14 +1522,15 @@ impl<'a> Parser<'a> {
15201522
if self.eat(&token::OpenDelim(token::Brace)) {
15211523
while self.token != token::CloseDelim(token::Brace) {
15221524
let field = self.parse_struct_decl_field().map_err(|e| {
1523-
self.recover_stmt();
1525+
self.consume_block(token::Brace, ConsumeClosingDelim::No);
15241526
recovered = true;
15251527
e
15261528
});
15271529
match field {
15281530
Ok(field) => fields.push(field),
15291531
Err(mut err) => {
15301532
err.emit();
1533+
break;
15311534
}
15321535
}
15331536
}
+21-7
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,30 @@
1-
impl dyn X { //~ ERROR cannot be made into an object
2-
//~^ ERROR missing
1+
impl dyn A { //~ ERROR missing
32
Y
43
}
54

65
struct S;
76

87
trait X { //~ ERROR missing
98
X() {}
10-
fn xxx() { ### } //~ ERROR missing
11-
//~^ ERROR expected
12-
L = M; //~ ERROR missing
13-
Z = { 2 + 3 }; //~ ERROR expected one of
9+
fn xxx() { ### }
10+
L = M;
11+
Z = { 2 + 3 };
12+
::Y ();
13+
}
14+
15+
trait A { //~ ERROR missing
16+
X() {}
17+
}
18+
trait B {
19+
fn xxx() { ### } //~ ERROR expected
20+
}
21+
trait C { //~ ERROR missing `fn`, `type`, or `const` for trait-item declaration
22+
L = M;
23+
}
24+
trait D { //~ ERROR missing `fn`, `type`, or `const` for trait-item declaration
25+
Z = { 2 + 3 };
26+
}
27+
trait E {
1428
::Y (); //~ ERROR expected one of
1529
}
1630

@@ -21,5 +35,5 @@ impl S {
2135
}
2236

2337
fn main() {
24-
S.hello_method();
38+
S.hello_method(); //~ no method named `hello_method` found for type `S` in the current scope
2539
}

0 commit comments

Comments
 (0)