rustc_errors: add `downgrade_to_delayed_bug` to `Diagnostic` itself.

4 files changed, 108 insertions, 30 deletions

diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
index 9db8f751390..2835afb0208 100644
--- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
+++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
@@ -66,7 +66,9 @@ fn source_string(file: Lrc<SourceFile>, line: &Line) -> String {
 /// Maps `Diagnostic::Level` to `snippet::AnnotationType`
 fn annotation_type_for_level(level: Level) -> AnnotationType {
     match level {
-        Level::Bug | Level::Fatal | Level::Error { .. } => AnnotationType::Error,
+        Level::Bug | Level::DelayedBug | Level::Fatal | Level::Error { .. } => {
+            AnnotationType::Error
+        }
         Level::Warning => AnnotationType::Warning,
         Level::Note => AnnotationType::Note,
         Level::Help => AnnotationType::Help,
diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs
index 8cfecafd20c..f62a3567c56 100644
--- a/compiler/rustc_errors/src/diagnostic.rs
+++ b/compiler/rustc_errors/src/diagnostic.rs
@@ -19,7 +19,10 @@ pub struct SuggestionsDisabled;
 #[must_use]
 #[derive(Clone, Debug, Encodable, Decodable)]
 pub struct Diagnostic {
-    pub level: Level,
+    // NOTE(eddyb) this is private to disallow arbitrary after-the-fact changes,
+    // outside of what methods in this crate themselves allow.
+    crate level: Level,
+
     pub message: Vec<(String, Style)>,
     pub code: Option<DiagnosticId>,
     pub span: MultiSpan,
@@ -117,9 +120,18 @@ impl Diagnostic {
         }
     }
 
+    #[inline(always)]
+    pub fn level(&self) -> Level {
+        self.level
+    }
+
     pub fn is_error(&self) -> bool {
         match self.level {
-            Level::Bug | Level::Fatal | Level::Error { .. } | Level::FailureNote => true,
+            Level::Bug
+            | Level::DelayedBug
+            | Level::Fatal
+            | Level::Error { .. }
+            | Level::FailureNote => true,
 
             Level::Warning | Level::Note | Level::Help | Level::Cancelled | Level::Allow => false,
         }
@@ -150,6 +162,33 @@ impl Diagnostic {
         self.level == Level::Cancelled
     }
 
+    /// Delay emission of this diagnostic as a bug.
+    ///
+    /// This can be useful in contexts where an error indicates a bug but
+    /// typically this only happens when other compilation errors have already
+    /// happened. In those cases this can be used to defer emission of this
+    /// diagnostic as a bug in the compiler only if no other errors have been
+    /// emitted.
+    ///
+    /// In the meantime, though, callsites are required to deal with the "bug"
+    /// locally in whichever way makes the most sense.
+    #[track_caller]
+    pub fn downgrade_to_delayed_bug(&mut self) -> &mut Self {
+        // FIXME(eddyb) this check is only necessary because cancellation exists,
+        // but hopefully that can be removed in the future, if enough callers
+        // of `.cancel()` can take `DiagnosticBuilder`, and by-value.
+        if !self.cancelled() {
+            assert!(
+                self.is_error(),
+                "downgrade_to_delayed_bug: cannot downgrade {:?} to DelayedBug: not an error",
+                self.level
+            );
+            self.level = Level::DelayedBug;
+        }
+
+        self
+    }
+
     /// Adds a span/label to be included in the resulting snippet.
     ///
     /// This is pushed onto the [`MultiSpan`] that was created when the diagnostic
diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs
index 3c8751a7a35..b4189cbfc62 100644
--- a/compiler/rustc_errors/src/diagnostic_builder.rs
+++ b/compiler/rustc_errors/src/diagnostic_builder.rs
@@ -105,10 +105,9 @@ impl<'a> DiagnosticBuilder<'a> {
     /// See `emit` and `delay_as_bug` for details.
     pub fn emit_unless(&mut self, delay: bool) {
         if delay {
-            self.delay_as_bug();
-        } else {
-            self.emit();
+            self.downgrade_to_delayed_bug();
         }
+        self.emit();
     }
 
     /// Stashes diagnostic for possible later improvement in a different,
@@ -162,12 +161,17 @@ impl<'a> DiagnosticBuilder<'a> {
     ///
     /// In the meantime, though, callsites are required to deal with the "bug"
     /// locally in whichever way makes the most sense.
+    #[track_caller]
     pub fn delay_as_bug(&mut self) {
-        self.level = Level::Bug;
-        self.handler.delay_as_bug((*self.diagnostic).clone());
-        self.cancel();
+        self.downgrade_to_delayed_bug();
+        self.emit();
     }
 
+    forward!(
+        #[track_caller]
+        pub fn downgrade_to_delayed_bug(&mut self,) -> &mut Self
+    );
+
     /// Appends a labeled span to the diagnostic.
     ///
     /// Labels are used to convey additional context for the diagnostic's primary span. They will
diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index a5c954cca13..539ddcec332 100644
--- a/compiler/rustc_errors/src/lib.rs
+++ b/compiler/rustc_errors/src/lib.rs
@@ -491,10 +491,15 @@ impl Drop for HandlerInner {
             self.flush_delayed(bugs, "no errors encountered even though `delay_span_bug` issued");
         }
 
+        // FIXME(eddyb) this explains what `delayed_good_path_bugs` are!
+        // They're `delayed_span_bugs` but for "require some diagnostic happened"
+        // instead of "require some error happened". Sadly that isn't ideal, as
+        // lints can be `#[allow]`'d, potentially leading to this triggering.
+        // Also, "good path" should be replaced with a better naming.
         if !self.has_any_message() {
             let bugs = std::mem::replace(&mut self.delayed_good_path_bugs, Vec::new());
             self.flush_delayed(
-                bugs.into_iter().map(DelayedDiagnostic::decorate).collect(),
+                bugs.into_iter().map(DelayedDiagnostic::decorate),
                 "no warnings or errors encountered even though `delayed_good_path_bugs` issued",
             );
         }
@@ -815,6 +820,8 @@ impl Handler {
         self.inner.borrow_mut().delay_span_bug(span, msg)
     }
 
+    // FIXME(eddyb) note the comment inside `impl Drop for HandlerInner`, that's
+    // where the explanation of what "good path" is (also, it should be renamed).
     pub fn delay_good_path_bug(&self, msg: &str) {
         self.inner.borrow_mut().delay_good_path_bug(msg)
     }
@@ -915,10 +922,6 @@ impl Handler {
     pub fn emit_unused_externs(&self, lint_level: &str, unused_externs: &[&str]) {
         self.inner.borrow_mut().emit_unused_externs(lint_level, unused_externs)
     }
-
-    pub fn delay_as_bug(&self, diagnostic: Diagnostic) {
-        self.inner.borrow_mut().delay_as_bug(diagnostic)
-    }
 }
 
 impl HandlerInner {
@@ -936,11 +939,24 @@ impl HandlerInner {
         diags.iter().for_each(|diag| self.emit_diagnostic(diag));
     }
 
+    // FIXME(eddyb) this should ideally take `diagnostic` by value.
     fn emit_diagnostic(&mut self, diagnostic: &Diagnostic) {
         if diagnostic.cancelled() {
             return;
         }
 
+        if diagnostic.level == Level::DelayedBug {
+            // FIXME(eddyb) this should check for `has_errors` and stop pushing
+            // once *any* errors were emitted (and truncate `delayed_span_bugs`
+            // when an error is first emitted, also), but maybe there's a case
+            // in which that's not sound? otherwise this is really inefficient.
+            self.delayed_span_bugs.push(diagnostic.clone());
+
+            if !self.flags.report_delayed_bugs {
+                return;
+            }
+        }
+
         if diagnostic.has_future_breakage() {
             self.future_breakage_diagnostics.push(diagnostic.clone());
         }
@@ -1119,14 +1135,16 @@ impl HandlerInner {
             // FIXME: don't abort here if report_delayed_bugs is off
             self.span_bug(sp, msg);
         }
-        let mut diagnostic = Diagnostic::new(Level::Bug, msg);
+        let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg);
         diagnostic.set_span(sp.into());
         diagnostic.note(&format!("delayed at {}", std::panic::Location::caller()));
-        self.delay_as_bug(diagnostic)
+        self.emit_diagnostic(&diagnostic)
     }
 
+    // FIXME(eddyb) note the comment inside `impl Drop for HandlerInner`, that's
+    // where the explanation of what "good path" is (also, it should be renamed).
     fn delay_good_path_bug(&mut self, msg: &str) {
-        let diagnostic = Diagnostic::new(Level::Bug, msg);
+        let diagnostic = Diagnostic::new(Level::DelayedBug, msg);
         if self.flags.report_delayed_bugs {
             self.emit_diagnostic(&diagnostic);
         }
@@ -1160,20 +1178,34 @@ impl HandlerInner {
         panic::panic_any(ExplicitBug);
     }
 
-    fn delay_as_bug(&mut self, diagnostic: Diagnostic) {
-        if self.flags.report_delayed_bugs {
-            self.emit_diagnostic(&diagnostic);
-        }
-        self.delayed_span_bugs.push(diagnostic);
-    }
+    fn flush_delayed(&mut self, bugs: impl IntoIterator<Item = Diagnostic>, explanation: &str) {
+        let mut no_bugs = true;
+        for mut bug in bugs {
+            if no_bugs {
+                // Put the overall explanation before the `DelayedBug`s, to
+                // frame them better (e.g. separate warnings from them).
+                self.emit_diagnostic(&Diagnostic::new(Bug, explanation));
+                no_bugs = false;
+            }
+
+            // "Undelay" the `DelayedBug`s (into plain `Bug`s).
+            if bug.level != Level::DelayedBug {
+                // NOTE(eddyb) not panicking here because we're already producing
+                // an ICE, and the more information the merrier.
+                bug.note(&format!(
+                    "`flushed_delayed` got diagnostic with level {:?}, \
+                     instead of the expected `DelayedBug`",
+                    bug.level,
+                ));
+            }
+            bug.level = Level::Bug;
 
-    fn flush_delayed(&mut self, bugs: Vec<Diagnostic>, explanation: &str) {
-        let has_bugs = !bugs.is_empty();
-        for bug in bugs {
             self.emit_diagnostic(&bug);
         }
-        if has_bugs {
-            panic!("{}", explanation);
+
+        // Panic with `ExplicitBug` to avoid "unexpected panic" messages.
+        if !no_bugs {
+            panic::panic_any(ExplicitBug);
         }
     }
 
@@ -1227,6 +1259,7 @@ impl DelayedDiagnostic {
 #[derive(Copy, PartialEq, Clone, Hash, Debug, Encodable, Decodable)]
 pub enum Level {
     Bug,
+    DelayedBug,
     Fatal,
     Error {
         /// If this error comes from a lint, don't abort compilation even when abort_if_errors() is called.
@@ -1250,7 +1283,7 @@ impl Level {
     fn color(self) -> ColorSpec {
         let mut spec = ColorSpec::new();
         match self {
-            Bug | Fatal | Error { .. } => {
+            Bug | DelayedBug | Fatal | Error { .. } => {
                 spec.set_fg(Some(Color::Red)).set_intense(true);
             }
             Warning => {
@@ -1270,7 +1303,7 @@ impl Level {
 
     pub fn to_str(self) -> &'static str {
         match self {
-            Bug => "error: internal compiler error",
+            Bug | DelayedBug => "error: internal compiler error",
             Fatal | Error { .. } => "error",
             Warning => "warning",
             Note => "note",