From d11ff2a06e7977685c9f093ec667301d3e5f63c8 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Wed, 22 Jun 2022 22:01:21 +1000 Subject: [PATCH 1/8] Impl `io::error::repr_unpacked::Repr::new` that accepts `ErrorData>` Signed-off-by: Jiahao XU --- library/std/src/io/error/repr_unpacked.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/std/src/io/error/repr_unpacked.rs b/library/std/src/io/error/repr_unpacked.rs index 3729c039c42d7..d6ad55b99f5c0 100644 --- a/library/std/src/io/error/repr_unpacked.rs +++ b/library/std/src/io/error/repr_unpacked.rs @@ -10,6 +10,10 @@ type Inner = ErrorData>; pub(super) struct Repr(Inner); impl Repr { + #[inline] + pub(super) fn new(dat: ErrorData>) -> Self { + Self(dat) + } pub(super) fn new_custom(b: Box) -> Self { Self(Inner::Custom(b)) } From 1713e25a411c3854e85baa5fe076d5e3e8cffe35 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Wed, 22 Jun 2022 22:01:51 +1000 Subject: [PATCH 2/8] Impl `io::error::repr_bitpacked::Repr::new` that accepts `ErrorData>` Signed-off-by: Jiahao XU --- library/std/src/io/error/repr_bitpacked.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/library/std/src/io/error/repr_bitpacked.rs b/library/std/src/io/error/repr_bitpacked.rs index e80068b46abb9..292bf4826fd23 100644 --- a/library/std/src/io/error/repr_bitpacked.rs +++ b/library/std/src/io/error/repr_bitpacked.rs @@ -132,6 +132,15 @@ unsafe impl Send for Repr {} unsafe impl Sync for Repr {} impl Repr { + pub(super) fn new(dat: ErrorData>) -> Self { + match dat { + ErrorData::Os(code) => Self::new_os(code), + ErrorData::Simple(kind) => Self::new_simple(kind), + ErrorData::SimpleMessage(simple_message) => Self::new_simple_message(simple_message), + ErrorData::Custom(b) => Self::new_custom(b), + } + } + pub(super) fn new_custom(b: Box) -> Self { let p = Box::into_raw(b).cast::(); // Should only be possible if an allocator handed out a pointer with From e0ea0c25341a34bbaa96a8173096e7713d472131 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Wed, 22 Jun 2022 22:43:54 +1000 Subject: [PATCH 3/8] Add new unstable API `Error::try_downgrade_inner` Signed-off-by: Jiahao XU --- library/std/src/io/error.rs | 57 +++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index 4a50e647c640e..b1931a522909a 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -795,6 +795,63 @@ impl Error { } } + /// Attempt to downgrade the inner error to `E` if any. + /// + /// If this [`Error`] was constructed via [`new`] then this function will + /// attempt to perform downgrade on it, otherwise it will return [`Err`]. + /// + /// If downgrade succeeds, it will return [`Ok`], otherwise it will also + /// return [`Err`]. + /// + /// [`new`]: Error::new + /// + /// # Examples + /// + /// ``` + /// #![feature(io_error_try_downcast_inner)] + /// + /// use std::fmt; + /// use std::io; + /// use std::error::Error; + /// + /// #[derive(Debug)] + /// enum E { + /// Io(io::Error), + /// SomeOtherVariant, + /// } + /// + /// impl fmt::Display for E { + /// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + /// todo!() + /// } + /// } + /// impl Error for E {} + /// + /// impl From for E { + /// fn from(err: io::Error) -> E { + /// err.try_downcast_inner::() + /// .map(|b| *b) + /// .unwrap_or_else(E::Io) + /// } + /// } + /// ``` + #[unstable(feature = "io_error_try_downcast_inner", issue = "none")] + pub fn try_downcast_inner(self) -> result::Result, Self> + where + E: error::Error + Send + Sync + 'static, + { + match self.repr.into_data() { + ErrorData::Custom(b) if b.error.is::() => { + let res = (*b).error.downcast::(); + + // Safety: b.error.is::() returns true, + // which means that res must be Ok(e). + Ok(unsafe { res.unwrap_unchecked() }) + } + repr_data => Err(Self { repr: Repr::new(repr_data) }), + } + } + /// Returns the corresponding [`ErrorKind`] for this error. /// /// # Examples From d2211c9fdc8fd49bdbd17e92a16a9f304eda1d8c Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Wed, 22 Jun 2022 23:09:30 +1000 Subject: [PATCH 4/8] Add new unit test `test_try_downcast_inner` Signed-off-by: Jiahao XU --- library/std/src/io/error/tests.rs | 53 ++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/library/std/src/io/error/tests.rs b/library/std/src/io/error/tests.rs index 8d7877bcad35d..558594c816e16 100644 --- a/library/std/src/io/error/tests.rs +++ b/library/std/src/io/error/tests.rs @@ -1,4 +1,4 @@ -use super::{const_io_error, Custom, Error, ErrorData, ErrorKind, Repr}; +use super::{const_io_error, Custom, Error, ErrorData, ErrorKind, Repr, SimpleMessage}; use crate::assert_matches::assert_matches; use crate::error; use crate::fmt; @@ -141,3 +141,54 @@ fn test_custom_error_packing() { }) if error.downcast_ref::().as_deref() == Some(&Bojji(true)), ); } + +#[derive(Debug)] +struct E; + +impl fmt::Display for E { + fn fmt(&self, _f: &mut fmt::Formatter<'_>) -> fmt::Result { + Ok(()) + } +} + +impl error::Error for E {} + +#[test] +fn test_try_downcast_inner() { + // Case 1: custom error, downcast succeeds + let io_error = Error::new(ErrorKind::Other, Bojji(true)); + let e: Box = io_error.try_downcast_inner().unwrap(); + assert!(e.0); + + // Case 2: custom error, downcast fails + let io_error = Error::new(ErrorKind::Other, Bojji(true)); + let io_error = io_error.try_downcast_inner::().unwrap_err(); + + // ensures that the custom error is intact + assert_eq!(ErrorKind::Other, io_error.kind()); + let e: Box = io_error.try_downcast_inner().unwrap(); + assert!(e.0); + + // Case 3: os error + let errno = 20; + let io_error = Error::from_raw_os_error(errno); + let io_error = io_error.try_downcast_inner::().unwrap_err(); + + assert_eq!(errno, io_error.raw_os_error().unwrap()); + + // Case 4: simple + let kind = ErrorKind::OutOfMemory; + let io_error: Error = kind.into(); + let io_error = io_error.try_downcast_inner::().unwrap_err(); + + assert_eq!(kind, io_error.kind()); + + // Case 5: simple message + const SIMPLE_MESSAGE: SimpleMessage = + SimpleMessage { kind: ErrorKind::Other, message: "simple message error test" }; + let io_error = Error::from_static_message(&SIMPLE_MESSAGE); + let io_error = io_error.try_downcast_inner::().unwrap_err(); + + assert_eq!(SIMPLE_MESSAGE.kind, io_error.kind()); + assert_eq!(SIMPLE_MESSAGE.message, &*format!("{io_error}")); +} From 516da4c93f804f27b1a5f664ae06b9054532f095 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 23 Jun 2022 12:58:33 +1000 Subject: [PATCH 5/8] Use `unwrap` instead of `unwrap_unchecked` Signed-off-by: Jiahao XU --- library/std/src/io/error.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index b1931a522909a..4c6fd55ad13a3 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -844,9 +844,13 @@ impl Error { ErrorData::Custom(b) if b.error.is::() => { let res = (*b).error.downcast::(); - // Safety: b.error.is::() returns true, - // which means that res must be Ok(e). - Ok(unsafe { res.unwrap_unchecked() }) + // downcast is a really trivial and is marked as inline, so + // it's likely be inlined here. + // + // And the compiler should be able to eliminate the branch + // that produces `Err` here since b.error.is::() + // returns true. + Ok(res.unwrap()) } repr_data => Err(Self { repr: Repr::new(repr_data) }), } From 111253c519adc17f5bbfa41b7368512ac7e1effd Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 14 Jul 2022 11:24:44 +1000 Subject: [PATCH 6/8] Rename `std::io::Error::try_downcast_inner` to `downcast` Signed-off-by: Jiahao XU --- library/std/src/io/error.rs | 8 ++++---- library/std/src/io/error/tests.rs | 14 +++++++------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index 4c6fd55ad13a3..8f2119425d112 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -808,7 +808,7 @@ impl Error { /// # Examples /// /// ``` - /// #![feature(io_error_try_downcast_inner)] + /// #![feature(io_error_downcast)] /// /// use std::fmt; /// use std::io; @@ -829,14 +829,14 @@ impl Error { /// /// impl From for E { /// fn from(err: io::Error) -> E { - /// err.try_downcast_inner::() + /// err.downcast::() /// .map(|b| *b) /// .unwrap_or_else(E::Io) /// } /// } /// ``` - #[unstable(feature = "io_error_try_downcast_inner", issue = "none")] - pub fn try_downcast_inner(self) -> result::Result, Self> + #[unstable(feature = "io_error_downcast", issue = "none")] + pub fn downcast(self) -> result::Result, Self> where E: error::Error + Send + Sync + 'static, { diff --git a/library/std/src/io/error/tests.rs b/library/std/src/io/error/tests.rs index 558594c816e16..c897a5e8701c4 100644 --- a/library/std/src/io/error/tests.rs +++ b/library/std/src/io/error/tests.rs @@ -154,32 +154,32 @@ impl fmt::Display for E { impl error::Error for E {} #[test] -fn test_try_downcast_inner() { +fn test_std_io_error_downcast() { // Case 1: custom error, downcast succeeds let io_error = Error::new(ErrorKind::Other, Bojji(true)); - let e: Box = io_error.try_downcast_inner().unwrap(); + let e: Box = io_error.downcast().unwrap(); assert!(e.0); // Case 2: custom error, downcast fails let io_error = Error::new(ErrorKind::Other, Bojji(true)); - let io_error = io_error.try_downcast_inner::().unwrap_err(); + let io_error = io_error.downcast::().unwrap_err(); // ensures that the custom error is intact assert_eq!(ErrorKind::Other, io_error.kind()); - let e: Box = io_error.try_downcast_inner().unwrap(); + let e: Box = io_error.downcast().unwrap(); assert!(e.0); // Case 3: os error let errno = 20; let io_error = Error::from_raw_os_error(errno); - let io_error = io_error.try_downcast_inner::().unwrap_err(); + let io_error = io_error.downcast::().unwrap_err(); assert_eq!(errno, io_error.raw_os_error().unwrap()); // Case 4: simple let kind = ErrorKind::OutOfMemory; let io_error: Error = kind.into(); - let io_error = io_error.try_downcast_inner::().unwrap_err(); + let io_error = io_error.downcast::().unwrap_err(); assert_eq!(kind, io_error.kind()); @@ -187,7 +187,7 @@ fn test_try_downcast_inner() { const SIMPLE_MESSAGE: SimpleMessage = SimpleMessage { kind: ErrorKind::Other, message: "simple message error test" }; let io_error = Error::from_static_message(&SIMPLE_MESSAGE); - let io_error = io_error.try_downcast_inner::().unwrap_err(); + let io_error = io_error.downcast::().unwrap_err(); assert_eq!(SIMPLE_MESSAGE.kind, io_error.kind()); assert_eq!(SIMPLE_MESSAGE.message, &*format!("{io_error}")); From d8aba1002a222d2a826c6c35fbb03dd4c1594a05 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 15 Jul 2022 13:04:06 +1000 Subject: [PATCH 7/8] Improve example of `downcast` Co-authored-by: Jane Losare-Lusby --- library/std/src/io/error.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index 8f2119425d112..811688175e091 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -821,9 +821,10 @@ impl Error { /// } /// /// impl fmt::Display for E { - /// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - /// todo!() - /// } + /// // ... + /// # fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + /// # todo!() + /// # } /// } /// impl Error for E {} /// From 8e8a3be22f1082da965f6a696be93fbbd7b5d4ba Mon Sep 17 00:00:00 2001 From: Jane Losare-Lusby Date: Fri, 15 Jul 2022 13:17:44 -0700 Subject: [PATCH 8/8] Apply suggestions from code review --- library/std/src/io/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index 811688175e091..ff7fdcae16f53 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -836,7 +836,7 @@ impl Error { /// } /// } /// ``` - #[unstable(feature = "io_error_downcast", issue = "none")] + #[unstable(feature = "io_error_downcast", issue = "99262")] pub fn downcast(self) -> result::Result, Self> where E: error::Error + Send + Sync + 'static,