Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proof of concept of dyn dispatch based report handler #97

Closed
wants to merge 10 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -20,6 +20,9 @@ rustversion = "1.0"
thiserror = "1.0"
trybuild = { version = "1.0.19", features = ["diff"] }

[dependencies]
once_cell = "1.4.0"

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
rustdoc-args = ["--cfg", "doc_cfg"]
27 changes: 0 additions & 27 deletions src/backtrace.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,3 @@
#[cfg(backtrace)]
pub(crate) use std::backtrace::Backtrace;

#[cfg(not(backtrace))]
pub(crate) enum Backtrace {}

#[cfg(backtrace)]
macro_rules! backtrace {
() => {
Some(Backtrace::capture())
};
}

#[cfg(not(backtrace))]
macro_rules! backtrace {
() => {
None
};
}

#[cfg(backtrace)]
macro_rules! backtrace_if_absent {
($err:expr) => {
@@ -27,10 +7,3 @@ macro_rules! backtrace_if_absent {
}
};
}

#[cfg(all(feature = "std", not(backtrace)))]
macro_rules! backtrace_if_absent {
($err:expr) => {
None
};
}
7 changes: 3 additions & 4 deletions src/context.rs
Original file line number Diff line number Diff line change
@@ -24,8 +24,7 @@ mod ext {
where
C: Display + Send + Sync + 'static,
{
let backtrace = backtrace_if_absent!(self);
Error::from_context(context, self, backtrace)
Error::from_context(context, self)
}
}

@@ -84,15 +83,15 @@ impl<T> Context<T, Infallible> for Option<T> {
where
C: Display + Send + Sync + 'static,
{
self.ok_or_else(|| Error::from_display(context, backtrace!()))
self.ok_or_else(|| Error::from_display(context))
}

fn with_context<C, F>(self, context: F) -> Result<T, Error>
where
C: Display + Send + Sync + 'static,
F: FnOnce() -> C,
{
self.ok_or_else(|| Error::from_display(context(), backtrace!()))
self.ok_or_else(|| Error::from_display(context()))
}
}

79 changes: 45 additions & 34 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use crate::alloc::Box;
use crate::backtrace::Backtrace;
use crate::chain::Chain;
use crate::{Error, StdError};
use core::any::TypeId;
use core::fmt::{self, Debug, Display};
use core::mem::{self, ManuallyDrop};
use core::ptr::{self, NonNull};
#[cfg(backtrace)]
use std::backtrace::Backtrace;

