Rollup merge of #110989 - jyn514:bug-report-url, r=WaffleLapkin

Make the BUG_REPORT_URL configurable by tools

This greatly simplifies how hard it is to set a custom bug report url; previously tools had to copy
the entire hook implementation.

I haven't changed clippy in case they want to make the change upstream instead of the subtree, but
I'm happy to do so here if the maintainers want - cc ````@rust-lang/clippy````

Fixes #109486.

Author: matthiaskrgr

compiler/rustc_driver_impl/src/lib.rs

@@ -25,7 +25,7 @@ use rustc_data_structures::profiling::{
 use rustc_data_structures::sync::SeqCst;
 use rustc_errors::registry::{InvalidErrorCode, Registry};
 use rustc_errors::{
-    DiagnosticMessage, ErrorGuaranteed, PResult, SubdiagnosticMessage, TerminalUrl,
+    DiagnosticMessage, ErrorGuaranteed, Handler, PResult, SubdiagnosticMessage, TerminalUrl,
 };
 use rustc_feature::find_gated_cfg;
 use rustc_fluent_macro::fluent_messages;
@@ -55,7 +55,7 @@ use std::panic::{self, catch_unwind};
 use std::path::PathBuf;
 use std::process::{self, Command, Stdio};
 use std::str;
-use std::sync::LazyLock;
+use std::sync::OnceLock;
 use std::time::Instant;
 
 // This import blocks the use of panicking `print` and `println` in all the code
@@ -119,7 +119,7 @@ pub const EXIT_SUCCESS: i32 = 0;
 /// Exit status code used for compilation failures and invalid flags.
 pub const EXIT_FAILURE: i32 = 1;
 
-const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust/issues/new\
+pub const DEFAULT_BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust/issues/new\
     ?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md";
 
 const ICE_REPORT_COMPILER_FLAGS: &[&str] = &["-Z", "-C", "--crate-type"];
@@ -1196,43 +1196,66 @@ pub fn catch_with_exit_code(f: impl FnOnce() -> interface::Result<()>) -> i32 {
     }
 }
 
