From 84982a4d463a4e4e83854ba885f1e08802aae695 Mon Sep 17 00:00:00 2001 From: Without Boats Date: Thu, 19 Sep 2019 00:27:54 +0200 Subject: [PATCH 01/10] Make `err!` macro record source location This is shown in the Debug display --- src/exception.rs | 13 ++++++++----- src/lib.rs | 4 ++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/exception.rs b/src/exception.rs index 5825e12..176b2be 100644 --- a/src/exception.rs +++ b/src/exception.rs @@ -17,10 +17,10 @@ impl Exception { Exception::construct(error, TypeId::of::()) } - pub fn new_adhoc(message: M) -> Exception where + pub fn new_adhoc(message: M, file: &'static str, line: u32, column: u32) -> Exception where M: Display + Debug + Send + Sync + 'static { - Exception::construct(MessageError(message), TypeId::of::()) + Exception::construct(MessageError(message, file, line, column), TypeId::of::()) } fn construct(error: E, type_id: TypeId) -> Exception where @@ -121,6 +121,7 @@ impl Drop for Exception { } } +// repr C to ensure that `E` remains in the final position #[repr(C)] struct InnerException { vtable: *const (), @@ -129,18 +130,20 @@ struct InnerException { error: E, } +// repr C to ensure that transmuting from trait objects is safe #[repr(C)] struct TraitObject { data: *const (), vtable: *const (), } -#[repr(transparent)] -struct MessageError(M); +// repr C to ensure that `M` remains in first position +#[repr(C)] +struct MessageError(M, &'static str, u32, u32); impl Debug for MessageError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - Debug::fmt(&self.0, f) + write!(f, "{} ({}:{}:{})", &self.0, self.1, self.2, self.3) } } diff --git a/src/lib.rs b/src/lib.rs index 86a0f9d..ec838bb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,6 @@ macro_rules! throw { #[macro_export] macro_rules! err { - ($e:expr) => { Exception::new_adhoc($e) }; - ($($arg:tt)*) => { Exception::new_adhoc(format!($($arg)*)) }; + ($e:expr) => { Exception::new_adhoc($e, file!(), line!(), column!()) }; + ($($arg:tt)*) => { Exception::new_adhoc(format!($($arg)*, file!(), line!(), column!())) }; } From 0058cf4bf8993959ad716d681ebccee0f8955f6b Mon Sep 17 00:00:00 2001 From: Without Boats Date: Thu, 19 Sep 2019 03:32:52 +0200 Subject: [PATCH 02/10] Update Debug/Display impls. --- src/exception.rs | 24 ++++++++++++++++++++++-- src/lib.rs | 2 +- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/exception.rs b/src/exception.rs index 176b2be..d48174a 100644 --- a/src/exception.rs +++ b/src/exception.rs @@ -40,6 +40,10 @@ impl Exception { } } + fn is_adhoc(&self) -> bool { + self.inner.type_id != self.inner.error().type_id(unsafe { mem::transmute(()) }) + } + pub fn backtrace(&self) -> &Backtrace { // NB: this unwrap can only fail if the underlying error's backtrace method is // nondeterministic, which would only happen in maliciously constructed code @@ -102,13 +106,29 @@ impl DerefMut for Exception { impl Debug for Exception { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "TODO") + if self.is_adhoc() { + writeln!(f, "error: {:?}\n", self.inner.error())?; + } else { + writeln!(f, "error: {}\n", self.inner.error())?; + } + + let errors: Vec<&(dyn Error + 'static)> = self.errors().skip(1).collect(); + + if !errors.is_empty() { + writeln!(f, "error causes:")?; + for (n, error) in errors.into_iter().rev().enumerate() { + writeln!(f, "{}: {}", n, error)?; + } + writeln!(f, "\n")?; + } + + writeln!(f, "{}", self.backtrace()) } } impl Display for Exception { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "TODO") + write!(f, "{}", self.inner.error()) } } diff --git a/src/lib.rs b/src/lib.rs index ec838bb..46333c7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,4 @@ -#![feature(backtrace)] +#![feature(backtrace, error_type_id)] mod as_error; mod exception; From 0ecd536c4e1e9ace6dee66c86bdf7fad78926669 Mon Sep 17 00:00:00 2001 From: Without Boats Date: Thu, 19 Sep 2019 03:37:13 +0200 Subject: [PATCH 03/10] fixups --- src/exception.rs | 3 ++- src/lib.rs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/exception.rs b/src/exception.rs index d48174a..7d7bca9 100644 --- a/src/exception.rs +++ b/src/exception.rs @@ -17,6 +17,7 @@ impl Exception { Exception::construct(error, TypeId::of::()) } + #[doc(hidden)] pub fn new_adhoc(message: M, file: &'static str, line: u32, column: u32) -> Exception where M: Display + Debug + Send + Sync + 'static { @@ -163,7 +164,7 @@ struct MessageError(M, &'static str, u32, u32); impl Debug for MessageError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{} ({}:{}:{})", &self.0, self.1, self.2, self.3) + write!(f, "{} (at {}:{}:{})", &self.0, self.1, self.2, self.3) } } diff --git a/src/lib.rs b/src/lib.rs index 46333c7..5bf8fee 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,6 @@ macro_rules! throw { #[macro_export] macro_rules! err { - ($e:expr) => { Exception::new_adhoc($e, file!(), line!(), column!()) }; - ($($arg:tt)*) => { Exception::new_adhoc(format!($($arg)*, file!(), line!(), column!())) }; + ($e:expr) => { $crate::Exception::new_adhoc($e, file!(), line!(), column!()) }; + ($($arg:tt)*) => { $crate::Exception::new_adhoc(format!($($arg)*, file!(), line!(), column!())) }; } From a28b7249354e3e6059bdae771d3859dc15bce0b1 Mon Sep 17 00:00:00 2001 From: Without Boats Date: Thu, 19 Sep 2019 03:38:31 +0200 Subject: [PATCH 04/10] Remove column entry --- src/exception.rs | 8 ++++---- src/lib.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/exception.rs b/src/exception.rs index 7d7bca9..62122fc 100644 --- a/src/exception.rs +++ b/src/exception.rs @@ -18,10 +18,10 @@ impl Exception { } #[doc(hidden)] - pub fn new_adhoc(message: M, file: &'static str, line: u32, column: u32) -> Exception where + pub fn new_adhoc(message: M, file: &'static str, line: u32) -> Exception where M: Display + Debug + Send + Sync + 'static { - Exception::construct(MessageError(message, file, line, column), TypeId::of::()) + Exception::construct(MessageError(message, file, line), TypeId::of::()) } fn construct(error: E, type_id: TypeId) -> Exception where @@ -160,11 +160,11 @@ struct TraitObject { // repr C to ensure that `M` remains in first position #[repr(C)] -struct MessageError(M, &'static str, u32, u32); +struct MessageError(M, &'static str, u32); impl Debug for MessageError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{} (at {}:{}:{})", &self.0, self.1, self.2, self.3) + write!(f, "{} (at {}:{})", &self.0, self.1, self.2) } } diff --git a/src/lib.rs b/src/lib.rs index 5bf8fee..6286fec 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,6 @@ macro_rules! throw { #[macro_export] macro_rules! err { - ($e:expr) => { $crate::Exception::new_adhoc($e, file!(), line!(), column!()) }; + ($e:expr) => { $crate::Exception::new_adhoc($e, file!(), line!()) }; ($($arg:tt)*) => { $crate::Exception::new_adhoc(format!($($arg)*, file!(), line!(), column!())) }; } From 9e07a3daf36f0a06168d76249c80d38d4259889e Mon Sep 17 00:00:00 2001 From: Without Boats Date: Thu, 19 Sep 2019 07:39:07 +0200 Subject: [PATCH 05/10] Stop using internal unstable API to check adhoc --- src/exception.rs | 11 ++++++----- src/lib.rs | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/exception.rs b/src/exception.rs index 62122fc..48f92c3 100644 --- a/src/exception.rs +++ b/src/exception.rs @@ -14,17 +14,17 @@ impl Exception { pub fn new(error: E) -> Exception where E: Error + Send + Sync + 'static { - Exception::construct(error, TypeId::of::()) + Exception::construct(error, TypeId::of::(), 0) } #[doc(hidden)] pub fn new_adhoc(message: M, file: &'static str, line: u32) -> Exception where M: Display + Debug + Send + Sync + 'static { - Exception::construct(MessageError(message, file, line), TypeId::of::()) + Exception::construct(MessageError(message, file, line), TypeId::of::(), 1) } - fn construct(error: E, type_id: TypeId) -> Exception where + fn construct(error: E, type_id: TypeId, is_adhoc: usize) -> Exception where E: Error + Send + Sync + 'static, { unsafe { @@ -34,7 +34,7 @@ impl Exception { }; let obj: TraitObject = mem::transmute(&error as &dyn Error); let vtable = obj.vtable; - let inner = InnerException { vtable, type_id, backtrace, error }; + let inner = InnerException { vtable, type_id, is_adhoc, backtrace, error }; Exception { inner: mem::transmute(Box::new(inner)) } @@ -42,7 +42,7 @@ impl Exception { } fn is_adhoc(&self) -> bool { - self.inner.type_id != self.inner.error().type_id(unsafe { mem::transmute(()) }) + self.inner.is_adhoc != 0 } pub fn backtrace(&self) -> &Backtrace { @@ -148,6 +148,7 @@ struct InnerException { vtable: *const (), type_id: TypeId, backtrace: Option, + is_adhoc: usize, error: E, } diff --git a/src/lib.rs b/src/lib.rs index 6286fec..29e084a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,4 @@ -#![feature(backtrace, error_type_id)] +#![feature(backtrace)] mod as_error; mod exception; From ea93ac69683baee2a4fb32677200dab5bf044a9a Mon Sep 17 00:00:00 2001 From: Without Boats Date: Fri, 20 Sep 2019 01:16:18 +0200 Subject: [PATCH 06/10] Remove location tracking from err!() --- src/exception.rs | 23 ++++++++++++++++------- src/lib.rs | 4 ++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/exception.rs b/src/exception.rs index 48f92c3..d304889 100644 --- a/src/exception.rs +++ b/src/exception.rs @@ -6,6 +6,16 @@ use std::mem; use std::ops::{Deref, DerefMut}; use std::ptr; +impl ErrorExt for MessageError { + fn type_id(&self) -> TypeId { + TypeId::of::() + } + + fn is_adhoc(&self) -> bool { + true + } +} + pub struct Exception { inner: Box>, } @@ -18,10 +28,10 @@ impl Exception { } #[doc(hidden)] - pub fn new_adhoc(message: M, file: &'static str, line: u32) -> Exception where + pub fn new_adhoc(message: M) -> Exception where M: Display + Debug + Send + Sync + 'static { - Exception::construct(MessageError(message, file, line), TypeId::of::(), 1) + Exception::construct(MessageError(message), TypeId::of::(), 1) } fn construct(error: E, type_id: TypeId, is_adhoc: usize) -> Exception where @@ -159,13 +169,12 @@ struct TraitObject { vtable: *const (), } -// repr C to ensure that `M` remains in first position -#[repr(C)] -struct MessageError(M, &'static str, u32); +#[repr(transparent)] +struct MessageError(M); impl Debug for MessageError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{} (at {}:{})", &self.0, self.1, self.2) + Debug::fmt(&self.0, f) } } @@ -175,7 +184,7 @@ impl Display for MessageError { } } -impl Error for MessageError { } +impl Error for MessageError { } impl InnerException<()> { fn error(&self) -> &(dyn Error + Send + Sync + 'static) { diff --git a/src/lib.rs b/src/lib.rs index 29e084a..339921c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,6 @@ macro_rules! throw { #[macro_export] macro_rules! err { - ($e:expr) => { $crate::Exception::new_adhoc($e, file!(), line!()) }; - ($($arg:tt)*) => { $crate::Exception::new_adhoc(format!($($arg)*, file!(), line!(), column!())) }; + ($e:expr) => { $crate::Exception::new_adhoc($e) }; + ($($arg:tt)*) => { $crate::Exception::new_adhoc(format!($($arg)*)) }; } From e1c46647d387d8409664173ced0d264c4e6118a7 Mon Sep 17 00:00:00 2001 From: Without Boats Date: Fri, 20 Sep 2019 01:26:57 +0200 Subject: [PATCH 07/10] Update Debug impl --- src/exception.rs | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/exception.rs b/src/exception.rs index d304889..d874173 100644 --- a/src/exception.rs +++ b/src/exception.rs @@ -1,21 +1,11 @@ use std::any::TypeId; -use std::backtrace::Backtrace; +use std::backtrace::{Backtrace, BacktraceStatus}; use std::error::Error; use std::fmt::{self, Debug, Display}; use std::mem; use std::ops::{Deref, DerefMut}; use std::ptr; -impl ErrorExt for MessageError { - fn type_id(&self) -> TypeId { - TypeId::of::() - } - - fn is_adhoc(&self) -> bool { - true - } -} - pub struct Exception { inner: Box>, } @@ -120,20 +110,33 @@ impl Debug for Exception { if self.is_adhoc() { writeln!(f, "error: {:?}\n", self.inner.error())?; } else { - writeln!(f, "error: {}\n", self.inner.error())?; + writeln!(f, "error: {}", self.inner.error())?; } - let errors: Vec<&(dyn Error + 'static)> = self.errors().skip(1).collect(); + let mut errors = self.errors().skip(1).enumerate(); - if !errors.is_empty() { - writeln!(f, "error causes:")?; - for (n, error) in errors.into_iter().rev().enumerate() { - writeln!(f, "{}: {}", n, error)?; + if let Some((n, error)) = errors.next() { + writeln!(f, "\ncaused by:")?; + writeln!(f, "\t{}: {}", n, error)?; + for (n, error) in errors { + writeln!(f, "\t{}: {}", n, error)?; + } + } + + let backtrace = self.backtrace(); + + match backtrace.status() { + BacktraceStatus::Captured => { + writeln!(f, "\n{}", backtrace)?; + } + BacktraceStatus::Disabled => { + writeln!(f, "backtrace disabled; run with RUST_BACKTRACE=1 environment variable \ + to display a backtrace")?; } - writeln!(f, "\n")?; + _ => { } } - writeln!(f, "{}", self.backtrace()) + Ok(()) } } From 89c44a14999c2aa7790d8cfa70028b3115be3242 Mon Sep 17 00:00:00 2001 From: Without Boats Date: Fri, 20 Sep 2019 01:32:01 +0200 Subject: [PATCH 08/10] No more adhoc special case needed --- src/exception.rs | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/exception.rs b/src/exception.rs index d874173..63b6801 100644 --- a/src/exception.rs +++ b/src/exception.rs @@ -14,17 +14,17 @@ impl Exception { pub fn new(error: E) -> Exception where E: Error + Send + Sync + 'static { - Exception::construct(error, TypeId::of::(), 0) + Exception::construct(error, TypeId::of::()) } #[doc(hidden)] pub fn new_adhoc(message: M) -> Exception where M: Display + Debug + Send + Sync + 'static { - Exception::construct(MessageError(message), TypeId::of::(), 1) + Exception::construct(MessageError(message), TypeId::of::()) } - fn construct(error: E, type_id: TypeId, is_adhoc: usize) -> Exception where + fn construct(error: E, type_id: TypeId) -> Exception where E: Error + Send + Sync + 'static, { unsafe { @@ -34,17 +34,13 @@ impl Exception { }; let obj: TraitObject = mem::transmute(&error as &dyn Error); let vtable = obj.vtable; - let inner = InnerException { vtable, type_id, is_adhoc, backtrace, error }; + let inner = InnerException { vtable, type_id, backtrace, error }; Exception { inner: mem::transmute(Box::new(inner)) } } } - fn is_adhoc(&self) -> bool { - self.inner.is_adhoc != 0 - } - pub fn backtrace(&self) -> &Backtrace { // NB: this unwrap can only fail if the underlying error's backtrace method is // nondeterministic, which would only happen in maliciously constructed code @@ -107,11 +103,7 @@ impl DerefMut for Exception { impl Debug for Exception { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - if self.is_adhoc() { - writeln!(f, "error: {:?}\n", self.inner.error())?; - } else { - writeln!(f, "error: {}", self.inner.error())?; - } + writeln!(f, "error: {}", self.inner.error())?; let mut errors = self.errors().skip(1).enumerate(); @@ -130,7 +122,7 @@ impl Debug for Exception { writeln!(f, "\n{}", backtrace)?; } BacktraceStatus::Disabled => { - writeln!(f, "backtrace disabled; run with RUST_BACKTRACE=1 environment variable \ + writeln!(f, "\nbacktrace disabled; run with RUST_BACKTRACE=1 environment variable \ to display a backtrace")?; } _ => { } @@ -161,7 +153,6 @@ struct InnerException { vtable: *const (), type_id: TypeId, backtrace: Option, - is_adhoc: usize, error: E, } From a4c2b1c1459483993c5c992e86282fc16ef5140e Mon Sep 17 00:00:00 2001 From: Without Boats Date: Fri, 20 Sep 2019 01:56:37 +0200 Subject: [PATCH 09/10] Fix Context backtraces --- src/context.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/context.rs b/src/context.rs index 9477c61..2d47cc3 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,3 +1,4 @@ +use std::backtrace::Backtrace; use std::fmt::{self, Debug, Display}; use std::error::Error; @@ -37,6 +38,10 @@ impl Display for Context { } impl Error for Context { + fn backtrace(&self) -> Option<&Backtrace> { + self.error.backtrace() + } + fn cause(&self) -> Option<&dyn Error> { Some(&self.error) } @@ -47,6 +52,10 @@ impl Error for Context { } impl Error for Context { + fn backtrace(&self) -> Option<&Backtrace> { + Some(self.error.backtrace()) + } + fn cause(&self) -> Option<&dyn Error> { Some(&*self.error) } From 735dd31af1810e653da89ab4c0637351b71a4e93 Mon Sep 17 00:00:00 2001 From: Without Boats Date: Fri, 20 Sep 2019 01:56:49 +0200 Subject: [PATCH 10/10] Update README --- README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/README.md b/README.md index a19f948..4c7e59e 100644 --- a/README.md +++ b/README.md @@ -75,8 +75,6 @@ contains a `context` method for injecting context around an error. # TODO -* Error derive * Possibly add a Display derive * Make throws work on closures and async blocks (attributes are not allowed on expressions on stable) -* Figure out best Debug and Display impls for Exception and Context