Remove a fourth DiagnosticBuilder::emit_without_consuming call.

nnethercote · nnethercote · commit c733a0216d55 · 2024-01-08T16:04:50.000+11:00
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&lt;ErrorGuaranteed&gt;`. 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.
diff --git 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<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
@@ -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
@@ -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);
                 }
