panic-in-panic-hook: formatting a message that's just a string is risk-free

[RalfJung]

This slightly improves the output in the 'panic while processing panic' case if the panic message does not involve any formatting. Follow-up to #122930.

r? @Amanieu

[RalfJung]

I could use the same logic as above in this branch as well, which would fix #122940 but degrade panic printing for post-fork panics that do need formatting. Let me know what you think.

[RalfJung]

@Amanieu any thoughts on this?

[Amanieu]

I think #122940 is better fixed with an additional flag on the global count.

[Amanieu]

@bors r+

[bors]

📌 Commit 0727b6a has been approved by Amanieu

It is now in the queue for this repository.

[correabuscar]

For some reason I thought that this would show the message as unformatted, not just when it contains no formatting args.
ie. I thought it shows both cases:

• panic!("some unformatted string"); would show as some unformatted string
• panic!("some formatted string i={} j={}", i, j); would show as some formatted string i={} j={} (literally)

but the latter doesn't show at all(which is indeed intended to be so by this PR, no problem),
therefore for my local rust copy that I'm gonna use, I had to change it and make it do that second case too(well, almost that: the {} won't be visible, it's acceptable compromise - I don't really need to see those, given that it would only complicate the code used to show them) by replacing lines 751 and 752 from above with:

let message=message.map(|m| m.unformat_it());

Index: /var/tmp/portage/dev-lang/rust-1.75.0-r1/work/rustc-1.75.0-src/library/core/src/fmt/mod.rs
===================================================================
--- .orig/var/tmp/portage/dev-lang/rust-1.75.0-r1/work/rustc-1.75.0-src/library/core/src/fmt/mod.rs
+++ /var/tmp/portage/dev-lang/rust-1.75.0-r1/work/rustc-1.75.0-src/library/core/src/fmt/mod.rs
@@ -352,6 +352,20 @@ impl<'a> Arguments<'a> {
         Arguments { pieces, fmt: Some(fmt), args }
     }
 
+    /// just return the unformatted string
+    /// so this:
+    /// "some formatted string i={} j={}"
+    /// would be seen as:
+    /// "some formatted string i= j="
+    /// because the {}s get removed and transformed into the args array.
+    /// this assumes `pub fn write` was already patched to handle the extra pieces,
+    /// else you only see the first piece like:
+    /// "some formatted string i="
+    #[inline]
+    pub fn unformat_it(&self) -> Arguments<'a> {
+        Arguments { pieces:self.pieces, fmt: None, args: &[] }
+    }
+
     /// Estimates the length of the formatted text.
     ///
     /// This is intended to be used for setting initial `String` capacity
@@ -1140,10 +1154,23 @@ pub fn write(output: &mut dyn Write, arg
     }
 
     // There can be only one trailing string piece left.
-    if let Some(piece) = args.pieces.get(idx) {
-        formatter.buf.write_str(*piece)?;
+    //if let Some(piece) = args.pieces.get(idx) {
+    //    formatter.buf.write_str(*piece)?;
+    //}
+    //Maybe there's a ton of pieces*, that had fmt: None and args: &[]
+    //and we wanna print them all, yes they'll be unformatted like due to .unformat_it() function
+    //from Arguments struct.
+    // * not just the one trailing string piece left.
+    if idx < args.pieces.len() {
+        // Iterate over the remaining string pieces starting from idx
+        for piece in &args.pieces[idx..] {
+            // Write each piece to the formatter buffer
+            formatter.buf.write_str(piece)?;
+        }
     }
 
+
+
     Ok(())
 }
 

I'm just saying this in case it may be useful to anyone for any reason. Certainly I don't expect any such change(s) to be incorporated into rust as it seems hacky or unnecessary.

---

Doing this however has uncovered an issue(playground link) whereby the following message from within .expect isn't shown (even before this PR, ie. rust stable):

use std::fmt::{Display, self};

struct MyStruct;

impl Display for MyStruct {
    fn fmt(&self, _: &mut fmt::Formatter<'_>) -> fmt::Result {
        None::<i32>.expect("oh snap");
        todo!();//ignore this, it's for return
    }
}

fn main() {
    
        let instance = MyStruct;

        assert!(false, "oh no, '{}' was unexpected", instance);
   
}

   Compiling playground v0.0.1 (/playground)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.66s
     Running `target/debug/playground`
panicked at src/main.rs:7:21:
thread panicked while processing panic. aborting.

feel free(anyone) to make an issue out of it, I'm just "busy" exploring how to fix it locally. Note that in this PR, message is seen as being None after the two lines that transform it, this is why it's not shown, however as to why it's None after when it's clearly shown initially to be there when displayed (as {:?}(Debug is same as Display) shows as Some(oh snap)) I'm uncertain yet (in progress, might update this later)

[RalfJung]

Doing this however has uncovered an issue(playground link) whereby the following message from within .expect isn't shown (even before this PR, ie. rust stable):

That's expected, the panic here uses formatting. It goes via this code path.

My PR is only expected to show the panic message for literal panic!("text here"), not for anything that involves {} in any way.

It will show in some extra cases due to this optimization, but that's not guaranteed.

[RalfJung]

you mean that None::.expect("oh snap"); uses formatting internally?

Yes, it does.