Skip to content

Commit

Permalink
Remove a fourth DiagnosticBuilder::emit_without_consuming call.
Browse files Browse the repository at this point in the history
The old code was very hard to understand, involving an
`emit_without_consuming` call *and* a `delay_as_bug_without_consuming`
call.

With slight changes both calls can be avoided. Not creating the error
until later is crucial, as is the early return in the `if recovered`
block.

It took me some time to come up with this reworking -- it went through
intermediate states much further from the original code than this final
version -- and it's isn't obvious at a glance that it is equivalent. But
I think it is, and the unchanged test behaviour is good supporting
evidence.

The commit also changes `check_trailing_angle_brackets` to return
`Option<ErrorGuaranteed>`. This provides a stricter proof that it
emitted an error message than asserting `dcx.has_errors().is_some()`,
which would succeed if any error had previously been emitted anywhere.
  • Loading branch information
nnethercote committed Jan 8, 2024
1 parent 1b6c8e7 commit c733a02
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 31 deletions.
17 changes: 8 additions & 9 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ use rustc_ast::{
use rustc_ast_pretty::pprust;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{
pluralize, AddToDiagnostic, Applicability, DiagCtxt, Diagnostic, DiagnosticBuilder, FatalError,
PErr, PResult,
pluralize, AddToDiagnostic, Applicability, DiagCtxt, Diagnostic, DiagnosticBuilder,
ErrorGuaranteed, FatalError, PErr, PResult,
};
use rustc_session::errors::ExprParenthesesNeeded;
use rustc_span::source_map::Spanned;
Expand Down Expand Up @@ -1049,9 +1049,9 @@ impl<'a> Parser<'a> {
&mut self,
segment: &PathSegment,
end: &[&TokenKind],
) -> bool {
) -> Option<ErrorGuaranteed> {
if !self.may_recover() {
return false;
return None;
}

// This function is intended to be invoked after parsing a path segment where there are two
Expand Down Expand Up @@ -1086,7 +1086,7 @@ impl<'a> Parser<'a> {
parsed_angle_bracket_args,
);
if !parsed_angle_bracket_args {
return false;
return None;
}

// Keep the span at the start so we can highlight the sequence of `>` characters to be
Expand Down Expand Up @@ -1124,7 +1124,7 @@ impl<'a> Parser<'a> {
number_of_gt, number_of_shr,
);
if number_of_gt < 1 && number_of_shr < 1 {
return false;
return None;
}

// Finally, double check that we have our end token as otherwise this is the
Expand All @@ -1139,10 +1139,9 @@ impl<'a> Parser<'a> {
let span = lo.until(self.token.span);

let num_extra_brackets = number_of_gt + number_of_shr * 2;
self.dcx().emit_err(UnmatchedAngleBrackets { span, num_extra_brackets });
return true;
return Some(self.dcx().emit_err(UnmatchedAngleBrackets { span, num_extra_brackets }));
}
false
None
}

/// Check if a method call with an intended turbofish has been written without surrounding
Expand Down
35 changes: 13 additions & 22 deletions compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1793,52 +1793,43 @@ impl<'a> Parser<'a> {
}
_ => {
let sp = self.prev_token.span.shrink_to_hi();
let mut err = self.dcx().struct_span_err(
sp,
format!("expected `,`, or `}}`, found {}", super::token_descr(&self.token)),
);
let msg =
format!("expected `,`, or `}}`, found {}", super::token_descr(&self.token));

// Try to recover extra trailing angle brackets
let mut recovered = false;
if let TyKind::Path(_, Path { segments, .. }) = &a_var.ty.kind {
if let Some(last_segment) = segments.last() {
recovered = self.check_trailing_angle_brackets(
let guar = self.check_trailing_angle_brackets(
last_segment,
&[&token::Comma, &token::CloseDelim(Delimiter::Brace)],
);
if recovered {
if let Some(_guar) = guar {
// Handle a case like `Vec<u8>>,` where we can continue parsing fields
// after the comma
self.eat(&token::Comma);
// `check_trailing_angle_brackets` already emitted a nicer error
// NOTE(eddyb) this was `.cancel()`, but `err`
// gets returned, so we can't fully defuse it.
err.delay_as_bug_without_consuming();

// `check_trailing_angle_brackets` already emitted a nicer error, as
// proven by the presence of `_guar`. We can continue parsing.
return Ok(a_var);
}
}
}

let mut err = self.dcx().struct_span_err(sp, msg);

if self.token.is_ident()
|| (self.token.kind == TokenKind::Pound
&& (self.look_ahead(1, |t| t == &token::OpenDelim(Delimiter::Bracket))))
{
// This is likely another field, TokenKind::Pound is used for `#[..]` attribute for next field,
// emit the diagnostic and keep going
// This is likely another field, TokenKind::Pound is used for `#[..]`
// attribute for next field. Emit the diagnostic and continue parsing.
err.span_suggestion(
sp,
"try adding a comma",
",",
Applicability::MachineApplicable,
);
err.emit_without_consuming();
recovered = true;
}

if recovered {
// Make sure an error was emitted (either by recovering an angle bracket,
// or by finding an identifier as the next token), since we're
// going to continue parsing
assert!(self.dcx().has_errors().is_some());
err.emit();
} else {
return Err(err);
}
Expand Down

0 comments on commit c733a02

Please sign in to comment.