Rollup merge of #119841 - nnethercote:rm-DiagnosticBuilder-buffer, r=oli-obk

Remove `DiagnosticBuilder::buffer`

`DiagnosticBuilder::buffer` doesn't do much, and part of what it does (for `-Ztreat-err-as-bug`) it shouldn't.

This PR strips it back, replaces its uses, and finally removes it, making a few cleanups in the vicinity along the way.

r? ``@oli-obk``

Author: matthiaskrgr

compiler/rustc_borrowck/src/lib.rs

@@ -2399,18 +2399,19 @@ mod error {
         /// and we want only the best of those errors.
         ///
         /// The `report_use_of_moved_or_uninitialized` function checks this map and replaces the
-        /// diagnostic (if there is one) if the `Place` of the error being reported is a prefix of the
-        /// `Place` of the previous most diagnostic. This happens instead of buffering the error. Once
-        /// all move errors have been reported, any diagnostics in this map are added to the buffer
-        /// to be emitted.
+        /// diagnostic (if there is one) if the `Place` of the error being reported is a prefix of
+        /// the `Place` of the previous most diagnostic. This happens instead of buffering the
+        /// error. Once all move errors have been reported, any diagnostics in this map are added
+        /// to the buffer to be emitted.
         ///
         /// `BTreeMap` is used to preserve the order of insertions when iterating. This is necessary
         /// when errors in the map are being re-added to the error buffer so that errors with the
         /// same primary span come out in a consistent order.
         buffered_move_errors:
             BTreeMap<Vec<MoveOutIndex>, (PlaceRef<'tcx>, DiagnosticBuilder<'tcx>)>,
         buffered_mut_errors: FxIndexMap<Span, (DiagnosticBuilder<'tcx>, usize)>,
-        /// Diagnostics to be reported buffer.
+        /// Buffer of diagnostics to be reported. Uses `Diagnostic` rather than `DiagnosticBuilder`
+        /// because it has a mixture of error diagnostics and non-error diagnostics.
         buffered: Vec<Diagnostic>,
         /// Set to Some if we emit an error during borrowck
         tainted_by_errors: Option<ErrorGuaranteed>,
@@ -2434,11 +2435,11 @@ mod error {
                     "diagnostic buffered but not emitted",
                 ))
             }
-            t.buffer(&mut self.buffered);
+            self.buffered.push(t.into_diagnostic());
         }
 
         pub fn buffer_non_error_diag(&mut self, t: DiagnosticBuilder<'_, ()>) {
-            t.buffer(&mut self.buffered);
+            self.buffered.push(t.into_diagnostic());
         }
 
         pub fn set_tainted_by_errors(&mut self, e: ErrorGuaranteed) {
@@ -2486,13 +2487,13 @@ mod error {
             // Buffer any move errors that we collected and de-duplicated.
             for (_, (_, diag)) in std::mem::take(&mut self.errors.buffered_move_errors) {
                 // We have already set tainted for this error, so just buffer it.
-                diag.buffer(&mut self.errors.buffered);
+                self.errors.buffered.push(diag.into_diagnostic());
             }
             for (_, (mut diag, count)) in std::mem::take(&mut self.errors.buffered_mut_errors) {
                 if count > 10 {
                     diag.note(format!("...and {} other attempted mutable borrows", count - 10));
                 }
-                diag.buffer(&mut self.errors.buffered);
+                self.errors.buffered.push(diag.into_diagnostic());
             }
 
             if !self.errors.buffered.is_empty() {

compiler/rustc_const_eval/src/transform/check_consts/check.rs

@@ -1,6 +1,6 @@
 //! The `Visitor` responsible for actually checking a `mir::Body` for invalid operations.
 
-use rustc_errors::{Diagnostic, ErrorGuaranteed};
+use rustc_errors::{DiagnosticBuilder, ErrorGuaranteed};
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_index::bit_set::BitSet;
@@ -214,7 +214,7 @@ pub struct Checker<'mir, 'tcx> {
     local_has_storage_dead: Option<BitSet<Local>>,
 
     error_emitted: Option<ErrorGuaranteed>,
-    secondary_errors: Vec<Diagnostic>,
+    secondary_errors: Vec<DiagnosticBuilder<'tcx>>,
 }
 
 impl<'mir, 'tcx> Deref for Checker<'mir, 'tcx> {
@@ -272,14 +272,17 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
         }
 
         // If we got through const-checking without emitting any "primary" errors, emit any
-        // "secondary" errors if they occurred.
+        // "secondary" errors if they occurred. Otherwise, cancel the "secondary" errors.
         let secondary_errors = mem::take(&mut self.secondary_errors);
         if self.error_emitted.is_none() {
             for error in secondary_errors {
-                self.tcx.dcx().emit_diagnostic(error);
+                error.emit();
             }
         } else {
             assert!(self.tcx.dcx().has_errors().is_some());
+            for error in secondary_errors {
+                error.cancel();
+            }
         }
     }
 
@@ -347,7 +350,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
                 self.error_emitted = Some(reported);
             }
 
-            ops::DiagnosticImportance::Secondary => err.buffer(&mut self.secondary_errors),
+            ops::DiagnosticImportance::Secondary => self.secondary_errors.push(err),
         }
     }
 

