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.

Sign up for GitHub

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

Jump to bottom

Replace fn backtrace() with Provider API in Error derive #200

Merged
merged 8 commits into from
Oct 4, 2022
Merged

Replace fn backtrace() with Provider API in Error derive #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ea8537a
Replace `fn backtrace()` with `Provider` API
ilslv Aug 31, 2022
406b245
CHANGELOG
ilslv Aug 31, 2022
7c9c103
Fix docs
ilslv Aug 31, 2022
8546c8c
Fix docs
ilslv Aug 31, 2022
b1f7b50
Minor corrections
tyranron Sep 8, 2022
422b37d
Fix unit tests
ilslv Sep 8, 2022
db6eabc
Remove mentions of `backtrace()` method in docs
ilslv Sep 12, 2022
56e9d15
Correction
ilslv Sep 12, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- Use a deterministic `HashSet` in all derives, this is needed for rust analyzer
to work correctly.
- Use `Provider` API for backtraces in `Error` derive.

## 0.99.10 - 2020-09-11

Expand Down
45 changes: 30 additions & 15 deletions doc/error.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

# Using #[derive(Error)]
Deriving `Error` will generate an `Error` implementation, that contains
(depending on the type) a `source()` and a `backtrace()` method. Please note,
at the time of writing `backtrace` is only supported on nightly rust. So you
(depending on the type) a `source()` and a `provide()` method. Please note,
at the time of writing `provide()` is only supported on nightly rust. So you
have to use that to make use of it.

For a struct, these methods always do the same. For an `enum` they have separate
Expand All @@ -17,22 +17,22 @@ often [`From`] as well.
[`Display`]: display.html
[`From`]: from.html

## When and how does it derive `backtrace()`?

1. It's a struct/variant with named fields and one of the fields is
called `backtrace`. Then it would return that field as the `backtrace`.
2. It's a tuple struct/variant and the type of exactly one of the fields is
called `Backtrace`. Then it would return that field as the `backtrace`.
3. One of the fields is annotated with `#[error(backtrace)]`. Then it would
return that field as the `backtrace`.

## When and how does it derive `source()`?

1. It's a struct/variant with named fields and one is the fields is
called `source`. Then it would return that field as the `source`.
2. It's a tuple struct/variant and there's exactly one field that is not used as
the `backtrace`. So either a tuple struct with one field, or one with two where one
is the `backtrace`. Then it returns this field as the `source`.
3. One of the fields is annotated with `#[error(source)]`. Then it would
return that field as the `source`.

## When and how does it derive `provide()`?

1. It's a struct/variant with named fields and one of the fields is
called `backtrace`. Then it would return that field as the `backtrace`.
2. It's a tuple struct/variant and the type of exactly one of the fields is
called `Backtrace`. Then it would return that field as the `backtrace`.
3. One of the fields is annotated with `#[error(backtrace)]`. Then it would
return that field as the `backtrace`.

Expand All @@ -47,10 +47,10 @@ ignored for one of these methods by using `#[error(not(backtrace))]` or
# Example usage

