Optimize representation of Error without backtrace

This change modifies Error to not store empty backtraces.
This saves space and allows zero-sized errors without
backtraces to be stored without any allocation.

src/backtrace/internal.rs

@@ -18,6 +18,9 @@ struct MaybeResolved {
 unsafe impl Send for MaybeResolved {}
 unsafe impl Sync for MaybeResolved {}
 
+
+pub(super) const NONE: InternalBacktrace = InternalBacktrace { backtrace: None };
+
 impl InternalBacktrace {
     pub(super) fn new() -> InternalBacktrace {
         static ENABLED: AtomicUsize = ATOMIC_USIZE_INIT;
@@ -45,10 +48,6 @@ impl InternalBacktrace {
         }
     }
 
-    pub(super) fn none() -> InternalBacktrace {
-        InternalBacktrace { backtrace: None }
-    }
-
     pub(super) fn as_backtrace(&self) -> Option<&Backtrace> {
         let bt = match self.backtrace {
             Some(ref bt) => bt,

src/backtrace/mod.rs

@@ -90,6 +90,9 @@ with_std! {
         internal: InternalBacktrace
     }
 
+
+    pub(crate) const NONE: Backtrace = Backtrace { internal: internal::NONE };
+
     impl Backtrace {
         /// Construct a new backtrace. This will only create a real backtrace
         /// if the cate is compiled in std mode and the `RUST_BACKTRACE`
@@ -98,8 +101,8 @@ with_std! {
             Backtrace { internal: InternalBacktrace::new() }
         }
 
-        pub(crate) fn none() -> Backtrace {
-            Backtrace { internal: InternalBacktrace::none() }
+        pub(crate) fn is_none(&self) -> bool {
+            self.internal.as_backtrace().is_none()
         }
     }
 

src/error.rs

@@ -1,10 +1,7 @@
 use core::fmt::{self, Display, Debug};
 
-use core::mem;
-use core::ptr;
-
 use Fail;
-use backtrace::Backtrace;
+use backtrace::{self, Backtrace};
 use context::Context;
 use compat::Compat;
 
@@ -19,23 +16,58 @@ use compat::Compat;
 /// information, and can be downcast into the `Fail`ure that underlies it for
 /// more detailed inspection.
 pub struct Error {
-    pub(crate) inner: Box<Inner<Fail>>,
+    pub(crate) inner: Box<FailOrWithBacktrace>,
 }
 
-pub(crate) struct Inner<F: ?Sized + Fail> {
+pub(crate) struct WithBacktrace<F: Fail> {
     backtrace: Backtrace,
-    pub(crate) failure: F,
+    failure: F,
+}
+
+// Unsafe because the `is_withbacktrace` method must be correct for memory safety.
+pub(crate) unsafe trait FailOrWithBacktrace: Send + Sync + 'static {
+    fn fail_ref(&self) -> &Fail;
+    fn fail_mut(&mut self) -> &mut Fail;
+    fn backtrace(&self) -> &Backtrace;
+    fn is_withbacktrace(&self) -> bool;
+}
+
+unsafe impl<T: Fail> FailOrWithBacktrace for T {
+    fn fail_ref(&self) -> &Fail { self }
+    fn fail_mut(&mut self) -> &mut Fail { self }
+    fn backtrace(&self) -> &Backtrace {
+        Fail::backtrace(self).unwrap_or(&backtrace::NONE)
+    }
+    fn is_withbacktrace(&self) -> bool { false }
+}
+
+unsafe impl<T: Fail> FailOrWithBacktrace for WithBacktrace<T> {
+    fn fail_ref(&self) -> &Fail { &self.failure }
+    fn fail_mut(&mut self) -> &mut Fail { &mut self.failure }
+    fn backtrace(&self) -> &Backtrace {
+        &self.backtrace
+    }
+    fn is_withbacktrace(&self) -> bool { true }
 }
 
 impl<F: Fail> From<F> for Error {
     fn from(failure: F) -> Error {
-        let inner: Inner<F> = {
-            let backtrace = if failure.backtrace().is_none() {
-                Backtrace::new()
-            } else { Backtrace::none() };
-            Inner { failure, backtrace }
+        let inner = if failure.backtrace().is_some() {
+            Box::new(failure)
+        } else {
+            // Attempt to add a backtrace
+            let backtrace = Backtrace::new();
+            if backtrace.is_none() {
+                Box::new(failure) as Box<FailOrWithBacktrace>
+            } else {
+                Box::new(WithBacktrace {
+                    backtrace,
+                    failure,
+                })
+            }
         };
-        Error { inner: Box::new(inner) }
+
+        Error { inner }
     }
 }
 
@@ -44,7 +76,7 @@ impl Error {
     /// method on `Fail`, this does not return an Option. The Error type
     /// always has an underlying `Fail`ure.
     pub fn cause(&self) -> &Fail {
-        &self.inner.failure
+        self.inner.fail_ref()
     }
 
     /// Get a reference to the Backtrace for this Error.
@@ -53,7 +85,7 @@ impl Error {
     /// be returned. Otherwise, the backtrace will have been constructed at
     /// the point that failure was cast into the Error type.
     pub fn backtrace(&self) -> &Backtrace {
-        self.inner.failure.backtrace().unwrap_or(&self.inner.backtrace)
+        self.inner.backtrace()
     }
 
     /// Provide context for this Error.
@@ -89,21 +121,19 @@ impl Error {
     /// the case that the underlying error is of a different type, the
     /// original Error is returned.
     pub fn downcast<T: Fail>(self) -> Result<T, Error> {
-        let ret: Option<T> = self.downcast_ref().map(|fail| {
-            unsafe {
-                // drop the backtrace
-                let _ = ptr::read(&self.inner.backtrace as *const Backtrace);
-                // read out the fail type
-                ptr::read(fail as *const T)
-            }
-        });
-        match ret {
-            Some(ret) => {
-                // forget self (backtrace is dropped, failure is moved
-                mem::forget(self);
-                Ok(ret)
-            }
-            _       => Err(self)
+        if self.downcast_ref::<T>().is_some() {
+            Ok(unsafe {
+                let is_withbacktrace = self.inner.is_withbacktrace();
+                let ptr: *mut FailOrWithBacktrace = Box::into_raw(self.inner);
+
+                if is_withbacktrace {
+                    Box::from_raw(ptr as *mut WithBacktrace<T>).failure
+                } else {
+                    *Box::from_raw(ptr as *mut T)
+                }
+            })
+        } else {
+            Err(self)
         }
     }
 
@@ -112,33 +142,32 @@ impl Error {
     ///
     /// If the underlying error is not of type `T`, this will return `None`.
     pub fn downcast_ref<T: Fail>(&self) -> Option<&T> {
-        self.inner.failure.downcast_ref()
+        self.inner.fail_ref().downcast_ref::<T>()
     }
 
     /// Attempt to downcast this Error to a particular `Fail` type by
     /// mutable reference.
     ///
     /// If the underlying error is not of type `T`, this will return `None`.
     pub fn downcast_mut<T: Fail>(&mut self) -> Option<&mut T> {
-        self.inner.failure.downcast_mut()
+        self.inner.fail_mut().downcast_mut::<T>()
     }
 }
 
 impl Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        Display::fmt(&self.inner.failure, f)
-    }
-}
-
-impl Debug for Inner<Fail> {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "Error {{ failure: {:?} }}\n\n{:?}", &self.failure, &self.backtrace)
+        Display::fmt(&self.cause(), f)
     }
 }
 
 impl Debug for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{:?}", &self.inner)
+        write!(f, "Error {{ failure: {:?} }}\n\n", self.inner.fail_ref())?;
+        let backtrace = self.inner.backtrace();
+        if !backtrace.is_none() {
+            write!(f, "{:?}", backtrace)?;
+        }
+        Ok(())
     }
 }