compiler/rustc_errors/src/diagnostic_builder.rs

@@ -255,35 +255,13 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
     /// Stashes diagnostic for possible later improvement in a different,
     /// later stage of the compiler. The diagnostic can be accessed with
     /// the provided `span` and `key` through [`DiagCtxt::steal_diagnostic()`].
-    ///
-    /// As with `buffer`, this is unless the dcx has disabled such buffering.
     pub fn stash(self, span: Span, key: StashKey) {
-        if let Some((diag, dcx)) = self.into_diagnostic() {
-            dcx.stash_diagnostic(span, key, diag);
-        }
-    }
-
-    /// Converts the builder to a `Diagnostic` for later emission,
-    /// unless dcx has disabled such buffering.
-    fn into_diagnostic(mut self) -> Option<(Diagnostic, &'a DiagCtxt)> {
-        if self.dcx.inner.lock().flags.treat_err_as_bug.is_some() {
-            self.emit();
-            return None;
-        }
-
-        let diag = self.take_diag();
-
-        // Logging here is useful to help track down where in logs an error was
-        // actually emitted.
-        debug!("buffer: diag={:?}", diag);
-
-        Some((diag, self.dcx))
+        self.dcx.stash_diagnostic(span, key, self.into_diagnostic());
     }
 
-    /// Buffers the diagnostic for later emission,
-    /// unless dcx has disabled such buffering.
-    pub fn buffer(self, buffered_diagnostics: &mut Vec<Diagnostic>) {
-        buffered_diagnostics.extend(self.into_diagnostic().map(|(diag, _)| diag));
+    /// Converts the builder to a `Diagnostic` for later emission.
+    pub fn into_diagnostic(mut self) -> Diagnostic {
+        self.take_diag()
     }
 
     /// Delay emission of this diagnostic as a bug.

compiler/rustc_errors/src/lib.rs

@@ -981,6 +981,10 @@ impl DiagCtxt {
 
         inner.emit_stashed_diagnostics();
 
+        if inner.treat_err_as_bug() {
+            return;
+        }
+
         let warnings = match inner.deduplicated_warn_count {
             0 => Cow::from(""),
             1 => Cow::from("1 warning emitted"),
@@ -991,9 +995,6 @@ impl DiagCtxt {
             1 => Cow::from("aborting due to 1 previous error"),
             count => Cow::from(format!("aborting due to {count} previous errors")),
         };
-        if inner.treat_err_as_bug() {
-            return;
-        }
 
         match (errors.len(), warnings.len()) {
             (0, 0) => return,
@@ -1168,7 +1169,8 @@ impl DiagCtxt {
         let mut inner = self.inner.borrow_mut();
 
         if loud && lint_level.is_error() {
-            inner.bump_err_count();
+            inner.err_count += 1;
+            inner.panic_if_treat_err_as_bug();
         }
 
         inner.emitter.emit_unused_externs(lint_level, unused_externs)
@@ -1255,7 +1257,7 @@ impl DiagCtxtInner {
     }
 
     fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option<ErrorGuaranteed> {
-        if matches!(diagnostic.level, Error | Fatal) && self.treat_err_as_bug() {
+        if matches!(diagnostic.level, Error | Fatal) && self.treat_next_err_as_bug() {
             diagnostic.level = Bug;
         }
 
@@ -1353,10 +1355,11 @@ impl DiagCtxtInner {
             }
             if diagnostic.is_error() {
                 if diagnostic.is_lint {
-                    self.bump_lint_err_count();
+                    self.lint_err_count += 1;
                 } else {
-                    self.bump_err_count();
+                    self.err_count += 1;
                 }
+                self.panic_if_treat_err_as_bug();
 
                 #[allow(deprecated)]
                 {
@@ -1447,16 +1450,6 @@ impl DiagCtxtInner {
         panic::panic_any(DelayedBugPanic);
     }
 
-    fn bump_lint_err_count(&mut self) {
-        self.lint_err_count += 1;
-        self.panic_if_treat_err_as_bug();
-    }
-
-    fn bump_err_count(&mut self) {
-        self.err_count += 1;
-        self.panic_if_treat_err_as_bug();
-    }
-
     fn panic_if_treat_err_as_bug(&self) {
         if self.treat_err_as_bug() {
             match (

compiler/rustc_hir_typeck/src/writeback.rs

@@ -498,14 +498,14 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
                     // order when emitting them.
                     let err =
                         self.tcx().dcx().struct_span_err(span, format!("user args: {user_args:?}"));
-                    err.buffer(&mut errors_buffer);
+                    errors_buffer.push(err);
                 }
             }
 
             if !errors_buffer.is_empty() {
                 errors_buffer.sort_by_key(|diag| diag.span.primary_span());
-                for diag in errors_buffer {
-                    self.tcx().dcx().emit_diagnostic(diag);
+                for err in errors_buffer {
+                    err.emit();
                 }
             }
         }

compiler/rustc_interface/src/interface.rs

@@ -82,7 +82,7 @@ pub(crate) fn parse_cfg(dcx: &DiagCtxt, cfgs: Vec<String>) -> Cfg {
                     Ok(..) => {}
                     Err(err) => err.cancel(),
                 },
-                Err(errs) => drop(errs),
+                Err(errs) => errs.into_iter().for_each(|err| err.cancel()),
             }
 
             // If the user tried to use a key="value" flag, but is missing the quotes, provide
@@ -129,9 +129,12 @@ pub(crate) fn parse_check_cfg(dcx: &DiagCtxt, specs: Vec<String>) -> CheckCfg {
             error!("expected `cfg(name, values(\"value1\", \"value2\", ... \"valueN\"))`")
         };
 
-        let Ok(mut parser) = maybe_new_parser_from_source_str(&sess, filename, s.to_string())
-        else {
-            expected_error();
+        let mut parser = match maybe_new_parser_from_source_str(&sess, filename, s.to_string()) {
+            Ok(parser) => parser,
+            Err(errs) => {
+                errs.into_iter().for_each(|err| err.cancel());
+                expected_error();
+            }
         };
 
         let meta_item = match parser.parse_meta_item() {

compiler/rustc_parse/src/lexer/mod.rs

@@ -7,7 +7,7 @@ use rustc_ast::ast::{self, AttrStyle};
 use rustc_ast::token::{self, CommentKind, Delimiter, Token, TokenKind};
 use rustc_ast::tokenstream::TokenStream;
 use rustc_ast::util::unicode::contains_text_flow_control_chars;
-use rustc_errors::{error_code, Applicability, DiagCtxt, Diagnostic, StashKey};
+use rustc_errors::{error_code, Applicability, DiagCtxt, DiagnosticBuilder, StashKey};
 use rustc_lexer::unescape::{self, EscapeError, Mode};
 use rustc_lexer::{Base, DocStyle, RawStrError};
 use rustc_lexer::{Cursor, LiteralKind};
@@ -42,12 +42,12 @@ pub struct UnmatchedDelim {
     pub candidate_span: Option<Span>,
 }
 
-pub(crate) fn parse_token_trees<'a>(
-    sess: &'a ParseSess,
-    mut src: &'a str,
+pub(crate) fn parse_token_trees<'sess, 'src>(
+    sess: &'sess ParseSess,
+    mut src: &'src str,
     mut start_pos: BytePos,
     override_span: Option<Span>,
-) -> Result<TokenStream, Vec<Diagnostic>> {
+) -> Result<TokenStream, Vec<DiagnosticBuilder<'sess>>> {
     // Skip `#!`, if present.
     if let Some(shebang_len) = rustc_lexer::strip_shebang(src) {
         src = &src[shebang_len..];
@@ -76,39 +76,39 @@ pub(crate) fn parse_token_trees<'a>(
             let mut buffer = Vec::with_capacity(1);
             for unmatched in unmatched_delims {
                 if let Some(err) = make_unclosed_delims_error(unmatched, sess) {
-                    err.buffer(&mut buffer);
+                    buffer.push(err);
                 }
             }
             if let Err(errs) = res {
                 // Add unclosing delimiter or diff marker errors
                 for err in errs {
-                    err.buffer(&mut buffer);
+                    buffer.push(err);
                 }
             }
             Err(buffer)
         }
     }
 }
 
-struct StringReader<'a> {
-    sess: &'a ParseSess,
+struct StringReader<'sess, 'src> {
+    sess: &'sess ParseSess,
     /// Initial position, read-only.
     start_pos: BytePos,
     /// The absolute offset within the source_map of the current character.
     pos: BytePos,
     /// Source text to tokenize.
-    src: &'a str,
+    src: &'src str,
     /// Cursor for getting lexer tokens.
-    cursor: Cursor<'a>,
+    cursor: Cursor<'src>,
     override_span: Option<Span>,
     /// When a "unknown start of token: \u{a0}" has already been emitted earlier
     /// in this file, it's safe to treat further occurrences of the non-breaking
     /// space character as whitespace.
     nbsp_is_whitespace: bool,
 }
 
-impl<'a> StringReader<'a> {
-    pub fn dcx(&self) -> &'a DiagCtxt {
+impl<'sess, 'src> StringReader<'sess, 'src> {
+    pub fn dcx(&self) -> &'sess DiagCtxt {
         &self.sess.dcx
     }
 
@@ -526,7 +526,7 @@ impl<'a> StringReader<'a> {
 
     /// Slice of the source text from `start` up to but excluding `self.pos`,
     /// meaning the slice does not include the character `self.ch`.
-    fn str_from(&self, start: BytePos) -> &'a str {
+    fn str_from(&self, start: BytePos) -> &'src str {
         self.str_from_to(start, self.pos)
     }
 
@@ -537,12 +537,12 @@ impl<'a> StringReader<'a> {
     }
 
     /// Slice of the source text spanning from `start` up to but excluding `end`.
-    fn str_from_to(&self, start: BytePos, end: BytePos) -> &'a str {
+    fn str_from_to(&self, start: BytePos, end: BytePos) -> &'src str {
         &self.src[self.src_index(start)..self.src_index(end)]
     }
 
     /// Slice of the source text spanning from `start` until the end
-    fn str_from_to_end(&self, start: BytePos) -> &'a str {
+    fn str_from_to_end(&self, start: BytePos) -> &'src str {
         &self.src[self.src_index(start)..]
     }