Tracking issue for PanicInfo::message

#44489 was closed when #51366 stabilized the corresponding language feature, but we didn’t realize that it was also the tracking issue for this standard library feature:

// In `core::panic`:

impl PanicInfo<'_> {
    /// The message that was given to the `panic!` macro.
    pub fn message(&self) -> PanicMessage<'_>;
}

// Opaque type that wraps a fmt::Arguments<'a>.
pub struct PanicMessage<'a> { .. }

impl Display for PanicMessage<'_>;
impl Debug for PanicMessage<'_>;

impl PanicMessage<'_> {
    pub fn as_str(&self) -> Option<&'static str>;
}

History:

Original tracking issue: Tracking issue for RFC 2070: stable mechanism to specify the behavior of panic! in no-std applications #44489
Discussion of payload vs message Tracking issue for PanicInfo::message #66745 (comment)
Making .message() infallible: Make PanicInfo::message infallible #115561
Split core/std PanicInfo into two types: Split core's PanicInfo and std's PanicInfo #115974
- This made core::panic::PanicInfo::message() infallible.
Use opaque return type: Return opaque type from PanicInfo::message() #126330
FCP
Stablization PR: Stabilize PanicInfo::message() and PanicMessage #126732

Unresolved questions:

Should it be infallible?
- Yes, by making std::panic::PanicHookInfo a different type.
What should the return type be?
- fmt::Arguments or an opaque PanicMessage? The first is simpler, but the latter allows a bit more flexibility for future changes.

@rfcbot

Instead of making a PR go through the queue to fix the tracking issue, should we stabilize this method?

@rfcbot fcp merge

The doc-comment is slightly wrong or out of date. Its second line shoudl be removed. core::panic! always creates a PanicInfo where message is Some, regardless of the number of arguments given. In the single-argument case, that argument is required to have type &str.

At first returning fmt::Arguments seemed unusual to me and I considered that we could make the method return Option<impl Display + 'a> instead, on the basis that calling its Display impl is almost the only useful thing to do with a Arguments. But Arguments has been stable and documented as the return type of format_args!(…) since 1.0.0, so maybe it’s fine.

The core::panic module also links to that closed tracking issue. Let’s consider the pFCP above to also include stabilizing it. (std::panic is stable.)

@rfcbot

@rfcbot concern single-arg core::panic

Should we change libcore’s panic macro so that its single-argument form creates a PanicInfo with a payload instead of one with a message? CC #66740 (comment)

Pushing this idea further, it’s tempting to maybe remove PanicInfo::message entirely and instead have a payload whose dynamic type can be fmt::Arguments. But that doesn’t work because downcast_ref is a method of dyn Any + 'static and fmt::Arguments<'_> is not 'static.

I thought the point of the message field was that core can't make a payload since it can't allocate?

The struct is:

pub struct PanicInfo<'a> {
    payload: &'a (dyn Any + Send),
    message: Option<&'a fmt::Arguments<'a>>,
    location: &'a Location<'a>,
}

Both the payload and message fields involve borrowing. As far as PanicInfo is concerned, they could both be borrowing form a stack frame. (And as far as I understand they are, in practice today.)

It’s in the case of unwinding that there’s an owned Box<dyn Any + Send> that catch_unwind can return, which requires allocating.

@RalfJung

@RalfJung Based on the timing of https://www.ralfj.de/blog/2019/11/25/how-to-panic-in-rust.html it looks like you may have thoughts on this :)

Ah, https://www.ralfj.de/blog/2019/11/25/how-to-panic-in-rust.html goes into how and why the panic handler in libstd (which is called by core::panic! if libstd is linked) ignores the payload it receives.

We could probably change this but it gets tricky: while most implementations of #[panic_handler] for no_std environments would be fine with receiving a borrowed payload, libstd wants to potentially take ownership and box it for unwinding.

I have zero recollection for why this is the way it is, and I'm not really particularly interested in doing all the archaeology to dig up why it is the way it is. I'm not a huge fan of just stabilizing things because they happen to be there, but I don't really see why we wouldn't want to stabilize this. If there were a reason to not stabilize this though an investigation would presumably turn that up, but I'm not up to do that investigation myself.

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

Rollup merge of rust-lang#126732 - StackOverflowExcept1on:master, r=m…

Unrolled build for rust-lang#126732

Update platform010 & platform010-aarch64 symlinks

wasm: fix PanicMessage following rust-lang/rust#66745

Navigation Menu

Repository navigation

Tracking issue for PanicInfo::message #66745

40 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Tracking issue for PanicInfo::message #66745

Description

Activity

SimonSapin commented on Nov 25, 2019

rfcbot commented on Nov 25, 2019

SimonSapin commented on Nov 25, 2019

SimonSapin commented on Nov 25, 2019

sfackler commented on Nov 25, 2019

SimonSapin commented on Nov 25, 2019

SimonSapin commented on Nov 25, 2019

SimonSapin commented on Nov 25, 2019

alexcrichton commented on Nov 25, 2019

40 remaining items

rfcbot commented on Jun 29, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions