Skip to content

Commit

Permalink
Rollup merge of #110721 - lukas-code:panic-fmt, r=Amanieu
Browse files Browse the repository at this point in the history
format panic message only once

For `panic!` and friends, the `std` panic runtime will always set the `.payload()` of `PanicInfo` to the formatted string. The linked issues show that formatting the message twice can cause problems, so we simply print the already formatted message instead of formatting it again. We can't remove the preformatted payload, because it can be observed by custom panic hooks.

fixes #110717
fixes rust-itertools/itertools#694

cc ```@Amanieu``` who broke this in #109507
  • Loading branch information
matthiaskrgr committed Apr 24, 2023
2 parents 1b0c552 + 7410960 commit 13e8f09
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 13 deletions.
26 changes: 13 additions & 13 deletions library/std/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,20 +249,20 @@ fn default_hook(info: &PanicInfo<'_>) {
let name = thread.as_ref().and_then(|t| t.name()).unwrap_or("<unnamed>");

let write = |err: &mut dyn crate::io::Write| {
// Use the panic message directly if available, otherwise take it from
// the payload.
if let Some(msg) = info.message() {
let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}");
// The std panic runtime always sets a `&str` or `String` payload for `panic!` and related
// macros with the formatted message.
// We try using the payload first to avoid formatting the message twice.
let msg: &dyn fmt::Display = if let Some(s) = info.payload().downcast_ref::<&'static str>()
{
s
} else if let Some(s) = info.payload().downcast_ref::<String>() {
s
} else if let Some(msg) = info.message() {
msg
} else {
let msg = if let Some(s) = info.payload().downcast_ref::<&'static str>() {
*s
} else if let Some(s) = info.payload().downcast_ref::<String>() {
&s[..]
} else {
"Box<dyn Any>"
};
let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}");
}
&"Box<dyn Any>"
};
let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}");

static FIRST_PANIC: AtomicBool = AtomicBool::new(true);

Expand Down
21 changes: 21 additions & 0 deletions tests/ui/panics/fmt-only-once.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// run-fail
// check-run-results
// exec-env:RUST_BACKTRACE=0

// Test that we format the panic message only once.
// Regression test for https://github.com/rust-lang/rust/issues/110717

use std::fmt;

struct PrintOnFmt;

impl fmt::Display for PrintOnFmt {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
eprintln!("fmt");
f.write_str("PrintOnFmt")
}
}

fn main() {
panic!("{}", PrintOnFmt)
}
3 changes: 3 additions & 0 deletions tests/ui/panics/fmt-only-once.run.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fmt
thread 'main' panicked at 'PrintOnFmt', $DIR/fmt-only-once.rs:20:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

0 comments on commit 13e8f09

Please sign in to comment.