```rust
#![feature(backtrace)]
#![feature(error_generic_member_access, provide_any)]
# #[macro_use] extern crate derive_more;
# use std::error::Error as _;
use std::backtrace::Backtrace;
use std::{any, backtrace::Backtrace};

// std::error::Error requires std::fmt::Debug and std::fmt::Display,
// so we can also use derive_more::Display for fully declarative
Expand Down Expand Up @@ -90,17 +90,32 @@ enum CompoundError {
WithSource {
source: Simple,
},
#[from(ignore)]
WithBacktraceFromSource {
#[error(backtrace)]
source: Simple,
},
#[display(fmt = "{source}")]
WithDifferentBacktrace {
source: Simple,
backtrace: Backtrace,
},
WithExplicitSource {
#[error(source)]
explicit_source: WithSource,
},
#[from(ignore)]
WithBacktraceFromExplicitSource {
#[error(backtrace, source)]
explicit_source: WithSource,
},
Tuple(WithExplicitSource),
WithoutSource(#[error(not(source))] Tuple),
}

fn main() {
assert!(Simple.source().is_none());
assert!(Simple.backtrace().is_none());
ilslv marked this conversation as resolved.
Show resolved Hide resolved
assert!(any::request_ref::<Backtrace>(&Simple).is_none());
assert!(WithSource::default().source().is_some());
assert!(WithExplicitSource::default().source().is_some());
assert!(Tuple::default().source().is_some());
Expand All @@ -110,7 +125,7 @@ fn main() {
backtrace: Backtrace::capture(),
};
assert!(with_source_and_backtrace.source().is_some());
assert!(with_source_and_backtrace.backtrace().is_some());
assert!(any::request_ref::<Backtrace>(&with_source_and_backtrace).is_some());

assert!(CompoundError::Simple.source().is_none());
assert!(CompoundError::from(Simple).source().is_some());
Expand Down
107 changes: 77 additions & 30 deletions src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub fn expand(
})
.collect();

let (bounds, source, backtrace) = match state.derive_type {
let (bounds, source, provide) = match state.derive_type {
DeriveType::Named | DeriveType::Unnamed => render_struct(&type_params, &state)?,
DeriveType::Enum => render_enum(&type_params, &state)?,
};
Expand All @@ -45,11 +45,11 @@ pub fn expand(
}
});

let backtrace = backtrace.map(|backtrace| {
let provide = provide.map(|provide| {
quote! {
fn backtrace(&self) -> Option<&::std::backtrace::Backtrace> {
#backtrace
}
fn provide<'_demand>(&'_demand self, demand: &mut ::std::any::Demand<'_demand>) {
#provide
}
}
});

Expand Down Expand Up @@ -82,7 +82,7 @@ pub fn expand(
let render = quote! {
impl#impl_generics ::std::error::Error for #ident#ty_generics #where_clause {
#source
#backtrace
#provide
}
};

Expand All @@ -96,9 +96,9 @@ fn render_struct(
let parsed_fields = parse_fields(type_params, state)?;

let source = parsed_fields.render_source_as_struct();
let backtrace = parsed_fields.render_backtrace_as_struct();
let provide = parsed_fields.render_provide_as_struct();

Ok((parsed_fields.bounds, source, backtrace))
Ok((parsed_fields.bounds, source, provide))
}

fn render_enum(
Expand All @@ -107,7 +107,7 @@ fn render_enum(
) -> Result<(HashSet<syn::Type>, Option<TokenStream>, Option<TokenStream>)> {
let mut bounds = HashSet::default();
let mut source_match_arms = Vec::new();
let mut backtrace_match_arms = Vec::new();
let mut provide_match_arms = Vec::new();

for variant in state.enabled_variant_data().variants {
let default_info = FullMetaInfo {
Expand All @@ -131,35 +131,31 @@ fn render_enum(
source_match_arms.push(expr);
}

if let Some(expr) = parsed_fields.render_backtrace_as_enum_variant_match_arm() {
backtrace_match_arms.push(expr);
if let Some(expr) = parsed_fields.render_provide_as_enum_variant_match_arm() {
provide_match_arms.push(expr);
}

bounds.extend(parsed_fields.bounds.into_iter());
}

let render = |match_arms: &mut Vec<TokenStream>| {
let render = |match_arms: &mut Vec<TokenStream>, unmatched| {
if !match_arms.is_empty() && match_arms.len() < state.variants.len() {
match_arms.push(quote!(_ => None));
match_arms.push(quote!(_ => #unmatched));
}

if !match_arms.is_empty() {
let expr = quote! {
(!match_arms.is_empty()).then(|| {
quote! {
match self {
#(#match_arms),*
}
};

Some(expr)
} else {
None
}
}
})
};

let source = render(&mut source_match_arms);
let backtrace = render(&mut backtrace_match_arms);
let source = render(&mut source_match_arms, quote!(None));
let provide = render(&mut provide_match_arms, quote!(()));

Ok((bounds, source, backtrace))
Ok((bounds, source, provide))
}

fn allowed_attr_params() -> AttrParams {
Expand Down Expand Up @@ -203,16 +199,67 @@ impl<'input, 'state> ParsedFields<'input, 'state> {
Some(quote!(#pattern => #expr))
}

fn render_backtrace_as_struct(&self) -> Option<TokenStream> {
fn render_provide_as_struct(&self) -> Option<TokenStream> {
let backtrace = self.backtrace?;
let backtrace_expr = &self.data.members[backtrace];
Some(quote!(Some(&#backtrace_expr)))

let source_provider = self.source.map(|source| {
let source_expr = &self.data.members[source];
quote! {
::std::error::Error::provide(&#source_expr, demand);
}
});
let backtrace_provider = self
.source
.filter(|source| *source == backtrace)
.is_none()
.then(|| {
let backtrace_expr = &self.data.members[backtrace];
quote! {
demand.provide_ref::<std::backtrace::Backtrace>(&#backtrace_expr);
Copy link
Owner

@JelteF JelteF Sep 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Food for thought: One of the reasons for this provider API (afaict) is so more types can be provided that just Backtrace, without needing to add more methods to the Error trait.

So probably we want to allow this too, e.g. to embed some additionaly context such as trace IDs in there for distributed tracing. This would probabyl be better as a separate PR. I think it makes sense to keep dedicated and easy to use support for the Backtrace type.

Another idea is to also provide the source through the provider API, like is done in the rust docs

impl std::error::Error for Error {
    fn provide<'a>(&'a self, demand: &mut Demand<'a>) {
        demand
            .provide_ref::<MyBacktrace>(&self.backtrace)
            .provide_ref::<dyn std::error::Error + 'static>(&self.source);
    }
}

Source: https://doc.rust-lang.org/nightly/std/error/trait.Error.html

Again this would be better in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JelteF I've already raised question of what should be provided and this implementation mirrors thiserror. As Provider API is quite new and source() default impl still returns None, I think that we shouldn't rush with providing dyn Error and wait how Provider API will be used throughout the ecosystem.

}
});

(source_provider.is_some() || backtrace_provider.is_some()).then(|| {
quote! {
#backtrace_provider
#source_provider
}
})
}

fn render_backtrace_as_enum_variant_match_arm(&self) -> Option<TokenStream> {
fn render_provide_as_enum_variant_match_arm(&self) -> Option<TokenStream> {
let backtrace = self.backtrace?;
let pattern = self.data.matcher(&[backtrace], &[quote!(backtrace)]);
Some(quote!(#pattern => Some(backtrace)))

match self.source {
Some(source) if source == backtrace => {
let pattern = self.data.matcher(&[source], &[quote!(source)]);
Some(quote! {
#pattern => {
::std::error::Error::provide(source, demand);
}
})
}
Some(source) => {
let pattern = self.data.matcher(
&[source, backtrace],
&[quote!(source), quote!(backtrace)],
);
Some(quote! {
#pattern => {
demand.provide_ref::<std::backtrace::Backtrace>(backtrace);
::std::error::Error::provide(source, demand);
}
})
}
None => {
let pattern = self.data.matcher(&[backtrace], &[quote!(backtrace)]);
Some(quote! {
#pattern => {
demand.provide_ref::<std::backtrace::Backtrace>(backtrace);
}
})
}
}
}
}

Expand Down
Loading