From 86993c25ab39d24a983eb76a208f51c502e17b90 Mon Sep 17 00:00:00 2001 From: Andrew Straw Date: Fri, 11 Jun 2021 22:17:47 +0200 Subject: [PATCH 1/4] allow #[from] and #[backtrace] on same field See https://github.com/dtolnay/thiserror/pull/93#pullrequestreview-666213667 --- impl/src/expand.rs | 79 ++++++++++++++++++++++++++++++---------------- impl/src/prop.rs | 28 ++++++++++++++++ 2 files changed, 80 insertions(+), 27 deletions(-) diff --git a/impl/src/expand.rs b/impl/src/expand.rs index fa85cbb..ffc412e 100644 --- a/impl/src/expand.rs +++ b/impl/src/expand.rs @@ -58,18 +58,25 @@ fn impl_struct(input: Struct) -> TokenStream { self.#source.as_dyn_error().backtrace() } }; - let combinator = if type_is_option(backtrace_field.ty) { + if &source_field.member == backtrace { quote! { - #source_backtrace.or(self.#backtrace.as_ref()) + use thiserror::private::AsDynError; + #source_backtrace } } else { + let combinator = if type_is_option(backtrace_field.ty) { + quote! { + #source_backtrace.or(self.#backtrace.as_ref()) + } + } else { + quote! { + std::option::Option::Some(#source_backtrace.unwrap_or(&self.#backtrace)) + } + }; quote! { - std::option::Option::Some(#source_backtrace.unwrap_or(&self.#backtrace)) + use thiserror::private::AsDynError; + #combinator } - }; - quote! { - use thiserror::private::AsDynError; - #combinator } } else if type_is_option(backtrace_field.ty) { quote! { @@ -123,20 +130,21 @@ fn impl_struct(input: Struct) -> TokenStream { } }); - let from_impl = input.from_field().map(|from_field| { - let backtrace_field = input.backtrace_field(); - let from = from_field.ty; - let body = from_initializer(from_field, backtrace_field); - quote! { - #[allow(unused_qualifications)] - impl #impl_generics std::convert::From<#from> for #ty #ty_generics #where_clause { - #[allow(deprecated)] - fn from(source: #from) -> Self { - #ty #body + let from_impl = input.from_and_distinct_backtrace_fields().map( + |(from_field, backtrace_field)| { + let from = from_field.ty; + let body = from_initializer(from_field, backtrace_field); + quote! { + #[allow(unused_qualifications)] + impl #impl_generics std::convert::From<#from> for #ty #ty_generics #where_clause { + #[allow(deprecated)] + fn from(source: #from) -> Self { + #ty #body + } } } - } - }); + }, + ); let error_trait = spanned_error_trait(input.original); @@ -235,14 +243,32 @@ fn impl_enum(input: Enum) -> TokenStream { } } (Some(backtrace_field), _) => { + let source = variant.from_field().map(|f| &f.member); let backtrace = &backtrace_field.member; - let body = if type_is_option(backtrace_field.ty) { - quote!(backtrace.as_ref()) + if source == Some(backtrace) { + let varsource = quote!(source); + let source_backtrace = quote_spanned! {source.span()=> + #varsource.as_dyn_error().backtrace() + }; + + quote! { + #ty::#ident { + #source: #varsource, + .. + } => { + use thiserror::private::AsDynError; + #source_backtrace + } + } } else { - quote!(std::option::Option::Some(backtrace)) - }; - quote! { - #ty::#ident {#backtrace: backtrace, ..} => #body, + let body = if type_is_option(backtrace_field.ty) { + quote!(backtrace.as_ref()) + } else { + quote!(std::option::Option::Some(backtrace)) + }; + quote! { + #ty::#ident {#backtrace: backtrace, ..} => #body, + } } } (None, _) => quote! { @@ -315,8 +341,7 @@ fn impl_enum(input: Enum) -> TokenStream { }; let from_impls = input.variants.iter().filter_map(|variant| { - let from_field = variant.from_field()?; - let backtrace_field = variant.backtrace_field(); + let (from_field, backtrace_field) = variant.from_and_distinct_backtrace_fields()?; let variant = &variant.ident; let from = from_field.ty; let body = from_initializer(from_field, backtrace_field); diff --git a/impl/src/prop.rs b/impl/src/prop.rs index 059b74b..f80ca3d 100644 --- a/impl/src/prop.rs +++ b/impl/src/prop.rs @@ -6,6 +6,20 @@ impl Struct<'_> { from_field(&self.fields) } + pub(crate) fn from_and_distinct_backtrace_fields(&self) -> Option<(&Field, Option<&Field>)> { + self.from_field().map(|from_field| { + if let Some(backtrace_field) = self.backtrace_field() { + if backtrace_field.member == from_field.member { + (from_field, None) + } else { + (from_field, Some(backtrace_field)) + } + } else { + (from_field, None) + } + }) + } + pub(crate) fn source_field(&self) -> Option<&Field> { source_field(&self.fields) } @@ -47,6 +61,20 @@ impl Variant<'_> { from_field(&self.fields) } + pub(crate) fn from_and_distinct_backtrace_fields(&self) -> Option<(&Field, Option<&Field>)> { + self.from_field().map(|from_field| { + if let Some(backtrace_field) = self.backtrace_field() { + if backtrace_field.member == from_field.member { + (from_field, None) + } else { + (from_field, Some(backtrace_field)) + } + } else { + (from_field, None) + } + }) + } + pub(crate) fn source_field(&self) -> Option<&Field> { source_field(&self.fields) } From da2454f989faf8de31b6612a1fbd52e0c61adfee Mon Sep 17 00:00:00 2001 From: Andrew Straw Date: Sat, 12 Jun 2021 16:16:37 +0200 Subject: [PATCH 2/4] describe `#[from]` and `#[backtrace]` together --- README.md | 14 ++++++++++++++ src/lib.rs | 16 ++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/README.md b/README.md index 76c436a..101a522 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,20 @@ pub enum DataStoreError { } ``` +- If a field is `#[from]` and `#[backtrace]`, the Error trait's `backtrace()` + method is forwarded to the `source`. + + ```rust + #[derive(Error, Debug)] + pub enum MyError { + Io { + #[from] + #[backtrace] + source: io::Error, + }, + } + ``` + - Errors may use `error(transparent)` to forward the source and Display methods straight through to an underlying error without adding an additional message. This would be appropriate for enums that need an "anything else" variant. diff --git a/src/lib.rs b/src/lib.rs index 02941c8..8884db3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -161,6 +161,22 @@ //! # }; //! ``` //! +//! - If a field is `#[from]` and `#[backtrace]`, the Error trait's `backtrace()` +//! method is forwarded to the field. +//! +//! ```rust +//! # const IGNORE: &str = stringify! { +//! #[derive(Error, Debug)] +//! pub enum MyError { +//! Io { +//! #[from] +//! #[backtrace] +//! source: io::Error, +//! }, +//! } +//! # }; +//! ``` +//! //! - Errors may use `error(transparent)` to forward the source and Display //! methods straight through to an underlying error without adding an //! additional message. This would be appropriate for enums that need an From 52dc286e79984bcadbaba399735250922219a56d Mon Sep 17 00:00:00 2001 From: Andrew Straw Date: Sat, 12 Jun 2021 17:03:49 +0200 Subject: [PATCH 3/4] test combined `#[from]` and `#[backtrace]` --- tests/test_backtrace.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/test_backtrace.rs b/tests/test_backtrace.rs index 09bc13d..608b405 100644 --- a/tests/test_backtrace.rs +++ b/tests/test_backtrace.rs @@ -50,6 +50,14 @@ pub mod structs { backtrace: Backtrace, } + #[derive(Error, Debug)] + #[error("...")] + pub struct CombinedBacktraceFrom { + #[from] + #[backtrace] + source: Inner, + } + #[derive(Error, Debug)] #[error("...")] pub struct OptBacktraceFrom { @@ -93,6 +101,9 @@ pub mod structs { let error = BacktraceFrom::from(Inner); assert!(error.backtrace().is_some()); + let error = CombinedBacktraceFrom::from(Inner); + assert!(error.backtrace().is_some()); + let error = OptBacktraceFrom::from(Inner); assert!(error.backtrace().is_some()); @@ -153,6 +164,16 @@ pub mod enums { }, } + #[derive(Error, Debug)] + pub enum CombinedBacktraceFrom { + #[error("...")] + Test { + #[from] + #[backtrace] + source: Inner, + }, + } + #[derive(Error, Debug)] pub enum OptBacktraceFrom { #[error("...")] @@ -200,6 +221,9 @@ pub mod enums { let error = BacktraceFrom::from(Inner); assert!(error.backtrace().is_some()); + let error = CombinedBacktraceFrom::from(Inner); + assert!(error.backtrace().is_some()); + let error = OptBacktraceFrom::from(Inner); assert!(error.backtrace().is_some()); From cd2b9db3aa84733cb10d2bb3d3d6251ace9357da Mon Sep 17 00:00:00 2001 From: Andrew Straw Date: Sun, 13 Jun 2021 09:44:18 +0200 Subject: [PATCH 4/4] fixup tests for combined from and backtrace --- tests/test_backtrace.rs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/tests/test_backtrace.rs b/tests/test_backtrace.rs index 608b405..42e37ca 100644 --- a/tests/test_backtrace.rs +++ b/tests/test_backtrace.rs @@ -6,9 +6,16 @@ use thiserror::Error; #[error("...")] pub struct Inner; +#[cfg(thiserror_nightly_testing)] +#[derive(Error, Debug)] +#[error("...")] +pub struct InnerBacktrace { + backtrace: std::backtrace::Backtrace, +} + #[cfg(thiserror_nightly_testing)] pub mod structs { - use super::Inner; + use super::{Inner, InnerBacktrace}; use std::backtrace::Backtrace; use std::error::Error; use std::sync::Arc; @@ -55,7 +62,7 @@ pub mod structs { pub struct CombinedBacktraceFrom { #[from] #[backtrace] - source: Inner, + source: InnerBacktrace, } #[derive(Error, Debug)] @@ -101,7 +108,9 @@ pub mod structs { let error = BacktraceFrom::from(Inner); assert!(error.backtrace().is_some()); - let error = CombinedBacktraceFrom::from(Inner); + let error = CombinedBacktraceFrom::from(InnerBacktrace { + backtrace: Backtrace::capture(), + }); assert!(error.backtrace().is_some()); let error = OptBacktraceFrom::from(Inner); @@ -114,7 +123,7 @@ pub mod structs { #[cfg(thiserror_nightly_testing)] pub mod enums { - use super::Inner; + use super::{Inner, InnerBacktrace}; use std::backtrace::Backtrace; use std::error::Error; use std::sync::Arc; @@ -170,7 +179,7 @@ pub mod enums { Test { #[from] #[backtrace] - source: Inner, + source: InnerBacktrace, }, } @@ -221,7 +230,9 @@ pub mod enums { let error = BacktraceFrom::from(Inner); assert!(error.backtrace().is_some()); - let error = CombinedBacktraceFrom::from(Inner); + let error = CombinedBacktraceFrom::from(InnerBacktrace { + backtrace: Backtrace::capture(), + }); assert!(error.backtrace().is_some()); let error = OptBacktraceFrom::from(Inner);