Split Level::DelayedBug in two.

The two kinds of delayed bug have quite different semantics so a
stronger conceptual separation is nice. (`is_error` is a good example,
because the two kinds have different behaviour.)

The commit also moves the `DelayedBug` variant after `Error` in `Level`,
to reflect the fact that it's weaker than `Error` -- it might trigger an
error but also might not. (The pre-existing `downgrade_to_delayed_bug`
function also reflects the notion that delayed bugs are lower/after
normal errors.)

Plus it condenses some of the comments on `Level` into a table, for
easier reading, and introduces `can_be_top_or_sub` to indicate which
levels can be used in top-level diagnostics vs. subdiagnostics.

Finally, it renames `DiagCtxtInner::span_delayed_bugs` as
`DiagCtxtInner::delayed_bugs`. The `span_` prefix is unnecessary because
some delayed bugs don't have a span.

compiler/rustc_error_messages/src/lib.rs

@@ -378,7 +378,7 @@ impl From<Cow<'static, str>> for DiagnosticMessage {
     }
 }
 
-/// A workaround for "good path" ICEs when formatting types in disabled lints.
+/// A workaround for good_path_delayed_bug ICEs when formatting types in disabled lints.
 ///
 /// Delays formatting until `.into(): DiagnosticMessage` is used.
 pub struct DelayDm<F>(pub F);

compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs

@@ -85,7 +85,11 @@ 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::DelayedBug(_) | Level::Fatal | Level::Error => AnnotationType::Error,
+        Level::Bug
+        | Level::Fatal
+        | Level::Error
+        | Level::DelayedBug
+        | Level::GoodPathDelayedBug => AnnotationType::Error,
         Level::ForceWarning(_) | Level::Warning => AnnotationType::Warning,
         Level::Note | Level::OnceNote => AnnotationType::Note,
         Level::Help | Level::OnceHelp => AnnotationType::Help,

compiler/rustc_errors/src/diagnostic.rs

@@ -1,8 +1,7 @@
 use crate::snippet::Style;
 use crate::{
-    CodeSuggestion, DelayedBugKind, DiagnosticBuilder, DiagnosticMessage, EmissionGuarantee,
+    CodeSuggestion, DiagnosticBuilder, DiagnosticMessage, EmissionGuarantee, ErrCode, Level,
-    ErrCode, Level, MultiSpan, SubdiagnosticMessage, Substitution, SubstitutionPart,
+    MultiSpan, SubdiagnosticMessage, Substitution, SubstitutionPart, SuggestionStyle,
-    SuggestionStyle,
 };
 use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
 use rustc_error_messages::fluent_value_from_str_list_sep_by_and;
