Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

io::Write::write_fmt: panic if the formatter fails when the stream does not fail #125012

Merged
merged 1 commit into from
May 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion library/alloc/src/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,9 @@ pub fn format(args: Arguments<'_>) -> string::String {
fn format_inner(args: Arguments<'_>) -> string::String {
let capacity = args.estimated_capacity();
let mut output = string::String::with_capacity(capacity);
output.write_fmt(args).expect("a formatting trait implementation returned an error");
output
.write_fmt(args)
.expect("a formatting trait implementation returned an error when the underlying stream did not");
output
}

Expand Down
6 changes: 5 additions & 1 deletion library/std/src/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1839,7 +1839,11 @@ pub trait Write {
if output.error.is_err() {
output.error
} else {
Err(error::const_io_error!(ErrorKind::Uncategorized, "formatter error"))
// This shouldn't happen: the underlying stream did not error, but somehow
// the formatter still errored?
panic!(
"a formatting trait implementation returned an error when the underlying stream did not"
);
}
}
}
Expand Down
29 changes: 18 additions & 11 deletions tests/ui/write-fmt-errors.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
//@ run-pass
//@ needs-unwind
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make it a should_panic unit test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I'd have to split it in 3 tests -- the parts that shouldn't panic, and the two panics.
I'd rather not do that.


#![feature(io_error_uncategorized)]

use std::fmt;
use std::io::{self, Error, Write, sink};
use std::panic::catch_unwind;

struct ErrorDisplay;

Expand All @@ -15,7 +17,6 @@ impl fmt::Display for ErrorDisplay {

struct ErrorWriter;

const FORMAT_ERROR: io::ErrorKind = io::ErrorKind::Uncategorized;
const WRITER_ERROR: io::ErrorKind = io::ErrorKind::NotConnected;

impl Write for ErrorWriter {
Expand All @@ -27,22 +28,28 @@ impl Write for ErrorWriter {
}

fn main() {
// Test that the error from the formatter is propagated.
let res = write!(sink(), "{} {} {}", 1, ErrorDisplay, "bar");
assert!(res.is_err(), "formatter error did not propagate");
assert_eq!(res.unwrap_err().kind(), FORMAT_ERROR);

// Test that an underlying error is propagated
let res = write!(ErrorWriter, "abc");
assert!(res.is_err(), "writer error did not propagate");

// Writer error
// Test that the error from the formatter is detected.
let res = catch_unwind(|| write!(sink(), "{} {} {}", 1, ErrorDisplay, "bar"));
let err = res.expect_err("formatter error did not lead to panic").downcast::<&str>().unwrap();
assert!(
err.contains("formatting trait implementation returned an error"),
"unexpected panic: {}", err
);

// Writer error when there's some string before the first `{}`
let res = write!(ErrorWriter, "abc {}", ErrorDisplay);
assert!(res.is_err(), "writer error did not propagate");
assert_eq!(res.unwrap_err().kind(), WRITER_ERROR);

// Formatter error
let res = write!(ErrorWriter, "{} abc", ErrorDisplay);
assert!(res.is_err(), "formatter error did not propagate");
assert_eq!(res.unwrap_err().kind(), FORMAT_ERROR);
// Formatter error when the `{}` comes first
let res = catch_unwind(|| write!(ErrorWriter, "{} abc", ErrorDisplay));
let err = res.expect_err("formatter error did not lead to panic").downcast::<&str>().unwrap();
assert!(
err.contains("formatting trait implementation returned an error"),
"unexpected panic: {}", err
);
}
Loading