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
Show file tree
Hide file tree
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
Expand Up @@ -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) => {
Expand All @@ -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
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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()))
}
}

Expand Down
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};
Expand All @@ -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.
Expand Down Expand Up @@ -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,
{
Expand All @@ -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,
{
Expand All @@ -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,
{
Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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())
}
Expand Down
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)?;
Expand All @@ -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")?;
Expand Down Expand Up @@ -97,7 +114,7 @@ where
}
}

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

Expand Down
8 changes: 2 additions & 6 deletions src/kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -69,7 +66,7 @@ impl Adhoc {
where
M: Display + Debug + Send + Sync + 'static,
{
Error::from_adhoc(message, backtrace!())
Error::from_adhoc(message)
}
}

Expand Down Expand Up @@ -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