Merge from rustc

4 files changed, 141 insertions, 120 deletions

diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
index 37f568f12a7..1a34a83c1a4 100644
--- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
+++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
@@ -85,11 +85,7 @@ 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
-        | Level::DelayedBug
-        | Level::GoodPathDelayedBug => AnnotationType::Error,
+        Level::Bug | Level::Fatal | Level::Error | Level::DelayedBug => AnnotationType::Error,
         Level::ForceWarning(_) | Level::Warning => AnnotationType::Warning,
         Level::Note | Level::OnceNote => AnnotationType::Note,
         Level::Help | Level::OnceHelp => AnnotationType::Help,
diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs
index 2deb18484ec..b14a12175c7 100644
--- a/compiler/rustc_errors/src/diagnostic.rs
+++ b/compiler/rustc_errors/src/diagnostic.rs
@@ -237,8 +237,7 @@ impl Diagnostic {
         match self.level {
             Level::Bug | Level::Fatal | Level::Error | Level::DelayedBug => true,
 
-            Level::GoodPathDelayedBug
-            | Level::ForceWarning(_)
+            Level::ForceWarning(_)
             | Level::Warning
             | Level::Note
             | Level::OnceNote
diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs
index b9e92dbb31c..38c6661377b 100644
--- a/compiler/rustc_errors/src/emitter.rs
+++ b/compiler/rustc_errors/src/emitter.rs
@@ -1635,7 +1635,8 @@ impl HumanEmitter {
                     let mut to_add = FxHashMap::default();
 
                     for (depth, style) in depths {
-                        if multilines.remove(&depth).is_none() {
+                        // FIXME(#120456) - is `swap_remove` correct?
+                        if multilines.swap_remove(&depth).is_none() {
                             to_add.insert(depth, style);
                         }
                     }
diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index da9ef6627be..d876f28040d 100644
--- a/compiler/rustc_errors/src/lib.rs
+++ b/compiler/rustc_errors/src/lib.rs
@@ -78,6 +78,7 @@ use std::fmt;
 use std::hash::Hash;
 use std::io::Write;
 use std::num::NonZeroUsize;
+use std::ops::DerefMut;
 use std::panic;
 use std::path::{Path, PathBuf};
 
@@ -435,7 +436,6 @@ struct DiagCtxtInner {
     lint_err_guars: Vec<ErrorGuaranteed>,
     /// The delayed bugs and their error guarantees.
     delayed_bugs: Vec<(DelayedDiagnostic, ErrorGuaranteed)>,
-    good_path_delayed_bugs: Vec<DelayedDiagnostic>,
 
     /// The number of stashed errors. Unlike the other counts, this can go up
     /// and down, so it doesn't guarantee anything.
@@ -446,13 +446,18 @@ struct DiagCtxtInner {
     /// The warning count shown to the user at the end.
     deduplicated_warn_count: usize,
 
+    emitter: Box<DynEmitter>,
+
+    /// Must we produce a diagnostic to justify the use of the expensive
+    /// `trimmed_def_paths` function?
+    must_produce_diag: bool,
+
     /// Has this diagnostic context printed any diagnostics? (I.e. has
     /// `self.emitter.emit_diagnostic()` been called?
     has_printed: bool,
 
-    emitter: Box<DynEmitter>,
     /// This flag indicates that an expected diagnostic was emitted and suppressed.
-    /// This is used for the `good_path_delayed_bugs` check.
+    /// This is used for the `must_produce_diag` check.
     suppressed_expected_diag: bool,
 
     /// This set contains the code of all emitted diagnostics to avoid
@@ -533,11 +538,6 @@ 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 _));
 
-enum DelayedBugKind {
-    Normal,
-    GoodPath,
-}
-
 #[derive(Copy, Clone, Default)]
 pub struct DiagCtxtFlags {
     /// If false, warning-level lints are suppressed.
@@ -563,11 +563,16 @@ impl Drop for DiagCtxtInner {
         self.emit_stashed_diagnostics();
 
         if self.err_guars.is_empty() {
-            self.flush_delayed(DelayedBugKind::Normal)
+            self.flush_delayed()
         }
 
         if !self.has_printed && !self.suppressed_expected_diag && !std::thread::panicking() {
-            self.flush_delayed(DelayedBugKind::GoodPath);
+            if self.must_produce_diag {
+                panic!(
+                    "must_produce_diag: trimmed_def_paths called but no diagnostics emitted; \
+                       use `DelayDm` for lints or `with_no_trimmed_paths` for debugging"
+                );
+            }
         }
 
         if self.check_unstable_expect_diagnostics {
@@ -609,12 +614,12 @@ impl DiagCtxt {
                 err_guars: Vec::new(),
                 lint_err_guars: Vec::new(),
                 delayed_bugs: Vec::new(),
-                good_path_delayed_bugs: Vec::new(),
                 stashed_err_count: 0,
                 deduplicated_err_count: 0,
                 deduplicated_warn_count: 0,
-                has_printed: false,
                 emitter,
+                must_produce_diag: false,
+                has_printed: false,
                 suppressed_expected_diag: false,
                 taught_diagnostics: Default::default(),
                 emitted_diagnostic_codes: Default::default(),
@@ -662,21 +667,51 @@ impl DiagCtxt {
     /// tools that want to reuse a `Parser` cleaning the previously emitted diagnostics as well as
     /// the overall count of emitted error diagnostics.
     pub fn reset_err_count(&self) {
+        // Use destructuring so that if a field gets added to `DiagCtxtInner`, it's impossible to
+        // fail to update this method as well.
         let mut inner = self.inner.borrow_mut();
-        inner.stashed_err_count = 0;
-        inner.deduplicated_err_count = 0;
-        inner.deduplicated_warn_count = 0;
-        inner.has_printed = false;
-
-        // actually free the underlying memory (which `clear` would not do)
-        inner.err_guars = Default::default();
-        inner.lint_err_guars = 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();
-        inner.emitted_diagnostics = Default::default();
-        inner.stashed_diagnostics = Default::default();
+        let DiagCtxtInner {
+            flags: _,
+            err_guars,
+            lint_err_guars,
+            delayed_bugs,
+            stashed_err_count,
+            deduplicated_err_count,
+            deduplicated_warn_count,
+            emitter: _,
+            must_produce_diag,
+            has_printed,
+            suppressed_expected_diag,
+            taught_diagnostics,
+            emitted_diagnostic_codes,
+            emitted_diagnostics,
+            stashed_diagnostics,
+            future_breakage_diagnostics,
+            check_unstable_expect_diagnostics,
+            unstable_expect_diagnostics,
+            fulfilled_expectations,
+            ice_file: _,
+        } = inner.deref_mut();
+
+        // For the `Vec`s and `HashMap`s, we overwrite with an empty container to free the
+        // underlying memory (which `clear` would not do).
+        *err_guars = Default::default();
+        *lint_err_guars = Default::default();
+        *delayed_bugs = Default::default();
+        *stashed_err_count = 0;
+        *deduplicated_err_count = 0;
+        *deduplicated_warn_count = 0;
+        *must_produce_diag = false;
+        *has_printed = false;
+        *suppressed_expected_diag = false;
+        *taught_diagnostics = Default::default();
+        *emitted_diagnostic_codes = Default::default();
+        *emitted_diagnostics = Default::default();
+        *stashed_diagnostics = Default::default();
+        *future_breakage_diagnostics = Default::default();
+        *check_unstable_expect_diagnostics = false;
+        *unstable_expect_diagnostics = Default::default();
+        *fulfilled_expectations = Default::default();
     }
 
     /// Stash a given diagnostic with the given `Span` and [`StashKey`] as the key.
@@ -702,7 +737,8 @@ impl DiagCtxt {
     pub fn steal_diagnostic(&self, span: Span, key: StashKey) -> Option<DiagnosticBuilder<'_, ()>> {
         let mut inner = self.inner.borrow_mut();
         let key = (span.with_parent(None), key);
-        let diag = inner.stashed_diagnostics.remove(&key)?;
+        // FIXME(#120456) - is `swap_remove` correct?
+        let diag = inner.stashed_diagnostics.swap_remove(&key)?;
         if diag.is_error() {
             if diag.is_lint.is_none() {
                 inner.stashed_err_count -= 1;
@@ -775,11 +811,12 @@ impl DiagCtxt {
         match (errors.len(), warnings.len()) {
             (0, 0) => return,
             (0, _) => {
-                // Use `inner.emitter` directly, otherwise the warning might not be emitted, e.g.
-                // with a configuration like `--cap-lints allow --force-warn bare_trait_objects`.
-                inner
-                    .emitter
-                    .emit_diagnostic(Diagnostic::new(Warning, DiagnosticMessage::Str(warnings)));
+                // Use `ForceWarning` rather than `Warning` to guarantee emission, e.g. with a
+                // configuration like `--cap-lints allow --force-warn bare_trait_objects`.
+                inner.emit_diagnostic(Diagnostic::new(
+                    ForceWarning(None),
+                    DiagnosticMessage::Str(warnings),
+                ));
             }
             (_, 0) => {
                 inner.emit_diagnostic(Diagnostic::new(Error, errors));
@@ -807,20 +844,23 @@ impl DiagCtxt {
                 error_codes.sort();
                 if error_codes.len() > 1 {
                     let limit = if error_codes.len() > 9 { 9 } else { error_codes.len() };
-                    inner.failure_note(format!(
+                    let msg1 = format!(
                         "Some errors have detailed explanations: {}{}",
                         error_codes[..limit].join(", "),
                         if error_codes.len() > 9 { "..." } else { "." }
-                    ));
-                    inner.failure_note(format!(
+                    );
+                    let msg2 = format!(
                         "For more information about an error, try `rustc --explain {}`.",
                         &error_codes[0]
-                    ));
+                    );
+                    inner.emit_diagnostic(Diagnostic::new(FailureNote, msg1));
+                    inner.emit_diagnostic(Diagnostic::new(FailureNote, msg2));
                 } else {
-                    inner.failure_note(format!(
+                    let msg = format!(
                         "For more information about this error, try `rustc --explain {}`.",
                         &error_codes[0]
-                    ));
+                    );
+                    inner.emit_diagnostic(Diagnostic::new(FailureNote, msg));
                 }
             }
         }
@@ -843,10 +883,6 @@ impl DiagCtxt {
         self.inner.borrow_mut().taught_diagnostics.insert(code)
     }
 
-    pub fn force_print_diagnostic(&self, db: Diagnostic) {
-        self.inner.borrow_mut().emitter.emit_diagnostic(db);
-    }
-
     pub fn emit_diagnostic(&self, diagnostic: Diagnostic) -> Option<ErrorGuaranteed> {
         self.inner.borrow_mut().emit_diagnostic(diagnostic)
     }
@@ -934,7 +970,13 @@ impl DiagCtxt {
     }
 
     pub fn flush_delayed(&self) {
-        self.inner.borrow_mut().flush_delayed(DelayedBugKind::Normal);
+        self.inner.borrow_mut().flush_delayed();
+    }
+
+    /// Used when trimmed_def_paths is called and we must produce a diagnostic
+    /// to justify its cost.
+    pub fn set_must_produce_diag(&self) {
+        self.inner.borrow_mut().must_produce_diag = true;
     }
 }
 
@@ -1108,13 +1150,6 @@ impl DiagCtxt {
         DiagnosticBuilder::<ErrorGuaranteed>::new(self, DelayedBug, msg).with_span(sp).emit()
     }
 
-    /// Ensures that a diagnostic is printed. See `Level::GoodPathDelayedBug`.
-    // No `#[rustc_lint_diagnostics]` because bug messages aren't user-facing.
-    #[track_caller]
-    pub fn good_path_delayed_bug(&self, msg: impl Into<DiagnosticMessage>) {
-        DiagnosticBuilder::<()>::new(self, GoodPathDelayedBug, msg).emit()
-    }
-
     #[rustc_lint_diagnostics]
     #[track_caller]
     pub fn struct_warn(&self, msg: impl Into<DiagnosticMessage>) -> DiagnosticBuilder<'_, ()> {
@@ -1175,7 +1210,7 @@ impl DiagCtxt {
         span: impl Into<MultiSpan>,
         msg: impl Into<DiagnosticMessage>,
     ) -> DiagnosticBuilder<'_, ()> {
-        DiagnosticBuilder::new(self, Note, msg).with_span(span)
+        self.struct_note(msg).with_span(span)
     }
 
     #[rustc_lint_diagnostics]
@@ -1205,6 +1240,15 @@ impl DiagCtxt {
 
     #[rustc_lint_diagnostics]
     #[track_caller]
+    pub fn struct_failure_note(
+        &self,
+        msg: impl Into<DiagnosticMessage>,
+    ) -> DiagnosticBuilder<'_, ()> {
+        DiagnosticBuilder::new(self, FailureNote, msg)
+    }
+
+    #[rustc_lint_diagnostics]
+    #[track_caller]
     pub fn struct_allow(&self, msg: impl Into<DiagnosticMessage>) -> DiagnosticBuilder<'_, ()> {
         DiagnosticBuilder::new(self, Allow, msg)
     }
@@ -1266,42 +1310,39 @@ impl DiagCtxtInner {
         if diagnostic.has_future_breakage() {
             // Future breakages aren't emitted if they're Level::Allow,
             // but they still need to be constructed and stashed below,
-            // so they'll trigger the good-path bug check.
+            // so they'll trigger the must_produce_diag check.
             self.suppressed_expected_diag = true;
             self.future_breakage_diagnostics.push(diagnostic.clone());
         }
 
-        if matches!(diagnostic.level, DelayedBug | GoodPathDelayedBug)
-            && self.flags.eagerly_emit_delayed_bugs
-        {
+        // Note that because this comes before the `match` below,
+        // `-Zeagerly-emit-delayed-bugs` continues to work even after we've
+        // issued an error and stopped recording new delayed bugs.
+        if diagnostic.level == DelayedBug && self.flags.eagerly_emit_delayed_bugs {
             diagnostic.level = Error;
         }
 
         match diagnostic.level {
-            // This must come after the possible promotion of `DelayedBug`/`GoodPathDelayedBug` to
+            // This must come after the possible promotion of `DelayedBug` to
             // `Error` above.
             Fatal | Error if self.treat_next_err_as_bug() => {
                 diagnostic.level = Bug;
             }
             DelayedBug => {
-                // FIXME(eddyb) this should check for `has_errors` and stop pushing
-                // 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();
-                // This `unchecked_error_guaranteed` is valid. It is where the
-                // `ErrorGuaranteed` for delayed bugs originates.
-                #[allow(deprecated)]
-                let guar = ErrorGuaranteed::unchecked_error_guaranteed();
-                self.delayed_bugs
-                    .push((DelayedDiagnostic::with_backtrace(diagnostic, backtrace), guar));
-                return Some(guar);
-            }
-            GoodPathDelayedBug => {
-                let backtrace = std::backtrace::Backtrace::capture();
-                self.good_path_delayed_bugs
-                    .push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace));
-                return None;
+                // If we have already emitted at least one error, we don't need
+                // to record the delayed bug, because it'll never be used.
+                return if let Some(guar) = self.has_errors_or_lint_errors() {
+                    Some(guar)
+                } else {
+                    let backtrace = std::backtrace::Backtrace::capture();
+                    // This `unchecked_error_guaranteed` is valid. It is where the
+                    // `ErrorGuaranteed` for delayed bugs originates.
+                    #[allow(deprecated)]
+                    let guar = ErrorGuaranteed::unchecked_error_guaranteed();
+                    self.delayed_bugs
+                        .push((DelayedDiagnostic::with_backtrace(diagnostic, backtrace), guar));
+                    Some(guar)
+                };
             }
             Warning if !self.flags.can_emit_warnings => {
                 if diagnostic.has_future_breakage() {
@@ -1367,6 +1408,16 @@ impl DiagCtxtInner {
             }
 
             if is_error {
+                // If we have any delayed bugs recorded, we can discard them
+                // because they won't be used. (This should only occur if there
+                // have been no errors previously emitted, because we don't add
+                // new delayed bugs once the first error is emitted.)
+                if !self.delayed_bugs.is_empty() {
+                    assert_eq!(self.lint_err_guars.len() + self.err_guars.len(), 0);
+                    self.delayed_bugs.clear();
+                    self.delayed_bugs.shrink_to_fit();
+                }
+
                 // This `unchecked_error_guaranteed` is valid. It is where the
                 // `ErrorGuaranteed` for errors and lint errors originates.
                 #[allow(deprecated)]
@@ -1410,27 +1461,14 @@ impl DiagCtxtInner {
             .or_else(|| self.delayed_bugs.get(0).map(|(_, guar)| guar).copied())
     }
 
-    fn failure_note(&mut self, msg: impl Into<DiagnosticMessage>) {
-        self.emit_diagnostic(Diagnostic::new(FailureNote, msg));
-    }
-
-    fn flush_delayed(&mut self, kind: DelayedBugKind) {
-        let (bugs, note1) = match kind {
-            DelayedBugKind::Normal => (
-                std::mem::take(&mut self.delayed_bugs).into_iter().map(|(b, _)| b).collect(),
-                "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 were created",
-            ),
-        };
-        let note2 = "those delayed bugs will now be shown as internal compiler errors";
-
-        if bugs.is_empty() {
+    fn flush_delayed(&mut self) {
+        if self.delayed_bugs.is_empty() {
             return;
         }
 
+        let bugs: Vec<_> =
+            std::mem::take(&mut self.delayed_bugs).into_iter().map(|(b, _)| b).collect();
+
         // If backtraces are enabled, also print the query stack
         let backtrace = std::env::var_os("RUST_BACKTRACE").map_or(true, |x| &x != "0");
         for (i, bug) in bugs.into_iter().enumerate() {
@@ -1454,6 +1492,8 @@ impl DiagCtxtInner {
                 // frame them better (e.g. separate warnings from them). Also,
                 // make it a note so it doesn't count as an error, because that
                 // could trigger `-Ztreat-err-as-bug`, which we don't want.
+                let note1 = "no errors encountered even though delayed bugs were created";
+                let note2 = "those delayed bugs will now be shown as internal compiler errors";
                 self.emit_diagnostic(Diagnostic::new(Note, note1));
                 self.emit_diagnostic(Diagnostic::new(Note, note2));
             }
@@ -1462,7 +1502,7 @@ impl DiagCtxtInner {
                 if backtrace || self.ice_file.is_none() { bug.decorate() } else { bug.inner };
 
             // "Undelay" the delayed bugs (into plain `Bug`s).
-            if !matches!(bug.level, DelayedBug | GoodPathDelayedBug) {
+            if bug.level != DelayedBug {
                 // NOTE(eddyb) not panicking here because we're already producing
                 // an ICE, and the more information the merrier.
                 bug.subdiagnostic(InvalidFlushedDelayedDiagnosticLevel {
@@ -1534,7 +1574,6 @@ impl DelayedDiagnostic {
 /// 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   -
@@ -1567,20 +1606,6 @@ pub enum Level {
     /// that should only be reached when compiling erroneous code.
     DelayedBug,
 
-    /// Like `DelayedBug`, but weaker: lets you register an error without emitting it. If
-    /// 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.
-    ///
-    /// FIXME(nnethercote) good path delayed bugs are semantically strange: if printed they produce
-    /// an ICE, but they don't satisfy `is_error` and they don't guarantee an error is emitted.
-    /// 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
-    /// to remove them.
-    GoodPathDelayedBug,
-
     /// A `force-warn` lint warning about the code being compiled. Does not prevent compilation
     /// from finishing.
     ///
@@ -1625,7 +1650,7 @@ impl Level {
     fn color(self) -> ColorSpec {
         let mut spec = ColorSpec::new();
         match self {
-            Bug | Fatal | Error | DelayedBug | GoodPathDelayedBug => {
+            Bug | Fatal | Error | DelayedBug => {
                 spec.set_fg(Some(Color::Red)).set_intense(true);
             }
             ForceWarning(_) | Warning => {
@@ -1645,7 +1670,7 @@ impl Level {
 
     pub fn to_str(self) -> &'static str {
         match self {
-            Bug | DelayedBug | GoodPathDelayedBug => "error: internal compiler error",
+            Bug | DelayedBug => "error: internal compiler error",
             Fatal | Error => "error",
             ForceWarning(_) | Warning => "warning",
             Note | OnceNote => "note",
@@ -1670,8 +1695,8 @@ impl Level {
     // subdiagnostic message?
     fn can_be_top_or_sub(&self) -> (bool, bool) {
         match self {
-            Bug | DelayedBug | Fatal | Error | GoodPathDelayedBug | ForceWarning(_)
-            | FailureNote | Allow | Expect(_) => (true, false),
+            Bug | DelayedBug | Fatal | Error | ForceWarning(_) | FailureNote | Allow
+            | Expect(_) => (true, false),
 
             Warning | Note | Help => (true, true),