From 1cec0121b8996c3507865e4c09a27a4751d40a65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20Ci=C4=99=C5=BCarkiewicz?= Date: Sat, 17 May 2025 11:02:02 -0700 Subject: [PATCH] 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 = 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. --- library/core/src/option.rs | 26 ++++++++++++++++++++++---- library/core/src/result.rs | 14 +++++++------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/library/core/src/option.rs b/library/core/src/option.rs index aed5a043c11a3..3cce3b788612c 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -573,6 +573,7 @@ #![stable(feature = "rust1", since = "1.0.0")] +use crate::fmt; use crate::iter::{self, FusedIterator, TrustedLen}; use crate::ops::{self, ControlFlow, Deref, DerefMut}; use crate::panicking::{panic, panic_display}; @@ -955,7 +956,7 @@ impl Option { pub const fn expect(self, msg: &str) -> T { match self { Some(val) => val, - None => expect_failed(msg), + None => expect_failed("expected: ", msg), } } @@ -1783,7 +1784,11 @@ impl Option { where P: FnOnce(&mut T) -> bool, { - if self.as_mut().map_or(false, predicate) { self.take() } else { None } + if self.as_mut().map_or(false, predicate) { + self.take() + } else { + None + } } /// Replaces the actual value in the option by the value given in parameter, @@ -2045,8 +2050,21 @@ const fn unwrap_failed() -> ! { #[cfg_attr(feature = "panic_immediate_abort", inline)] #[cold] #[track_caller] -const fn expect_failed(msg: &str) -> ! { - panic_display(&msg) +const fn expect_failed(prefix: &str, msg: &str) -> ! { + // TODO: FIXME: there seem to be no way currently to print anything other than a single `&str` + // in `panic` in `const` context. + struct MsgWithPrefix<'a> { + prefix: &'a str, + msg: &'a str, + } + + impl<'a> fmt::Display for MsgWithPrefix<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.prefix)?; + f.write_str(self.msg) + } + } + panic_display(&MsgWithPrefix { prefix, msg }) } ///////////////////////////////////////////////////////////////////////////// diff --git a/library/core/src/result.rs b/library/core/src/result.rs index 736ffb7d0caf3..0628e9309e32b 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1086,7 +1086,7 @@ impl Result { { match self { Ok(t) => t, - Err(e) => unwrap_failed(msg, &e), + Err(e) => unwrap_failed("expected: ", msg, &e), } } @@ -1134,7 +1134,7 @@ impl Result { { match self { Ok(t) => t, - Err(e) => unwrap_failed("called `Result::unwrap()` on an `Err` value", &e), + Err(e) => unwrap_failed("", "called `Result::unwrap()` on an `Err` value", &e), } } @@ -1197,7 +1197,7 @@ impl Result { T: fmt::Debug, { match self { - Ok(t) => unwrap_failed(msg, &t), + Ok(t) => unwrap_failed("expected error: ", msg, &t), Err(e) => e, } } @@ -1228,7 +1228,7 @@ impl Result { T: fmt::Debug, { match self { - Ok(t) => unwrap_failed("called `Result::unwrap_err()` on an `Ok` value", &t), + Ok(t) => unwrap_failed("", "called `Result::unwrap_err()` on an `Ok` value", &t), Err(e) => e, } } @@ -1728,8 +1728,8 @@ impl Result, E> { #[inline(never)] #[cold] #[track_caller] -fn unwrap_failed(msg: &str, error: &dyn fmt::Debug) -> ! { - panic!("{msg}: {error:?}") +fn unwrap_failed(prefix: &str, msg: &str, error: &dyn fmt::Debug) -> ! { + panic!("{prefix}{msg}: {error:?}") } // This is a separate function to avoid constructing a `dyn Debug` @@ -1740,7 +1740,7 @@ fn unwrap_failed(msg: &str, error: &dyn fmt::Debug) -> ! { #[inline] #[cold] #[track_caller] -fn unwrap_failed(_msg: &str, _error: &T) -> ! { +fn unwrap_failed(_prefix: &str, _msg: &str, _error: &T) -> ! { panic!() }