@@ -235,14 +234,11 @@ impl Diagnostic {
 
     pub fn is_error(&self) -> bool {
         match self.level {
-            Level::Bug
+            Level::Bug | Level::Fatal | Level::Error | Level::DelayedBug => true,
-            | Level::DelayedBug(DelayedBugKind::Normal)
-            | Level::Fatal
-            | Level::Error => true,
 
-            Level::ForceWarning(_)
+            Level::GoodPathDelayedBug
+            | Level::ForceWarning(_)
             | Level::Warning
-            | Level::DelayedBug(DelayedBugKind::GoodPath)
             | Level::Note
             | Level::OnceNote
             | Level::Help
@@ -306,11 +302,11 @@ impl Diagnostic {
     #[track_caller]
     pub fn downgrade_to_delayed_bug(&mut self) {
         assert!(
-            matches!(self.level, Level::Error | Level::DelayedBug(_)),
+            matches!(self.level, Level::Error | Level::DelayedBug),
             "downgrade_to_delayed_bug: cannot downgrade {:?} to DelayedBug: not an error",
             self.level
         );
-        self.level = Level::DelayedBug(DelayedBugKind::Normal);
+        self.level = Level::DelayedBug;
     }
 
     /// Appends a labeled span to the diagnostic.

compiler/rustc_errors/src/emitter.rs

@@ -2116,6 +2116,7 @@ impl HumanEmitter {
                 }
                 if !self.short_message {
                     for child in children {
+                        assert!(child.level.can_be_top_or_sub().1);
                         let span = &child.span;
                         if let Err(err) = self.emit_messages_default_inner(
                             span,

compiler/rustc_errors/src/lib.rs

@@ -439,7 +439,7 @@ struct DiagCtxtInner {
     has_printed: bool,
 
     emitter: Box<DynEmitter>,
-    span_delayed_bugs: Vec<DelayedDiagnostic>,
+    delayed_bugs: Vec<DelayedDiagnostic>,
     good_path_delayed_bugs: Vec<DelayedDiagnostic>,
     /// This flag indicates that an expected diagnostic was emitted and suppressed.
     /// This is used for the `good_path_delayed_bugs` check.
@@ -523,8 +523,7 @@ fn default_track_diagnostic(diag: Diagnostic, f: &mut dyn FnMut(Diagnostic)) {
 pub static TRACK_DIAGNOSTIC: AtomicRef<fn(Diagnostic, &mut dyn FnMut(Diagnostic))> =
     AtomicRef::new(&(default_track_diagnostic as _));
 
-#[derive(Copy, PartialEq, Eq, Clone, Hash, Debug, Encodable, Decodable)]
+enum DelayedBugKind {
-pub enum DelayedBugKind {
     Normal,
     GoodPath,
 }
@@ -557,11 +556,6 @@ impl Drop for DiagCtxtInner {
             self.flush_delayed(DelayedBugKind::Normal)
         }
 
-        // FIXME(eddyb) this explains what `good_path_delayed_bugs` are!
-        // They're `span_delayed_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_printed && !self.suppressed_expected_diag && !std::thread::panicking() {
             self.flush_delayed(DelayedBugKind::GoodPath);
         }
@@ -608,7 +602,7 @@ impl DiagCtxt {
                 deduplicated_warn_count: 0,
                 has_printed: false,
                 emitter,
-                span_delayed_bugs: Vec::new(),
+                delayed_bugs: Vec::new(),
                 good_path_delayed_bugs: Vec::new(),
                 suppressed_expected_diag: false,
                 taught_diagnostics: Default::default(),
@@ -664,7 +658,7 @@ impl DiagCtxt {
         inner.has_printed = false;
 
         // actually free the underlying memory (which `clear` would not do)
-        inner.span_delayed_bugs = Default::default();
+        inner.delayed_bugs = Default::default();
         inner.good_path_delayed_bugs = Default::default();
         inner.taught_diagnostics = Default::default();
         inner.emitted_diagnostic_codes = Default::default();
@@ -865,8 +859,7 @@ impl DiagCtxt {
     /// directly).
     #[track_caller]
     pub fn delayed_bug(&self, msg: impl Into<DiagnosticMessage>) -> ErrorGuaranteed {
-        DiagnosticBuilder::<ErrorGuaranteed>::new(self, DelayedBug(DelayedBugKind::Normal), msg)
+        DiagnosticBuilder::<ErrorGuaranteed>::new(self, DelayedBug, msg).emit()
-            .emit()
     }
 
     /// Like `delayed_bug`, but takes an additional span.
@@ -879,15 +872,12 @@ impl DiagCtxt {
         sp: impl Into<MultiSpan>,
         msg: impl Into<DiagnosticMessage>,
     ) -> ErrorGuaranteed {
-        DiagnosticBuilder::<ErrorGuaranteed>::new(self, DelayedBug(DelayedBugKind::Normal), msg)
+        DiagnosticBuilder::<ErrorGuaranteed>::new(self, DelayedBug, msg).with_span(sp).emit()
-            .with_span(sp)
-            .emit()
     }
 
-    // FIXME(eddyb) note the comment inside `impl Drop for DiagCtxtInner`, that's
+    /// Ensures that a diagnostic is printed. See `Level::GoodPathDelayedBug`.
-    // where the explanation of what "good path" is (also, it should be renamed).
     pub fn good_path_delayed_bug(&self, msg: impl Into<DiagnosticMessage>) {
-        DiagnosticBuilder::<()>::new(self, DelayedBug(DelayedBugKind::GoodPath), msg).emit()
+        DiagnosticBuilder::<()>::new(self, GoodPathDelayedBug, msg).emit()
     }
 
     #[track_caller]
@@ -961,7 +951,7 @@ impl DiagCtxt {
     pub fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
         let inner = self.inner.borrow();
         let result =
-            inner.has_errors() || inner.lint_err_count > 0 || !inner.span_delayed_bugs.is_empty();
+            inner.has_errors() || inner.lint_err_count > 0 || !inner.delayed_bugs.is_empty();
         result.then(|| {
             #[allow(deprecated)]
             ErrorGuaranteed::unchecked_claim_error_was_emitted()
@@ -1247,6 +1237,8 @@ impl DiagCtxtInner {
     }
 
     fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option<ErrorGuaranteed> {
+        assert!(diagnostic.level.can_be_top_or_sub().0);
+
         if let Some(expectation_id) = diagnostic.level.get_expectation_id() {
             // The `LintExpectationId` can be stable or unstable depending on when it was created.
             // Diagnostics created before the definition of `HirId`s are unstable and can not yet
@@ -1268,27 +1260,29 @@ impl DiagCtxtInner {
             self.future_breakage_diagnostics.push(diagnostic.clone());
         }
 
-        if matches!(diagnostic.level, DelayedBug(_)) && self.flags.eagerly_emit_delayed_bugs {
+        if matches!(diagnostic.level, DelayedBug | GoodPathDelayedBug)
+            && self.flags.eagerly_emit_delayed_bugs
+        {
             diagnostic.level = Error;
         }
 
         match diagnostic.level {
-            // This must come after the possible promotion of `DelayedBug` to `Error` above.
+            // This must come after the possible promotion of `DelayedBug`/`GoodPathDelayedBug` to
+            // `Error` above.
             Fatal | Error if self.treat_next_err_as_bug() => {
                 diagnostic.level = Bug;
             }
-            DelayedBug(DelayedBugKind::Normal) => {
+            DelayedBug => {
                 // FIXME(eddyb) this should check for `has_errors` and stop pushing
-                // once *any* errors were emitted (and truncate `span_delayed_bugs`
+                // once *any* errors were emitted (and truncate `delayed_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.
                 let backtrace = std::backtrace::Backtrace::capture();
-                self.span_delayed_bugs
+                self.delayed_bugs.push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace));
-                    .push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace));
                 #[allow(deprecated)]
                 return Some(ErrorGuaranteed::unchecked_claim_error_was_emitted());
             }
-            DelayedBug(DelayedBugKind::GoodPath) => {
+            GoodPathDelayedBug => {
                 let backtrace = std::backtrace::Backtrace::capture();
                 self.good_path_delayed_bugs
                     .push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace));
@@ -1392,12 +1386,12 @@ impl DiagCtxtInner {
     fn flush_delayed(&mut self, kind: DelayedBugKind) {
         let (bugs, note1) = match kind {
             DelayedBugKind::Normal => (
-                std::mem::take(&mut self.span_delayed_bugs),
+                std::mem::take(&mut self.delayed_bugs),
-                "no errors encountered even though `span_delayed_bug` issued",
+                "no errors encountered even though delayed bugs were created",
             ),
             DelayedBugKind::GoodPath => (
                 std::mem::take(&mut self.good_path_delayed_bugs),
-                "no warnings or errors encountered even though `good_path_delayed_bugs` issued",
+                "no warnings or errors encountered even though good path delayed bugs were created",
             ),
         };
         let note2 = "those delayed bugs will now be shown as internal compiler errors";
@@ -1436,8 +1430,8 @@ impl DiagCtxtInner {
             let mut bug =
                 if backtrace || self.ice_file.is_none() { bug.decorate() } else { bug.inner };
 
-            // "Undelay" the `DelayedBug`s (into plain `Bug`s).
+            // "Undelay" the delayed bugs (into plain `Bug`s).
-            if !matches!(bug.level, DelayedBug(_)) {
+            if !matches!(bug.level, DelayedBug | GoodPathDelayedBug) {
                 // NOTE(eddyb) not panicking here because we're already producing
                 // an ICE, and the more information the merrier.
                 bug.subdiagnostic(InvalidFlushedDelayedDiagnosticLevel {
@@ -1503,85 +1497,89 @@ impl DelayedDiagnostic {
     }
 }
 
+/// Level              is_error  EmissionGuarantee         Top-level  Sub   Used in lints?
+/// -----              --------  -----------------         ---------  ---   --------------
+/// Bug                yes       BugAbort                  yes        -     -
+/// Fatal              yes       FatalAbort/FatalError(*)  yes        -     -
+/// Error              yes       ErrorGuaranteed           yes        -     yes
+/// DelayedBug         yes       ErrorGuaranteed           yes        -     -
+/// GoodPathDelayedBug -         ()                        yes        -     -
+/// ForceWarning       -         ()                        yes        -     lint-only
+/// Warning            -         ()                        yes        yes   yes
+/// Note               -         ()                        rare       yes   -
+/// OnceNote           -         ()                        -          yes   lint-only
+/// Help               -         ()                        rare       yes   -
+/// OnceHelp           -         ()                        -          yes   lint-only
+/// FailureNote        -         ()                        rare       -     -
+/// Allow              -         ()                        yes        -     lint-only
+/// Expect             -         ()                        yes        -     lint-only
+///
+/// (*) `FatalAbort` normally, `FatalError` in the non-aborting "almost fatal" case that is
+///     occasionally used.
+///
 #[derive(Copy, PartialEq, Eq, Clone, Hash, Debug, Encodable, Decodable)]
 pub enum Level {
     /// For bugs in the compiler. Manifests as an ICE (internal compiler error) panic.
-    ///
-    /// Its `EmissionGuarantee` is `BugAbort`.
     Bug,
 
+    /// An error that causes an immediate abort. Used for things like configuration errors,
+    /// internal overflows, some file operation errors.
+    Fatal,
+
+    /// An error in the code being compiled, which prevents compilation from finishing. This is the
+    /// most common case.
+    Error,
+
     /// This is a strange one: lets you register an error without emitting it. If compilation ends
     /// without any other errors occurring, this will be emitted as a bug. Otherwise, it will be
     /// silently dropped. I.e. "expect other errors are emitted" semantics. Useful on code paths
     /// that should only be reached when compiling erroneous code.
-    ///
+    DelayedBug,
-    /// Its `EmissionGuarantee` is `ErrorGuaranteed` for `Normal` delayed bugs, and `()` for
-    /// `GoodPath` delayed bugs.
-    DelayedBug(DelayedBugKind),
 
-    /// An error that causes an immediate abort. Used for things like configuration errors,
+    /// Like `DelayedBug`, but weaker: lets you register an error without emitting it. If
-    /// internal overflows, some file operation errors.
+    /// compilation ends without any other diagnostics being emitted (and without an expected lint
+    /// being suppressed), this will be emitted as a bug. Otherwise, it will be silently dropped.
+    /// I.e. "expect other diagnostics are emitted (or suppressed)" semantics. Useful on code paths
+    /// that should only be reached when emitting diagnostics, e.g. for expensive one-time
+    /// diagnostic formatting operations.
     ///
-    /// Its `EmissionGuarantee` is `FatalAbort`, except in the non-aborting "almost fatal" case
+    /// FIXME(nnethercote) good path delayed bugs are semantically strange: if printed they produce
-    /// that is occasionally used, where it is `FatalError`.
+    /// an ICE, but they don't satisfy `is_error` and they don't guarantee an error is emitted.
-    Fatal,
+    /// Plus there's the extra complication with expected (suppressed) lints. They have limited
-
+    /// use, and are used in very few places, and "good path" isn't a good name. It would be good
-    /// An error in the code being compiled, which prevents compilation from finishing. This is the
+    /// to remove them.
-    /// most common case.
+    GoodPathDelayedBug,
-    ///
-    /// Its `EmissionGuarantee` is `ErrorGuaranteed`.
-    Error,
 
     /// A `force-warn` lint warning about the code being compiled. Does not prevent compilation
     /// from finishing.
     ///
     /// The [`LintExpectationId`] is used for expected lint diagnostics. In all other cases this
     /// should be `None`.
-    ///
-    /// Its `EmissionGuarantee` is `()`.
     ForceWarning(Option<LintExpectationId>),
 
     /// A warning about the code being compiled. Does not prevent compilation from finishing.
-    ///
-    /// Its `EmissionGuarantee` is `()`.
     Warning,
 
-    /// A message giving additional context. Rare, because notes are more commonly attached to other
+    /// A message giving additional context.
-    /// diagnostics such as errors.
-    ///
-    /// Its `EmissionGuarantee` is `()`.
     Note,
 
-    /// A note that is only emitted once. Rare, mostly used in circumstances relating to lints.
+    /// A note that is only emitted once.
-    ///
-    /// Its `EmissionGuarantee` is `()`.
     OnceNote,
 
-    /// A message suggesting how to fix something. Rare, because help messages are more commonly
+    /// A message suggesting how to fix something.
-    /// attached to other diagnostics such as errors.
-    ///
-    /// Its `EmissionGuarantee` is `()`.
     Help,
 
-    /// A help that is only emitted once. Rare.
+    /// A help that is only emitted once.
-    ///
-    /// Its `EmissionGuarantee` is `()`.
     OnceHelp,
 
     /// Similar to `Note`, but used in cases where compilation has failed. When printed for human
-    /// consumption, it doesn't have any kind of `note:` label. Rare.
+    /// consumption, it doesn't have any kind of `note:` label.
-    ///
-    /// Its `EmissionGuarantee` is `()`.
     FailureNote,
 
     /// Only used for lints.
-    ///
-    /// Its `EmissionGuarantee` is `()`.
     Allow,
 
     /// Only used for lints.
-    ///
-    /// Its `EmissionGuarantee` is `()`.
     Expect(LintExpectationId),
 }
 
@@ -1595,7 +1593,7 @@ impl Level {
     fn color(self) -> ColorSpec {
         let mut spec = ColorSpec::new();
         match self {
-            Bug | DelayedBug(_) | Fatal | Error => {
+            Bug | Fatal | Error | DelayedBug | GoodPathDelayedBug => {
                 spec.set_fg(Some(Color::Red)).set_intense(true);
             }
             ForceWarning(_) | Warning => {
@@ -1615,7 +1613,7 @@ impl Level {
 
     pub fn to_str(self) -> &'static str {
         match self {
-            Bug | DelayedBug(_) => "error: internal compiler error",
+            Bug | DelayedBug | GoodPathDelayedBug => "error: internal compiler error",
             Fatal | Error => "error",
             ForceWarning(_) | Warning => "warning",
             Note | OnceNote => "note",
@@ -1635,6 +1633,19 @@ impl Level {
             _ => None,
         }
     }
+
+    // Can this level be used in a top-level diagnostic message and/or a
+    // subdiagnostic message?
+    fn can_be_top_or_sub(&self) -> (bool, bool) {
+        match self {
+            Bug | DelayedBug | Fatal | Error | GoodPathDelayedBug | ForceWarning(_)
+            | FailureNote | Allow | Expect(_) => (true, false),
+
+            Warning | Note | Help => (true, true),
+
+            OnceNote | OnceHelp => (false, true),
+        }
+    }
 }
 
 // FIXME(eddyb) this doesn't belong here AFAICT, should be moved to callsite.

tests/ui/impl-trait/equality-in-canonical-query.clone.stderr

@@ -1,4 +1,4 @@
-note: no errors encountered even though `span_delayed_bug` issued
+note: no errors encountered even though delayed bugs were created
 
 note: those delayed bugs will now be shown as internal compiler errors
 

tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr

@@ -1,4 +1,4 @@
-note: no errors encountered even though `span_delayed_bug` issued
+note: no errors encountered even though delayed bugs were created
 
 note: those delayed bugs will now be shown as internal compiler errors