`format!`/`to_string` does not panic on `std::fmt::Error`; intentional or not?

use std::fmt;

struct S;
impl fmt::Display for S {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        f.write_str("S");
        Err(fmt::Error)
    }
}

fn main() {
    let a: String = format!("{}", S);
    println!("{}", a);
}

…prints S and exits normally. (Same for ToString::to_string, they share the same implementation.)

I was very surprised as I thought this will panic (possibly after printing S, as this is hard to revert). I discovered this behavior while auditing Chrono’s formatting error semantics (#47) and I was unable to find a rationale for this. I did pinpoint the commit by @alexcrichton that introduced this change but that seems an unconscious change; can anyone confirm or deny my doubt?

1 Like

It won’t panic. The result is checked inside the core::fmt::write function, and will break the loop of writing arguments if there’s an error. The entire point of it is to handle if the Write target is an io::Write that returns an io::Error. Instead of trying to write more, it stops.

I’m not sure I’m following your words, but write! is much different from format! in that the former returns the error while the latter doesn’t. Also, println! would try to ignore the error by your logic, but it actually panics when stdout().write_fmt(args) fails (at least in the current implementation).

I probably should have include a link. I meant the write function the core::fmt module, that runs the formatter.

The 'try!'s will cause the loops to break if there’s an io::Error.

1 Like

It was a conscious decision to ignore the error in to_string because it was originally thought that std::fmt::Error would only be generated from the writer itself which the one used by to_string never generates. I believe there may have also been some performance and/or binary size wins by not having a check/panic in that case.

Idiomatically I believe that no types should return std::fmt::Error for a case such as “I can’t be formatted right now”, so I would personally chalk this up to “a misbehaving Display impl”, but I could also see this possibly changing.

2 Likes

Thank you. I was unable to find any documented rationale for that and a bit wary of potential problems when it were unintentional. Chrono probably needs to be adjusted to take account for that :slight_smile:

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.