From c733a0216d55977e51a26ead6f2446668f006e02 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 5 Jan 2024 11:10:18 +1100 Subject: [PATCH] Remove a fourth `DiagnosticBuilder::emit_without_consuming` call. 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`. 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. --- .../rustc_parse/src/parser/diagnostics.rs | 17 +++++---- compiler/rustc_parse/src/parser/item.rs | 35 +++++++------------ 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 776d0ace875..a9cf26d991c 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -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; @@ -1049,9 +1049,9 @@ impl<'a> Parser<'a> { &mut self, segment: &PathSegment, end: &[&TokenKind], - ) -> bool { + ) -> Option { if !self.may_recover() { - return false; + return None; } // This function is intended to be invoked after parsing a path segment where there are two @@ -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 @@ -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 @@ -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 diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 381ea21a244..fed3c83a81d 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -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>,` 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); }