Merge pull request 137 from astraw/combined-from-and-backtrace-field

dtolnay · Aug 28, 2021 · c45d7e4 · c45d7e4
2 parents 031fea6 + cd2b9db
commit c45d7e4
Show file tree

Hide file tree

Showing 5 changed files with 147 additions and 29 deletions.
diff --git 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/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! {
@@ -127,20 +134,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);
 
@@ -239,14 +247,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! {
@@ -325,8 +351,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
@@ -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)
     }

diff --git 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

diff --git a/tests/test_backtrace.rs b/tests/test_backtrace.rs
@@ -10,9 +10,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;
@@ -54,6 +61,14 @@ pub mod structs {
         backtrace: Backtrace,
     }
 
+    #[derive(Error, Debug)]
+    #[error("...")]
+    pub struct CombinedBacktraceFrom {
+        #[from]
+        #[backtrace]
+        source: InnerBacktrace,
+    }
+
     #[derive(Error, Debug)]
     #[error("...")]
     pub struct OptBacktraceFrom {
@@ -97,6 +112,11 @@ pub mod structs {
         let error = BacktraceFrom::from(Inner);
         assert!(error.backtrace().is_some());
 
+        let error = CombinedBacktraceFrom::from(InnerBacktrace {
+            backtrace: Backtrace::capture(),
+        });
+        assert!(error.backtrace().is_some());
+
         let error = OptBacktraceFrom::from(Inner);
         assert!(error.backtrace().is_some());
 
@@ -107,7 +127,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;
@@ -157,6 +177,16 @@ pub mod enums {
         },
     }
 
+    #[derive(Error, Debug)]
+    pub enum CombinedBacktraceFrom {
+        #[error("...")]
+        Test {
+            #[from]
+            #[backtrace]
+            source: InnerBacktrace,
+        },
+    }
+
     #[derive(Error, Debug)]
     pub enum OptBacktraceFrom {
         #[error("...")]
@@ -204,6 +234,11 @@ pub mod enums {
         let error = BacktraceFrom::from(Inner);
         assert!(error.backtrace().is_some());
 
+        let error = CombinedBacktraceFrom::from(InnerBacktrace {
+            backtrace: Backtrace::capture(),
+        });
+        assert!(error.backtrace().is_some());
+
         let error = OptBacktraceFrom::from(Inner);
         assert!(error.backtrace().is_some());