Merge pull request #361 from shepmaster/implicit-data-with-source

.cirrus.yml

@@ -145,6 +145,10 @@ nightly_test_task:
     - cd compatibility-tests/report-try-trait/
     - rustc --version
     - cargo test
+  report_provider_api_test_script:
+    - cd compatibility-tests/backtrace-provider-api/
+    - rustc --version
+    - cargo test
   before_cache_script: rm -rf $CARGO_HOME/registry/index
 
 unstable_std_backtraces_test_task:

compatibility-tests/backtrace-provider-api/Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "backtrace-provider-api"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+snafu = { path = "../..", features = ["unstable-provider-api"] }

compatibility-tests/backtrace-provider-api/rust-toolchain

@@ -0,0 +1 @@
+nightly

compatibility-tests/backtrace-provider-api/src/lib.rs

@@ -0,0 +1,44 @@
+#![cfg(test)]
+#![feature(error_generic_member_access, provide_any)]
+
+use snafu::{prelude::*, IntoError};
+
+#[test]
+fn does_not_capture_a_backtrace_when_source_provides_a_backtrace() {
+    #[derive(Debug, Snafu)]
+    struct InnerError {
+        backtrace: snafu::Backtrace,
+    }
+
+    #[derive(Debug, Snafu)]
+    struct OuterError {
+        source: InnerError,
+        backtrace: Option<snafu::Backtrace>,
+    }
+
+    enable_backtrace_capture();
+    let e = OuterSnafu.into_error(InnerSnafu.build());
+
+    assert!(e.backtrace.is_none());
+}
+
+#[test]
+fn does_capture_a_backtrace_when_source_does_not_provide_a_backtrace() {
+    #[derive(Debug, Snafu)]
+    struct InnerError;
+
+    #[derive(Debug, Snafu)]
+    struct OuterError {
+        source: InnerError,
+        backtrace: Option<snafu::Backtrace>,
+    }
+
+    enable_backtrace_capture();
+    let e = OuterSnafu.into_error(InnerSnafu.build());
+
+    assert!(e.backtrace.is_some());
+}
+
+fn enable_backtrace_capture() {
+    std::env::set_var("RUST_LIB_BACKTRACE", "1");
+}

snafu-derive/src/shared.rs

