From 9ad005b4f229a3b6d1f72ac4936f3a28a10575b3 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 5 Sep 2023 09:24:54 +0000 Subject: [PATCH 1/3] Make `PanicInfo::message` infallible --- library/core/src/panic/panic_info.rs | 15 +++++---------- library/core/src/panicking.rs | 4 ++-- library/std/src/lib.rs | 1 + library/std/src/panicking.rs | 8 +++++--- .../wasm-exceptions-nostd/src/panicking.rs | 7 ++----- tests/ui/panics/panic_info_message.rs | 11 +++++++++++ 6 files changed, 26 insertions(+), 20 deletions(-) create mode 100644 tests/ui/panics/panic_info_message.rs diff --git a/library/core/src/panic/panic_info.rs b/library/core/src/panic/panic_info.rs index c77e9675a6ac5..7d719600afa7f 100644 --- a/library/core/src/panic/panic_info.rs +++ b/library/core/src/panic/panic_info.rs @@ -25,7 +25,7 @@ use crate::panic::Location; #[derive(Debug)] pub struct PanicInfo<'a> { payload: &'a (dyn Any + Send), - message: Option<&'a fmt::Arguments<'a>>, + message: &'a fmt::Arguments<'a>, location: &'a Location<'a>, can_unwind: bool, force_no_backtrace: bool, @@ -40,7 +40,7 @@ impl<'a> PanicInfo<'a> { #[doc(hidden)] #[inline] pub fn internal_constructor( - message: Option<&'a fmt::Arguments<'a>>, + message: &'a fmt::Arguments<'a>, location: &'a Location<'a>, can_unwind: bool, force_no_backtrace: bool, @@ -92,7 +92,7 @@ impl<'a> PanicInfo<'a> { /// returns that message ready to be used for example with [`fmt::write`] #[must_use] #[unstable(feature = "panic_info_message", issue = "66745")] - pub fn message(&self) -> Option<&fmt::Arguments<'_>> { + pub fn message(&self) -> &fmt::Arguments<'_> { self.message } @@ -161,13 +161,8 @@ impl fmt::Display for PanicInfo<'_> { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { formatter.write_str("panicked at ")?; self.location.fmt(formatter)?; - if let Some(message) = self.message { - formatter.write_str(":\n")?; - formatter.write_fmt(*message)?; - } else if let Some(payload) = self.payload.downcast_ref::<&'static str>() { - formatter.write_str(":\n")?; - formatter.write_str(payload)?; - } + formatter.write_str(":\n")?; + formatter.write_fmt(*self.message)?; // NOTE: we cannot use downcast_ref::() here // since String is not available in core! // The payload is a String when `std::panic!` is called with multiple arguments, diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs index 281f8c1e1663c..059a5ccfe2285 100644 --- a/library/core/src/panicking.rs +++ b/library/core/src/panicking.rs @@ -62,7 +62,7 @@ pub const fn panic_fmt(fmt: fmt::Arguments<'_>) -> ! { } let pi = PanicInfo::internal_constructor( - Some(&fmt), + &fmt, Location::caller(), /* can_unwind */ true, /* force_no_backtrace */ false, @@ -96,7 +96,7 @@ pub fn panic_nounwind_fmt(fmt: fmt::Arguments<'_>, force_no_backtrace: bool) -> // PanicInfo with the `can_unwind` flag set to false forces an abort. let pi = PanicInfo::internal_constructor( - Some(&fmt), + &fmt, Location::caller(), /* can_unwind */ false, force_no_backtrace, diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 1955ef815ff80..fdfc7d556e3be 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -292,6 +292,7 @@ #![feature(float_gamma)] #![feature(float_minimum_maximum)] #![feature(float_next_up_down)] +#![feature(fmt_internals)] #![feature(hasher_prefixfree_extras)] #![feature(hashmap_internals)] #![feature(ip)] diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index b20e5464e6e01..f6b70a4395feb 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -615,7 +615,7 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { } let loc = info.location().unwrap(); // The current implementation always returns Some - let msg = info.message().unwrap(); // The current implementation always returns Some + let msg = info.message(); // The current implementation always returns Some crate::sys_common::backtrace::__rust_end_short_backtrace(move || { // FIXME: can we just pass `info` along rather than taking it apart here, only to have // `rust_panic_with_hook` construct a new `PanicInfo`? @@ -658,9 +658,11 @@ pub const fn begin_panic(msg: M) -> ! { let loc = Location::caller(); return crate::sys_common::backtrace::__rust_end_short_backtrace(move || { + let message = + *(&msg as &dyn Any).downcast_ref::<&'static str>().unwrap_or(&""); rust_panic_with_hook( &mut PanicPayload::new(msg), - None, + &core::fmt::Arguments::new_v1(&[message], &[]), loc, /* can_unwind */ true, /* force_no_backtrace */ false, @@ -707,7 +709,7 @@ pub const fn begin_panic(msg: M) -> ! { /// abort or unwind. fn rust_panic_with_hook( payload: &mut dyn BoxMeUp, - message: Option<&fmt::Arguments<'_>>, + message: &fmt::Arguments<'_>, location: &Location<'_>, can_unwind: bool, force_no_backtrace: bool, diff --git a/tests/run-make/wasm-exceptions-nostd/src/panicking.rs b/tests/run-make/wasm-exceptions-nostd/src/panicking.rs index 4a8923fd43db6..414c9f6a1650b 100644 --- a/tests/run-make/wasm-exceptions-nostd/src/panicking.rs +++ b/tests/run-make/wasm-exceptions-nostd/src/panicking.rs @@ -17,11 +17,8 @@ fn panic_handler(info: &core::panic::PanicInfo<'_>) -> ! { use alloc::boxed::Box; use alloc::string::ToString; - let msg = info - .message() - .map(|msg| msg.to_string()) - .unwrap_or("(no message)".to_string()); - let exception = Box::new(msg.to_string()); + let msg = info.message().to_string(); + let exception = Box::new(msg); unsafe { let exception_raw = Box::into_raw(exception); wasm_throw(exception_raw as *mut u8); diff --git a/tests/ui/panics/panic_info_message.rs b/tests/ui/panics/panic_info_message.rs new file mode 100644 index 0000000000000..686d8fda1f0cf --- /dev/null +++ b/tests/ui/panics/panic_info_message.rs @@ -0,0 +1,11 @@ +// revisions: e2015 e2018 e2021 +//[e2018] compile-flags: --edition=2018 +//[e2021] compile-flags: --edition=2021 +// run-pass + +#![feature(panic_info_message)] + +fn main() { + std::panic::set_hook(Box::new(|info| assert_eq!(info.message().as_str().unwrap(), "cake"))); + let _ = std::panic::catch_unwind(|| panic!("cake")); +} From df19b2941609002a3e4e4459d3cc14c4fe3620cd Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 5 Sep 2023 09:27:59 +0000 Subject: [PATCH 2/3] Remove an unnecessary indirection --- library/core/src/panic/panic_info.rs | 8 ++++---- library/core/src/panicking.rs | 4 ++-- library/std/src/panicking.rs | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/library/core/src/panic/panic_info.rs b/library/core/src/panic/panic_info.rs index 7d719600afa7f..15e85ddbd793e 100644 --- a/library/core/src/panic/panic_info.rs +++ b/library/core/src/panic/panic_info.rs @@ -25,7 +25,7 @@ use crate::panic::Location; #[derive(Debug)] pub struct PanicInfo<'a> { payload: &'a (dyn Any + Send), - message: &'a fmt::Arguments<'a>, + message: fmt::Arguments<'a>, location: &'a Location<'a>, can_unwind: bool, force_no_backtrace: bool, @@ -40,7 +40,7 @@ impl<'a> PanicInfo<'a> { #[doc(hidden)] #[inline] pub fn internal_constructor( - message: &'a fmt::Arguments<'a>, + message: fmt::Arguments<'a>, location: &'a Location<'a>, can_unwind: bool, force_no_backtrace: bool, @@ -92,7 +92,7 @@ impl<'a> PanicInfo<'a> { /// returns that message ready to be used for example with [`fmt::write`] #[must_use] #[unstable(feature = "panic_info_message", issue = "66745")] - pub fn message(&self) -> &fmt::Arguments<'_> { + pub fn message(&self) -> fmt::Arguments<'_> { self.message } @@ -162,7 +162,7 @@ impl fmt::Display for PanicInfo<'_> { formatter.write_str("panicked at ")?; self.location.fmt(formatter)?; formatter.write_str(":\n")?; - formatter.write_fmt(*self.message)?; + formatter.write_fmt(self.message)?; // NOTE: we cannot use downcast_ref::() here // since String is not available in core! // The payload is a String when `std::panic!` is called with multiple arguments, diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs index 059a5ccfe2285..15cd547a6aba8 100644 --- a/library/core/src/panicking.rs +++ b/library/core/src/panicking.rs @@ -62,7 +62,7 @@ pub const fn panic_fmt(fmt: fmt::Arguments<'_>) -> ! { } let pi = PanicInfo::internal_constructor( - &fmt, + fmt, Location::caller(), /* can_unwind */ true, /* force_no_backtrace */ false, @@ -96,7 +96,7 @@ pub fn panic_nounwind_fmt(fmt: fmt::Arguments<'_>, force_no_backtrace: bool) -> // PanicInfo with the `can_unwind` flag set to false forces an abort. let pi = PanicInfo::internal_constructor( - &fmt, + fmt, Location::caller(), /* can_unwind */ false, force_no_backtrace, diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index f6b70a4395feb..c9f2cf2010f11 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -629,7 +629,7 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { ); } else { rust_panic_with_hook( - &mut PanicPayload::new(msg), + &mut PanicPayload::new(&msg), info.message(), loc, info.can_unwind(), @@ -662,7 +662,7 @@ pub const fn begin_panic(msg: M) -> ! { *(&msg as &dyn Any).downcast_ref::<&'static str>().unwrap_or(&""); rust_panic_with_hook( &mut PanicPayload::new(msg), - &core::fmt::Arguments::new_v1(&[message], &[]), + core::fmt::Arguments::new_v1(&[message], &[]), loc, /* can_unwind */ true, /* force_no_backtrace */ false, @@ -709,7 +709,7 @@ pub const fn begin_panic(msg: M) -> ! { /// abort or unwind. fn rust_panic_with_hook( payload: &mut dyn BoxMeUp, - message: &fmt::Arguments<'_>, + message: fmt::Arguments<'_>, location: &Location<'_>, can_unwind: bool, force_no_backtrace: bool, From 6042f5275c72a23bfbf3a235a47ae8dc0eb355d6 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 5 Sep 2023 15:58:49 +0000 Subject: [PATCH 3/3] Remove now-outdated comments --- library/std/src/panicking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index c9f2cf2010f11..81c0858beba52 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -615,7 +615,7 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { } let loc = info.location().unwrap(); // The current implementation always returns Some - let msg = info.message(); // The current implementation always returns Some + let msg = info.message(); crate::sys_common::backtrace::__rust_end_short_backtrace(move || { // FIXME: can we just pass `info` along rather than taking it apart here, only to have // `rust_panic_with_hook` construct a new `PanicInfo`?