#[cfg(feature = "std")]
use core::ops::{Deref, DerefMut};
@@ -24,8 +25,7 @@ impl Error {
where
E: StdError + Send + Sync + 'static,
{
let backtrace = backtrace_if_absent!(error);
Error::from_std(error, backtrace)
Error::from_std(error)
}

/// Create a new error object from a printable error message.
@@ -69,11 +69,11 @@ impl Error {
where
M: Display + Debug + Send + Sync + 'static,
{
Error::from_adhoc(message, backtrace!())
Error::from_adhoc(message)
}

#[cfg(feature = "std")]
pub(crate) fn from_std<E>(error: E, backtrace: Option<Backtrace>) -> Self
pub(crate) fn from_std<E>(error: E) -> Self
where
E: StdError + Send + Sync + 'static,
{
@@ -88,10 +88,10 @@ impl Error {
};

// Safety: passing vtable that operates on the right type E.
unsafe { Error::construct(error, vtable, backtrace) }
unsafe { Error::construct(error, vtable) }
}

pub(crate) fn from_adhoc<M>(message: M, backtrace: Option<Backtrace>) -> Self
pub(crate) fn from_adhoc<M>(message: M) -> Self
where
M: Display + Debug + Send + Sync + 'static,
{
@@ -109,10 +109,10 @@ impl Error {

// Safety: MessageError is repr(transparent) so it is okay for the
// vtable to allow casting the MessageError<M> to M.
unsafe { Error::construct(error, vtable, backtrace) }
unsafe { Error::construct(error, vtable) }
}

pub(crate) fn from_display<M>(message: M, backtrace: Option<Backtrace>) -> Self
pub(crate) fn from_display<M>(message: M) -> Self
where
M: Display + Send + Sync + 'static,
{
@@ -130,11 +130,11 @@ impl Error {

// Safety: DisplayError is repr(transparent) so it is okay for the
// vtable to allow casting the DisplayError<M> to M.
unsafe { Error::construct(error, vtable, backtrace) }
unsafe { Error::construct(error, vtable) }
}

#[cfg(feature = "std")]
pub(crate) fn from_context<C, E>(context: C, error: E, backtrace: Option<Backtrace>) -> Self
pub(crate) fn from_context<C, E>(context: C, error: E) -> Self
where
C: Display + Send + Sync + 'static,
E: StdError + Send + Sync + 'static,
@@ -152,14 +152,11 @@ impl Error {
};

// Safety: passing vtable that operates on the right type.
unsafe { Error::construct(error, vtable, backtrace) }
unsafe { Error::construct(error, vtable) }
}

#[cfg(feature = "std")]
pub(crate) fn from_boxed(
error: Box<dyn StdError + Send + Sync>,
backtrace: Option<Backtrace>,
) -> Self {
pub(crate) fn from_boxed(error: Box<dyn StdError + Send + Sync>) -> Self {
use crate::wrapper::BoxedError;
let error = BoxedError(error);
let vtable = &ErrorVTable {
@@ -174,25 +171,26 @@ impl Error {

// Safety: BoxedError is repr(transparent) so it is okay for the vtable
// to allow casting to Box<dyn StdError + Send + Sync>.
unsafe { Error::construct(error, vtable, backtrace) }
unsafe { Error::construct(error, vtable) }
}

// Takes backtrace as argument rather than capturing it here so that the
// user sees one fewer layer of wrapping noise in the backtrace.
//
// Unsafe because the given vtable must have sensible behavior on the error
// value of type E.
unsafe fn construct<E>(
error: E,
vtable: &'static ErrorVTable,
backtrace: Option<Backtrace>,
) -> Self
unsafe fn construct<E>(error: E, vtable: &'static ErrorVTable) -> Self
where
E: StdError + Send + Sync + 'static,
{
let handler = crate::HOOK
.get_or_init(|| Box::new(|error| Box::new(crate::DefaultHandler::new(error))))(
&error
);

let inner = Box::new(ErrorImpl {
vtable,
backtrace,
handler,
_object: error,
});
// Erase the concrete type of E from the compile-time type system. This
@@ -279,11 +277,8 @@ impl Error {
object_drop_rest: context_chain_drop_rest::<C>,
};

// As the cause is anyhow::Error, we already have a backtrace for it.
let backtrace = None;

// Safety: passing vtable that operates on the right type.
unsafe { Error::construct(error, vtable, backtrace) }
unsafe { Error::construct(error, vtable) }
}

/// Get the backtrace for this Error.
@@ -310,6 +305,16 @@ impl Error {
self.inner.backtrace()
}

#[cfg(feature = "std")]
pub fn handler(&self) -> &dyn crate::ReportHandler {
self.inner.handler()
}

#[cfg(feature = "std")]
pub fn handler_mut(&mut self) -> &mut dyn crate::ReportHandler {
self.inner.handler_mut()
}

/// An iterator of the chain of source errors contained by this Error.
///
/// This iterator will visit every error in the cause chain of this error
@@ -469,8 +474,7 @@ where
E: StdError + Send + Sync + 'static,
{
fn from(error: E) -> Self {
let backtrace = backtrace_if_absent!(error);
Error::from_std(error, backtrace)
Error::from_std(error)
}
}

@@ -680,7 +684,7 @@ where
#[repr(C)]
pub(crate) struct ErrorImpl<E> {
vtable: &'static ErrorVTable,
backtrace: Option<Backtrace>,
handler: Box<dyn crate::ReportHandler>,
// NOTE: Don't use directly. Use only through vtable. Erased type may have
// different alignment.
_object: E,
@@ -722,12 +726,19 @@ impl ErrorImpl<()> {
// This unwrap can only panic if the underlying error's backtrace method
// is nondeterministic, which would only happen in maliciously
// constructed code.
self.backtrace
.as_ref()
.or_else(|| self.error().backtrace())
.expect("backtrace capture failed")
self.handler.backtrace(self.error())
}

pub(crate) fn handler(&self) -> &dyn crate::ReportHandler {
self.handler.as_ref()
}

#[cfg(feature = "std")]
pub(crate) fn handler_mut(&mut self) -> &mut dyn crate::ReportHandler {
self.handler.as_mut()
}

#[cfg(feature = "std")]
pub(crate) fn chain(&self) -> Chain {
Chain::new(self.error())
}
41 changes: 29 additions & 12 deletions src/fmt.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,42 @@
use crate::chain::Chain;
use crate::error::ErrorImpl;
use core::fmt::{self, Debug, Write};
use core::fmt::{self, Write};

impl ErrorImpl<()> {
pub(crate) fn display(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}", self.error())?;

if f.alternate() {
for cause in self.chain().skip(1) {
write!(f, ": {}", cause)?;
}
}
let error = self.error();
let handler = self.handler();

Ok(())
handler.display(error, f)
}

pub(crate) fn debug(&self, f: &mut fmt::Formatter) -> fmt::Result {
let error = self.error();
let handler = self.handler();

handler.debug(error, f)
}
}

impl crate::ReportHandler for crate::DefaultHandler {
#[cfg(backtrace)]
fn backtrace<'a>(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we can have a publicly exported method that depends on a non feature cfg, I'll make sure to test this when I port color-eyre to depend on this branch

&'a self,
error: &'a (dyn crate::StdError + 'static),
) -> &'a std::backtrace::Backtrace {
error
.backtrace()
.or_else(|| self.backtrace.as_ref())
.expect("backtrace must have been captured")
}

fn debug(
&self,
error: &(dyn crate::StdError + 'static),
f: &mut core::fmt::Formatter<'_>,
) -> core::fmt::Result {
if f.alternate() {
return Debug::fmt(error, f);
return core::fmt::Debug::fmt(error, f);
}

write!(f, "{}", error)?;
@@ -42,7 +59,7 @@ impl ErrorImpl<()> {
{
use std::backtrace::BacktraceStatus;

let backtrace = self.backtrace();
let backtrace = self.backtrace(error);
if let BacktraceStatus::Captured = backtrace.status() {
let mut backtrace = backtrace.to_string();
write!(f, "\n\n")?;
@@ -97,7 +114,7 @@ where
}
}

#[cfg(test)]
#[cfg(all(test, feature = "std"))]
mod tests {
use super::*;

8 changes: 2 additions & 6 deletions src/kind.rs
Original file line number Diff line number Diff line change
@@ -50,9 +50,6 @@ use core::fmt::{Debug, Display};
#[cfg(feature = "std")]
use crate::StdError;

#[cfg(backtrace)]
use std::backtrace::Backtrace;

pub struct Adhoc;

pub trait AdhocKind: Sized {
@@ -69,7 +66,7 @@ impl Adhoc {
where
M: Display + Debug + Send + Sync + 'static,
{
Error::from_adhoc(message, backtrace!())
Error::from_adhoc(message)
}
}

@@ -110,7 +107,6 @@ impl BoxedKind for Box<dyn StdError + Send + Sync> {}
#[cfg(feature = "std")]
impl Boxed {
pub fn new(self, error: Box<dyn StdError + Send + Sync>) -> Error {
let backtrace = backtrace_if_absent!(error);
Error::from_boxed(error, backtrace)
Error::from_boxed(error)
}
}
Loading