@@ -199,13 +199,35 @@ pub mod context_selector {
         }
 
         fn construct_implicit_fields(&self) -> TokenStream {
+            let crate_root = self.crate_root;
+            let expression = quote! {
+                #crate_root::GenerateImplicitData::generate()
+            };
+
+            self.construct_implicit_fields_with_expression(expression)
+        }
+
+        fn construct_implicit_fields_with_source(&self) -> TokenStream {
+            let crate_root = self.crate_root;
+            let expression = quote! { {
+                use #crate_root::AsErrorSource;
+                let error = error.as_error_source();
+                #crate_root::GenerateImplicitData::generate_with_source(error)
+            } };
+
+            self.construct_implicit_fields_with_expression(expression)
+        }
+
+        fn construct_implicit_fields_with_expression(
+            &self,
+            expression: TokenStream,
+        ) -> TokenStream {
             self.implicit_fields
                 .iter()
                 .chain(self.backtrace_field)
                 .map(|field| {
-                    let crate_root = self.crate_root;
                     let name = &field.name;
-                    quote! { #name: #crate_root::GenerateImplicitData::generate(), }
+                    quote! { #name: #expression, }
                 })
                 .collect()
         }
@@ -286,7 +308,11 @@ pub mod context_selector {
             let user_field_generics = self.user_field_generics();
             let extended_where_clauses = self.extended_where_clauses();
             let transfer_user_fields = self.transfer_user_fields();
-            let construct_implicit_fields = self.construct_implicit_fields();
+            let construct_implicit_fields = if source_field.is_some() {
+                self.construct_implicit_fields_with_source()
+            } else {
+                self.construct_implicit_fields()
+            };
 
             let (source_ty, transfer_source_field) = match source_field {
                 Some(source_field) => {
@@ -309,8 +335,8 @@ pub mod context_selector {
                     #track_caller
                     fn into_error(self, error: Self::Source) -> #parameterized_error_name {
                         #error_constructor_name {
-                            #transfer_source_field
                             #construct_implicit_fields
+                            #transfer_source_field
                             #(#transfer_user_fields),*
                         }
                     }
@@ -327,6 +353,8 @@ pub mod context_selector {
             let parameterized_error_name = self.parameterized_error_name;
             let error_constructor_name = self.error_constructor_name;
             let construct_implicit_fields = self.construct_implicit_fields();
+            let construct_implicit_fields_with_source =
+                self.construct_implicit_fields_with_source();
 
             // testme: transform
 
@@ -356,18 +384,18 @@ pub mod context_selector {
                     #track_caller
                     fn without_source(message: String) -> Self {
                         #error_constructor_name {
+                            #construct_implicit_fields
                             #empty_source_field
                             #message_field_name: message,
-                            #construct_implicit_fields
                         }
                     }
 
                     #track_caller
                     fn with_source(error: Self::Source, message: String) -> Self {
                         #error_constructor_name {
+                            #construct_implicit_fields_with_source
                             #transfer_source_field
                             #message_field_name: message,
-                            #construct_implicit_fields
                         }
                     }
                 }
@@ -377,7 +405,8 @@ pub mod context_selector {
         fn generate_from_source(self, source_field: &crate::SourceField) -> TokenStream {
             let parameterized_error_name = self.parameterized_error_name;
             let error_constructor_name = self.error_constructor_name;
-            let construct_implicit_fields = self.construct_implicit_fields();
+            let construct_implicit_fields_with_source =
+                self.construct_implicit_fields_with_source();
             let original_generics_without_defaults = self.original_generics_without_defaults;
             let user_field_generics = self.user_field_generics();
             let where_clauses = self.where_clauses;
@@ -394,8 +423,8 @@ pub mod context_selector {
                     #track_caller
                     fn from(error: #source_field_type) -> Self {
                         #error_constructor_name {
+                            #construct_implicit_fields_with_source
                             #transfer_source_field
-                            #construct_implicit_fields
                         }
                     }
                 }

src/lib.rs

@@ -1138,6 +1138,16 @@ pub trait FromString {
 pub trait GenerateImplicitData {
     /// Build the data.
     fn generate() -> Self;
+
+    /// Build the data using the given source
+    #[cfg_attr(feature = "rust_1_46", track_caller)]
+    fn generate_with_source(source: &dyn crate::Error) -> Self
+    where
+        Self: Sized,
+    {
+        let _source = source;
+        Self::generate()
+    }
 }
 
 /// View a backtrace-like value as an optional backtrace.
@@ -1155,32 +1165,46 @@ pub trait AsBacktrace {
 /// This value will be tested only once per program execution;
 /// changing the environment variable after it has been checked will
 /// have no effect.
+///
+/// ## Interaction with the Provider API
+///
+/// If you enable the [`unstable-provider-api` feature
+/// flag][provider-ff], a backtrace will not be captured if the
+/// original error is able to provide a `Backtrace`, even if the
+/// appropriate environment variables are set. This prevents capturing
+/// a redundant backtrace.
+///
+/// [provider-ff]: crate::guide::feature_flags#unstable-provider-api
 #[cfg(any(feature = "std", test))]
 impl GenerateImplicitData for Option<Backtrace> {
     fn generate() -> Self {
-        use std::env;
-        use std::sync::{
-            atomic::{AtomicBool, Ordering},
-            Once,
-        };
-
-        static START: Once = Once::new();
-        static ENABLED: AtomicBool = AtomicBool::new(false);
-
-        START.call_once(|| {
-            // TODO: What values count as "true"?
-            let enabled = env::var_os("RUST_LIB_BACKTRACE")
-                .or_else(|| env::var_os("RUST_BACKTRACE"))
-                .map_or(false, |v| v == "1");
-            ENABLED.store(enabled, Ordering::SeqCst);
-        });
-
-        if ENABLED.load(Ordering::SeqCst) {
+        if backtrace_collection_enabled() {
             Some(Backtrace::generate())
         } else {
             None
         }
     }
+
+    fn generate_with_source(source: &dyn crate::Error) -> Self {
+        #[cfg(feature = "unstable-provider-api")]
+        {
+            use core::any;
+
+            if !backtrace_collection_enabled() {
+                None
+            } else if any::request_ref::<Backtrace>(source).is_some() {
+                None
+            } else {
+                Some(Backtrace::generate_with_source(source))
+            }
+        }
+
+        #[cfg(not(feature = "unstable-provider-api"))]
+        {
+            let _source = source;
+            Self::generate()
+        }
+    }
 }
 
 #[cfg(any(feature = "std", test))]
@@ -1190,6 +1214,30 @@ impl AsBacktrace for Option<Backtrace> {
     }
 }
 
+#[cfg(any(feature = "std", test))]
+fn backtrace_collection_enabled() -> bool {
+    use std::{
+        env,
+        sync::{
+            atomic::{AtomicBool, Ordering},
+            Once,
+        },
+    };
+
+    static START: Once = Once::new();
+    static ENABLED: AtomicBool = AtomicBool::new(false);
+
+    START.call_once(|| {
+        // TODO: What values count as "true"?
+        let enabled = env::var_os("RUST_LIB_BACKTRACE")
+            .or_else(|| env::var_os("RUST_BACKTRACE"))
+            .map_or(false, |v| v == "1");
+        ENABLED.store(enabled, Ordering::SeqCst);
+    });
+
+    ENABLED.load(Ordering::SeqCst)
+}
+
 #[cfg(feature = "backtraces-impl-backtrace-crate")]
 impl GenerateImplicitData for Backtrace {
     fn generate() -> Self {

tests/implicit.rs

@@ -64,3 +64,96 @@ mod multiple_fields {
         assert_eq!(one, two);
     }
 }
+
+mod with_and_without_source {
+    use snafu::{prelude::*, FromString, IntoError};
+
+    #[derive(Debug, PartialEq)]
+    enum ItWas {
+        Generate,
+        GenerateWithSource,
+    }
+
+    #[derive(Debug)]
+    struct ImplicitData(ItWas);
+
+    impl snafu::GenerateImplicitData for ImplicitData {
+        fn generate() -> Self {
+            Self(ItWas::Generate)
+        }
+
+        fn generate_with_source(_: &dyn snafu::Error) -> Self {
+            Self(ItWas::GenerateWithSource)
+        }
+    }
+
+    #[derive(Debug, Snafu)]
+    struct InnerError;
+
+    #[derive(Debug, Snafu)]
+    struct HasSource {
+        source: InnerError,
+        #[snafu(implicit)]
+        data: ImplicitData,
+    }
+
+    #[derive(Debug, Snafu)]
+    struct NoSource {
+        #[snafu(implicit)]
+        data: ImplicitData,
+    }
+
+    #[derive(Debug, Snafu)]
+    #[snafu(context(false))]
+    struct HasSourceNoContext {
+        source: InnerError,
+        #[snafu(implicit)]
+        data: ImplicitData,
+    }
+
+    #[derive(Debug, Snafu)]
+    #[snafu(whatever, display("{message}"))]
+    struct MyOwnWhatever {
+        message: String,
+        #[snafu(source(from(Box<dyn std::error::Error>, Some)))]
+        source: Option<Box<dyn std::error::Error>>,
+        #[snafu(implicit)]
+        data: ImplicitData,
+    }
+
+    #[test]
+    fn calls_generate_for_no_source() {
+        let e = NoSourceSnafu.build();
+        assert_eq!(e.data.0, ItWas::Generate);
+    }
+
+    #[test]
+    fn calls_generate_with_source_for_source() {
+        let e = HasSourceSnafu.into_error(InnerError);
+        assert_eq!(e.data.0, ItWas::GenerateWithSource);
+    }
+
+    #[test]
+    fn calls_generate_for_none() {
+        let e = NoSourceSnafu.into_error(snafu::NoneError);
+        assert_eq!(e.data.0, ItWas::Generate);
+    }
+
+    #[test]
+    fn calls_generate_with_source_for_no_context() {
+        let e = HasSourceNoContext::from(InnerError);
+        assert_eq!(e.data.0, ItWas::GenerateWithSource);
+    }
+
+    #[test]
+    fn calls_generate_for_whatever_with_no_source() {
+        let e = MyOwnWhatever::without_source("bang".into());
+        assert_eq!(e.data.0, ItWas::Generate);
+    }
+
+    #[test]
+    fn calls_generate_with_source_for_whatever_with_source() {
+        let e = MyOwnWhatever::with_source(Box::new(InnerError), "bang".into());
+        assert_eq!(e.data.0, ItWas::GenerateWithSource);
+    }
+}