Skip to content

Commit 1cec012

Browse files
committed
improve panic message on expect
This is something that has been bothering me for years, which (naively) seems simple to fix. The message passed to `.expect(msg)` is printed in (at least potentially) confusing way to the user: ``` let x: Option<u32> = None; x.expect("valid x value"); // thread 'main' panicked at ...: // valid x value ``` IMO, it would be much better if it got reported as: ``` // thread 'main' panicked at ...: // expected: valid x value ``` Then, no matter how the message is formulated, it should be clear that it was the *expectation*, not the failing condition itself.
1 parent 2c12b4a commit 1cec012

File tree

2 files changed

+29
-11
lines changed

2 files changed

+29
-11
lines changed

library/core/src/option.rs

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,7 @@
573573
574574
#![stable(feature = "rust1", since = "1.0.0")]
575575

576+
use crate::fmt;
576577
use crate::iter::{self, FusedIterator, TrustedLen};
577578
use crate::ops::{self, ControlFlow, Deref, DerefMut};
578579
use crate::panicking::{panic, panic_display};
@@ -955,7 +956,7 @@ impl<T> Option<T> {
955956
pub const fn expect(self, msg: &str) -> T {
956957
match self {
957958
Some(val) => val,
958-
None => expect_failed(msg),
959+
None => expect_failed("expected: ", msg),
959960
}
960961
}
961962

@@ -1783,7 +1784,11 @@ impl<T> Option<T> {
17831784
where
17841785
P: FnOnce(&mut T) -> bool,
17851786
{
1786-
if self.as_mut().map_or(false, predicate) { self.take() } else { None }
1787+
if self.as_mut().map_or(false, predicate) {
1788+
self.take()
1789+
} else {
1790+
None
1791+
}
17871792
}
17881793

17891794
/// Replaces the actual value in the option by the value given in parameter,
@@ -2045,8 +2050,21 @@ const fn unwrap_failed() -> ! {
20452050
#[cfg_attr(feature = "panic_immediate_abort", inline)]
20462051
#[cold]
20472052
#[track_caller]
2048-
const fn expect_failed(msg: &str) -> ! {
2049-
panic_display(&msg)
2053+
const fn expect_failed(prefix: &str, msg: &str) -> ! {
2054+
// TODO: FIXME: there seem to be no way currently to print anything other than a single `&str`
2055+
// in `panic` in `const` context.
2056+
struct MsgWithPrefix<'a> {
2057+
prefix: &'a str,
2058+
msg: &'a str,
2059+
}
2060+
2061+
impl<'a> fmt::Display for MsgWithPrefix<'a> {
2062+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
2063+
f.write_str(self.prefix)?;
2064+
f.write_str(self.msg)
2065+
}
2066+
}
2067+
panic_display(&MsgWithPrefix { prefix, msg })
20502068
}
20512069

20522070
/////////////////////////////////////////////////////////////////////////////

library/core/src/result.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,7 +1086,7 @@ impl<T, E> Result<T, E> {
10861086
{
10871087
match self {
10881088
Ok(t) => t,
1089-
Err(e) => unwrap_failed(msg, &e),
1089+
Err(e) => unwrap_failed("expected: ", msg, &e),
10901090
}
10911091
}
10921092

@@ -1134,7 +1134,7 @@ impl<T, E> Result<T, E> {
11341134
{
11351135
match self {
11361136
Ok(t) => t,
1137-
Err(e) => unwrap_failed("called `Result::unwrap()` on an `Err` value", &e),
1137+
Err(e) => unwrap_failed("", "called `Result::unwrap()` on an `Err` value", &e),
11381138
}
11391139
}
11401140

@@ -1197,7 +1197,7 @@ impl<T, E> Result<T, E> {
11971197
T: fmt::Debug,
11981198
{
11991199
match self {
1200-
Ok(t) => unwrap_failed(msg, &t),
1200+
Ok(t) => unwrap_failed("expected error: ", msg, &t),
12011201
Err(e) => e,
12021202
}
12031203
}
@@ -1228,7 +1228,7 @@ impl<T, E> Result<T, E> {
12281228
T: fmt::Debug,
12291229
{
12301230
match self {
1231-
Ok(t) => unwrap_failed("called `Result::unwrap_err()` on an `Ok` value", &t),
1231+
Ok(t) => unwrap_failed("", "called `Result::unwrap_err()` on an `Ok` value", &t),
12321232
Err(e) => e,
12331233
}
12341234
}
@@ -1728,8 +1728,8 @@ impl<T, E> Result<Result<T, E>, E> {
17281728
#[inline(never)]
17291729
#[cold]
17301730
#[track_caller]
1731-
fn unwrap_failed(msg: &str, error: &dyn fmt::Debug) -> ! {
1732-
panic!("{msg}: {error:?}")
1731+
fn unwrap_failed(prefix: &str, msg: &str, error: &dyn fmt::Debug) -> ! {
1732+
panic!("{prefix}{msg}: {error:?}")
17331733
}
17341734

17351735
// This is a separate function to avoid constructing a `dyn Debug`
@@ -1740,7 +1740,7 @@ fn unwrap_failed(msg: &str, error: &dyn fmt::Debug) -> ! {
17401740
#[inline]
17411741
#[cold]
17421742
#[track_caller]
1743-
fn unwrap_failed<T>(_msg: &str, _error: &T) -> ! {
1743+
fn unwrap_failed<T>(_prefix: &str, _msg: &str, _error: &T) -> ! {
17441744
panic!()
17451745
}
17461746

0 commit comments

Comments
 (0)