Auto merge of #121790 - jhpratt:rollup-yocg203, r=jhpratt

Rollup of 10 pull requests

Successful merges:

 - #119748 (Increase visibility of `join_path` and `split_paths`)
 - #120291 (Have `String` use `SliceIndex` impls from `str`)
 - #121723 (Two diagnostic things)
 - #121740 (Changing some attributes to only_local.)
 - #121745 (Deeply normalize obligations in `refining_impl_trait`)
 - #121748 (Restore the standard library review rotation to its former glory)
 - #121768 (Implement unwind safety for Condvar on all platforms )
 - #121777 (Fix typo in `rustc_passes/messages.ftl`)
 - #121778 (Document potential memory leak in unbounded channel)
 - #121779 (Remove unused diagnostic struct)

r? `@ghost`
`@rustbot` modify labels: rollup

2 files changed, 40 insertions, 21 deletions

diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs
index 01f36ad6a78..0cf519d2029 100644
--- a/compiler/rustc_errors/src/diagnostic.rs
+++ b/compiler/rustc_errors/src/diagnostic.rs
@@ -108,6 +108,25 @@ impl EmissionGuarantee for rustc_span::fatal_error::FatalError {
 
 /// Trait implemented by error types. This is rarely implemented manually. Instead, use
 /// `#[derive(Diagnostic)]` -- see [rustc_macros::Diagnostic].
+///
+/// When implemented manually, it should be generic over the emission
+/// guarantee, i.e.:
+/// ```ignore (fragment)
+/// impl<'a, G: EmissionGuarantee> IntoDiagnostic<'a, G> for Foo { ... }
+/// ```
+/// rather than being specific:
+/// ```ignore (fragment)
+/// impl<'a> IntoDiagnostic<'a> for Bar { ... }  // the default type param is `ErrorGuaranteed`
+/// impl<'a> IntoDiagnostic<'a, ()> for Baz { ... }
+/// ```
+/// There are two reasons for this.
+/// - A diagnostic like `Foo` *could* be emitted at any level -- `level` is
+///   passed in to `into_diagnostic` from outside. Even if in practice it is
+///   always emitted at a single level, we let the diagnostic creation/emission
+///   site determine the level (by using `create_err`, `emit_warn`, etc.)
+///   rather than the `IntoDiagnostic` impl.
+/// - Derived impls are always generic, and it's good for the hand-written
+///   impls to be consistent with them.
 #[rustc_diagnostic_item = "IntoDiagnostic"]
 pub trait IntoDiagnostic<'a, G: EmissionGuarantee = ErrorGuaranteed> {
     /// Write out as a diagnostic out of `DiagCtxt`.
diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index bc338b01d8b..2652feff62a 100644
--- a/compiler/rustc_errors/src/lib.rs
+++ b/compiler/rustc_errors/src/lib.rs
@@ -1498,14 +1498,26 @@ impl DiagCtxtInner {
         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() {
-            if let Some(file) = self.ice_file.as_ref()
-                && let Ok(mut out) = std::fs::File::options().create(true).append(true).open(file)
-            {
-                let _ = write!(
-                    &mut out,
+        let decorate = backtrace || self.ice_file.is_none();
+        let mut out = self
+            .ice_file
+            .as_ref()
+            .and_then(|file| std::fs::File::options().create(true).append(true).open(file).ok());
+
+        // Put the overall explanation before the `DelayedBug`s, to frame them
+        // better (e.g. separate warnings from them). Also, use notes, which
+        // don't count as errors, to avoid possibly triggering
+        // `-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(DiagInner::new(Note, note1));
+        self.emit_diagnostic(DiagInner::new(Note, note2));
+
+        for bug in bugs {
+            if let Some(out) = &mut out {
+                _ = write!(
+                    out,
                     "delayed bug: {}\n{}\n",
                     bug.inner
                         .messages
@@ -1516,21 +1528,9 @@ impl DiagCtxtInner {
                 );
             }
 
-            if i == 0 {
-                // Put the overall explanation before the `DelayedBug`s, to
-                // 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(DiagInner::new(Note, note1));
-                self.emit_diagnostic(DiagInner::new(Note, note2));
-            }
-
-            let mut bug =
-                if backtrace || self.ice_file.is_none() { bug.decorate(self) } else { bug.inner };
+            let mut bug = if decorate { bug.decorate(self) } else { bug.inner };
 
-            // "Undelay" the delayed bugs (into plain `Bug`s).
+            // "Undelay" the delayed bugs into plain bugs.
             if bug.level != DelayedBug {
                 // NOTE(eddyb) not panicking here because we're already producing
                 // an ICE, and the more information the merrier.