-static DEFAULT_HOOK: LazyLock<Box<dyn Fn(&panic::PanicInfo<'_>) + Sync + Send + 'static>> =
-    LazyLock::new(|| {
-        let hook = panic::take_hook();
-        panic::set_hook(Box::new(|info| {
-            // If the error was caused by a broken pipe then this is not a bug.
-            // Write the error and return immediately. See #98700.
-            #[cfg(windows)]
-            if let Some(msg) = info.payload().downcast_ref::<String>() {
-                if msg.starts_with("failed printing to stdout: ") && msg.ends_with("(os error 232)")
-                {
-                    early_error_no_abort(ErrorOutputType::default(), &msg);
-                    return;
-                }
-            };
+/// Stores the default panic hook, from before [`install_ice_hook`] was called.
+static DEFAULT_HOOK: OnceLock<Box<dyn Fn(&panic::PanicInfo<'_>) + Sync + Send + 'static>> =
+    OnceLock::new();
+
+/// Installs a panic hook that will print the ICE message on unexpected panics.
+///
+/// The hook is intended to be useable even by external tools. You can pass a custom
+/// `bug_report_url`, or report arbitrary info in `extra_info`. Note that `extra_info` is called in
+/// a context where *the thread is currently panicking*, so it must not panic or the process will
+/// abort.
+///
+/// If you have no extra info to report, pass the empty closure `|_| ()` as the argument to
+/// extra_info.
+///
+/// A custom rustc driver can skip calling this to set up a custom ICE hook.
+pub fn install_ice_hook(bug_report_url: &'static str, extra_info: fn(&Handler)) {
+    // If the user has not explicitly overridden "RUST_BACKTRACE", then produce
+    // full backtraces. When a compiler ICE happens, we want to gather
+    // as much information as possible to present in the issue opened
+    // by the user. Compiler developers and other rustc users can
+    // opt in to less-verbose backtraces by manually setting "RUST_BACKTRACE"
+    // (e.g. `RUST_BACKTRACE=1`)
+    if std::env::var("RUST_BACKTRACE").is_err() {
+        std::env::set_var("RUST_BACKTRACE", "full");
+    }
 
-            // Invoke the default handler, which prints the actual panic message and optionally a backtrace
-            // Don't do this for delayed bugs, which already emit their own more useful backtrace.
-            if !info.payload().is::<rustc_errors::DelayedBugPanic>() {
-                (*DEFAULT_HOOK)(info);
+    let default_hook = DEFAULT_HOOK.get_or_init(panic::take_hook);
 
-                // Separate the output with an empty line
-                eprintln!();
+    panic::set_hook(Box::new(move |info| {
+        // If the error was caused by a broken pipe then this is not a bug.
+        // Write the error and return immediately. See #98700.
+        #[cfg(windows)]
+        if let Some(msg) = info.payload().downcast_ref::<String>() {
+            if msg.starts_with("failed printing to stdout: ") && msg.ends_with("(os error 232)") {
+                early_error_no_abort(ErrorOutputType::default(), &msg);
+                return;
             }
+        };
 
-            // Print the ICE message
-            report_ice(info, BUG_REPORT_URL);
-        }));
-        hook
-    });
+        // Invoke the default handler, which prints the actual panic message and optionally a backtrace
+        // Don't do this for delayed bugs, which already emit their own more useful backtrace.
+        if !info.payload().is::<rustc_errors::DelayedBugPanic>() {
+            (*default_hook)(info);
+
+            // Separate the output with an empty line
+            eprintln!();
+        }
+
+        // Print the ICE message
+        report_ice(info, bug_report_url, extra_info);
+    }));
+}
 
 /// Prints the ICE message, including query stack, but without backtrace.
 ///
 /// The message will point the user at `bug_report_url` to report the ICE.
 ///
 /// When `install_ice_hook` is called, this function will be called as the panic
 /// hook.
-pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) {
+pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str, extra_info: fn(&Handler)) {
     let fallback_bundle =
         rustc_errors::fallback_fluent_bundle(crate::DEFAULT_LOCALE_RESOURCES.to_vec(), false);
     let emitter = Box::new(rustc_errors::emitter::EmitterWriter::stderr(
@@ -1277,29 +1300,17 @@ pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) {
 
     interface::try_print_query_stack(&handler, num_frames);
 
+    // We don't trust this callback not to panic itself, so run it at the end after we're sure we've
+    // printed all the relevant info.
+    extra_info(&handler);
+
     #[cfg(windows)]
     if env::var("RUSTC_BREAK_ON_ICE").is_ok() {
         // Trigger a debugger if we crashed during bootstrap
         unsafe { windows::Win32::System::Diagnostics::Debug::DebugBreak() };
     }
 }
 
-/// Installs a panic hook that will print the ICE message on unexpected panics.
-///
-/// A custom rustc driver can skip calling this to set up a custom ICE hook.
-pub fn install_ice_hook() {
-    // If the user has not explicitly overridden "RUST_BACKTRACE", then produce
-    // full backtraces. When a compiler ICE happens, we want to gather
-    // as much information as possible to present in the issue opened
-    // by the user. Compiler developers and other rustc users can
-    // opt in to less-verbose backtraces by manually setting "RUST_BACKTRACE"
-    // (e.g. `RUST_BACKTRACE=1`)
-    if std::env::var("RUST_BACKTRACE").is_err() {
-        std::env::set_var("RUST_BACKTRACE", "full");
-    }
-    LazyLock::force(&DEFAULT_HOOK);
-}
-
 /// This allows tools to enable rust logging without having to magically match rustc's
 /// tracing crate version.
 pub fn init_rustc_env_logger() {
@@ -1370,7 +1381,7 @@ pub fn main() -> ! {
     init_rustc_env_logger();
     signal_handler::install();
     let mut callbacks = TimePassesCallbacks::default();
-    install_ice_hook();
+    install_ice_hook(DEFAULT_BUG_REPORT_URL, |_| ());
     let exit_code = catch_with_exit_code(|| {
         let args = env::args_os()
             .enumerate()

src/librustdoc/lib.rs

@@ -156,15 +156,19 @@ pub fn main() {
         }
     }
 
-    rustc_driver::install_ice_hook();
+    rustc_driver::install_ice_hook(
+        "https://github.com/rust-lang/rust/issues/new\
+    ?labels=C-bug%2C+I-ICE%2C+T-rustdoc&template=ice.md",
+        |_| (),
+    );
 
-    // When using CI artifacts (with `download_stage1 = true`), tracing is unconditionally built
+    // When using CI artifacts with `download-rustc`, tracing is unconditionally built
     // with `--features=static_max_level_info`, which disables almost all rustdoc logging. To avoid
     // this, compile our own version of `tracing` that logs all levels.
     // NOTE: this compiles both versions of tracing unconditionally, because
     // - The compile time hit is not that bad, especially compared to rustdoc's incremental times, and
-    // - Otherwise, there's no warning that logging is being ignored when `download_stage1 = true`.
-    // NOTE: The reason this doesn't show double logging when `download_stage1 = false` and
+    // - Otherwise, there's no warning that logging is being ignored when `download-rustc` is enabled
+    // NOTE: The reason this doesn't show double logging when `download-rustc = false` and
     // `debug_logging = true` is because all rustc logging goes to its version of tracing (the one
     // in the sysroot), and all of rustdoc's logging goes to its version (the one in Cargo.toml).
     init_logging();

src/tools/clippy/src/driver.rs

@@ -11,7 +11,6 @@
 // FIXME: switch to something more ergonomic here, once available.
 // (Currently there is no way to opt into sysroot crates without `extern crate`.)
 extern crate rustc_driver;
-extern crate rustc_errors;
 extern crate rustc_interface;
 extern crate rustc_session;
 extern crate rustc_span;
@@ -20,13 +19,10 @@ use rustc_interface::interface;
 use rustc_session::parse::ParseSess;
 use rustc_span::symbol::Symbol;
 
-use std::borrow::Cow;
 use std::env;
 use std::ops::Deref;
-use std::panic;
 use std::path::Path;
 use std::process::exit;
-use std::sync::LazyLock;
 
 /// If a command-line option matches `find_arg`, then apply the predicate `pred` on its value. If
 /// true, then return it. The parameter is assumed to be either `--arg=value` or `--arg value`.
@@ -198,66 +194,18 @@ You can use tool lints to allow or deny lints from your code, eg.:
 
 const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust-clippy/issues/new";
 
-type PanicCallback = dyn Fn(&panic::PanicInfo<'_>) + Sync + Send + 'static;
-static ICE_HOOK: LazyLock<Box<PanicCallback>> = LazyLock::new(|| {
-    let hook = panic::take_hook();
-    panic::set_hook(Box::new(|info| report_clippy_ice(info, BUG_REPORT_URL)));
-    hook
-});
-
-fn report_clippy_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) {
-    // Invoke our ICE handler, which prints the actual panic message and optionally a backtrace
-    (*ICE_HOOK)(info);
-
-    // Separate the output with an empty line
-    eprintln!();
-
-    let fallback_bundle = rustc_errors::fallback_fluent_bundle(rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), false);
-    let emitter = Box::new(rustc_errors::emitter::EmitterWriter::stderr(
-        rustc_errors::ColorConfig::Auto,
-        None,
-        None,
-        fallback_bundle,
-        false,
-        false,
-        None,
-        false,
-        false,
-        rustc_errors::TerminalUrl::No,
-    ));
-    let handler = rustc_errors::Handler::with_emitter(true, None, emitter);
-
-    // a .span_bug or .bug call has already printed what
-    // it wants to print.
-    if !info.payload().is::<rustc_errors::ExplicitBug>() {
-        let mut d = rustc_errors::Diagnostic::new(rustc_errors::Level::Bug, "unexpected panic");
-        handler.emit_diagnostic(&mut d);
-    }
-
-    let version_info = rustc_tools_util::get_version_info!();
-
-    let xs: Vec<Cow<'static, str>> = vec![
-        "the compiler unexpectedly panicked. this is a bug.".into(),
-        format!("we would appreciate a bug report: {bug_report_url}").into(),
-        format!("Clippy version: {version_info}").into(),
-    ];
-
-    for note in &xs {
-        handler.note_without_error(note.as_ref());
-    }
-
-    // If backtraces are enabled, also print the query stack
-    let backtrace = env::var_os("RUST_BACKTRACE").map_or(false, |x| &x != "0");
-
-    let num_frames = if backtrace { None } else { Some(2) };
-
-    interface::try_print_query_stack(&handler, num_frames);
-}
-
 #[allow(clippy::too_many_lines)]
 pub fn main() {
     rustc_driver::init_rustc_env_logger();
-    LazyLock::force(&ICE_HOOK);
+
+    rustc_driver::install_ice_hook(BUG_REPORT_URL, |handler| {
+        // FIXME: this macro calls unwrap internally but is called in a panicking context!  It's not
+        // as simple as moving the call from the hook to main, because `install_ice_hook` doesn't
+        // accept a generic closure.
+        let version_info = rustc_tools_util::get_version_info!();
+        handler.note_without_error(format!("Clippy version: {version_info}"));
+    });
+
     exit(rustc_driver::catch_with_exit_code(move || {
         let mut orig_args: Vec<String> = env::args().collect();
         let has_sysroot_arg = arg_value(&orig_args, "--sysroot", |_| true).is_some();

src/tools/miri/src/bin/miri.rs

@@ -286,11 +286,10 @@ fn main() {
     // (`install_ice_hook` might change `RUST_BACKTRACE`.)
     let env_snapshot = env::vars_os().collect::<Vec<_>>();
 
-    // Earliest rustc setup.
-    rustc_driver::install_ice_hook();
-
     // If the environment asks us to actually be rustc, then do that.
     if let Some(crate_kind) = env::var_os("MIRI_BE_RUSTC") {
+        // Earliest rustc setup.
+        rustc_driver::install_ice_hook(rustc_driver::DEFAULT_BUG_REPORT_URL, |_| ());
         rustc_driver::init_rustc_env_logger();
 
         let target_crate = if crate_kind == "target" {
@@ -309,6 +308,9 @@ fn main() {
         )
     }
 
+    // Add an ICE bug report hook.
+    rustc_driver::install_ice_hook("https://github.com/rust-lang/miri/issues/new", |_| ());
+
     // Init loggers the Miri way.
     init_early_loggers();
 

src/tools/rustfmt/src/bin/main.rs

@@ -1,3 +1,5 @@
+#![feature(rustc_private)]
+
 use anyhow::{format_err, Result};
 
 use io::Error as IoError;
@@ -19,7 +21,14 @@ use crate::rustfmt::{
     FormatReportFormatterBuilder, Input, Session, Verbosity,
 };
 
+const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rustfmt/issues/new?labels=bug";
+
+// N.B. these crates are loaded from the sysroot, so they need extern crate.
+extern crate rustc_driver;
+
 fn main() {
+    rustc_driver::install_ice_hook(BUG_REPORT_URL, |_| ());
+
     env_logger::Builder::from_env("RUSTFMT_LOG").init();
     let opts = make_opts();
 

tests/rustdoc-ui/ice-bug-report-url.rs

@@ -0,0 +1,14 @@
+// compile-flags: -Ztreat-err-as-bug
+// failure-status: 101
+// error-pattern: aborting due to
+// error-pattern: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-rustdoc&template=ice.md
+
+// normalize-stderr-test "note: compiler flags.*\n\n" -> ""
+// normalize-stderr-test "note: rustc.*running on.*" -> "note: rustc {version} running on {platform}"
+// normalize-stderr-test "thread.*panicked at .*, compiler.*" -> "thread panicked at 'aborting due to `-Z treat-err-as-bug`'"
+// normalize-stderr-test "\s*\d{1,}: .*\n" -> ""
+// normalize-stderr-test "\s at .*\n" -> ""
+// normalize-stderr-test ".*note: Some details are omitted.*\n" -> ""
+
+fn wrong()
+//~^ ERROR expected one of

tests/rustdoc-ui/ice-bug-report-url.stderr

@@ -0,0 +1,16 @@
+error: expected one of `->`, `where`, or `{`, found `<eof>`
+  --> $DIR/ice-bug-report-url.rs:13:10
+   |
+LL | fn wrong()
+   |          ^ expected one of `->`, `where`, or `{`
+
+thread panicked at 'aborting due to `-Z treat-err-as-bug`'
+stack backtrace:
+error: the compiler unexpectedly panicked. this is a bug.
+
+note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-rustdoc&template=ice.md
+
+note: rustc {version} running on {platform}
+
+query stack during panic:
+end of query stack