From 21198c9c0ab5052091cbb0b7be364a1297cd8cd0 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Tue, 13 Sep 2022 11:31:32 -0700 Subject: [PATCH 1/6] Move multiple-provide test into test_backtrace Currently fails with: error[E0034]: multiple applicable items in scope --> tests/test_backtrace.rs:165:13 | 165 | x: std::io::Error, | ^ multiple `provide` found | = note: candidate #1 is defined in an impl of the trait `Provider` for the type `E` = note: candidate #2 is defined in an impl of the trait `std::error::Error` for the type `std::io::Error` help: disambiguate the associated function for candidate #1 | 165 | Provider::provide(&x, Error): std::io::Error, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ help: disambiguate the associated function for candidate #2 | 165 | std::error::Error::provide(&x, Error): std::io::Error, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ --- tests/test_backtrace.rs | 19 +++++++++++++++++++ tests/ui/multiple-provide.rs | 19 ------------------- tests/ui/multiple-provide.stderr | 16 ---------------- 3 files changed, 19 insertions(+), 35 deletions(-) delete mode 100644 tests/ui/multiple-provide.rs delete mode 100644 tests/ui/multiple-provide.stderr diff --git a/tests/test_backtrace.rs b/tests/test_backtrace.rs index d197447..43f68b8 100644 --- a/tests/test_backtrace.rs +++ b/tests/test_backtrace.rs @@ -149,6 +149,25 @@ pub mod structs { }; assert!(any::request_ref::(&error).is_some()); } + + // https://github.com/dtolnay/thiserror/issues/185 -- std::error::Error and + // std::any::Provide both have a method called 'provide', so directly + // calling it from generated code could be ambiguous. + #[test] + fn test_provide_name_collision() { + use std::any::Provider; + + #[derive(Error, Debug)] + #[error("...")] + struct MyError { + #[source] + #[backtrace] + x: std::io::Error, + } + + let _: dyn Error; + let _: dyn Provider; + } } #[cfg(thiserror_nightly_testing)] diff --git a/tests/ui/multiple-provide.rs b/tests/ui/multiple-provide.rs deleted file mode 100644 index dc2c52b..0000000 --- a/tests/ui/multiple-provide.rs +++ /dev/null @@ -1,19 +0,0 @@ -#![feature(error_generic_member_access, provide_any)] - -use thiserror::Error; -use std::any::Provider; -use std::error::Error; - -// FIXME: this should work. https://github.com/dtolnay/thiserror/issues/185 -#[derive(Error, Debug)] -#[error("...")] -struct MyError { - #[source] - #[backtrace] - x: std::io::Error, -} - -fn main() { - let _: dyn Error; - let _: dyn Provider; -} diff --git a/tests/ui/multiple-provide.stderr b/tests/ui/multiple-provide.stderr deleted file mode 100644 index 14dcffa..0000000 --- a/tests/ui/multiple-provide.stderr +++ /dev/null @@ -1,16 +0,0 @@ -error[E0034]: multiple applicable items in scope - --> tests/ui/multiple-provide.rs:13:5 - | -13 | x: std::io::Error, - | ^ multiple `provide` found - | - = note: candidate #1 is defined in an impl of the trait `Provider` for the type `E` - = note: candidate #2 is defined in an impl of the trait `std::error::Error` for the type `std::io::Error` -help: disambiguate the associated function for candidate #1 - | -13 | Provider::provide(&x, Error): std::io::Error, - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -help: disambiguate the associated function for candidate #2 - | -13 | std::error::Error::provide(&x, Error): std::io::Error, - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 3bcad5957d22c0331dbb65580eb42b66daf782f3 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Mon, 5 Sep 2022 14:31:18 -0700 Subject: [PATCH 2/6] Revert "Directly call source.provide instead of going through dyn error" This reverts commit f924c251ecae1b4cce08422ed5f7a7eb69776ff9. --- impl/src/expand.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/impl/src/expand.rs b/impl/src/expand.rs index fac6d6f..f2f7116 100644 --- a/impl/src/expand.rs +++ b/impl/src/expand.rs @@ -67,12 +67,12 @@ fn impl_struct(input: Struct) -> TokenStream { let source_provide = if type_is_option(source_field.ty) { quote_spanned! {source.span()=> if let std::option::Option::Some(source) = &self.#source { - source.provide(#demand); + source.as_dyn_error().provide(#demand); } } } else { quote_spanned! {source.span()=> - self.#source.provide(#demand); + self.#source.as_dyn_error().provide(#demand); } }; let self_provide = if source == backtrace { @@ -89,8 +89,7 @@ fn impl_struct(input: Struct) -> TokenStream { }) }; quote! { - #[allow(unused_imports)] - use std::error::Error as _; + use thiserror::__private::AsDynError; #source_provide #self_provide } @@ -260,12 +259,12 @@ fn impl_enum(input: Enum) -> TokenStream { let source_provide = if type_is_option(source_field.ty) { quote_spanned! {source.span()=> if let std::option::Option::Some(source) = #varsource { - source.provide(#demand); + source.as_dyn_error().provide(#demand); } } } else { quote_spanned! {source.span()=> - #varsource.provide(#demand); + #varsource.as_dyn_error().provide(#demand); } }; let self_provide = if type_is_option(backtrace_field.ty) { @@ -285,8 +284,7 @@ fn impl_enum(input: Enum) -> TokenStream { #source: #varsource, .. } => { - #[allow(unused_imports)] - use std::error::Error as _; + use thiserror::__private::AsDynError; #source_provide #self_provide } @@ -300,18 +298,17 @@ fn impl_enum(input: Enum) -> TokenStream { let source_provide = if type_is_option(source_field.ty) { quote_spanned! {backtrace.span()=> if let std::option::Option::Some(source) = #varsource { - source.provide(#demand); + source.as_dyn_error().provide(#demand); } } } else { quote_spanned! {backtrace.span()=> - #varsource.provide(#demand); + #varsource.as_dyn_error().provide(#demand); } }; quote! { #ty::#ident {#backtrace: #varsource, ..} => { - #[allow(unused_imports)] - use std::error::Error as _; + use thiserror::__private::AsDynError; #source_provide } } From 293b127bc8971c1b629719ae8702c9b15ac2a263 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Mon, 5 Sep 2022 14:43:58 -0700 Subject: [PATCH 3/6] Add build script to detect Provider support --- build.rs | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 build.rs diff --git a/build.rs b/build.rs new file mode 100644 index 0000000..004dfb0 --- /dev/null +++ b/build.rs @@ -0,0 +1,66 @@ +use std::env; +use std::fs; +use std::path::Path; +use std::process::{Command, ExitStatus, Stdio}; +use std::str; + +// This code exercises the surface area that we expect of the Provider API. If +// the current toolchain is able to compile it, then thiserror is able to use +// providers for backtrace support. +const PROBE: &str = r#" + #![feature(provide_any)] + + use std::any::{Demand, Provider}; + + fn _f<'a, P: Provider>(p: &'a P, demand: &mut Demand<'a>) { + p.provide(demand); + } +"#; + +fn main() { + match compile_probe() { + Some(status) if status.success() => println!("cargo:rustc-cfg=provide_any"), + _ => {} + } +} + +fn compile_probe() -> Option { + let rustc = env::var_os("RUSTC")?; + let out_dir = env::var_os("OUT_DIR")?; + let probefile = Path::new(&out_dir).join("probe.rs"); + fs::write(&probefile, PROBE).ok()?; + + // Make sure to pick up Cargo rustc configuration. + let mut cmd = if let Some(wrapper) = env::var_os("RUSTC_WRAPPER") { + let mut cmd = Command::new(wrapper); + // The wrapper's first argument is supposed to be the path to rustc. + cmd.arg(rustc); + cmd + } else { + Command::new(rustc) + }; + + cmd.stderr(Stdio::null()) + .arg("--edition=2018") + .arg("--crate-name=thiserror_build") + .arg("--crate-type=lib") + .arg("--emit=metadata") + .arg("--out-dir") + .arg(out_dir) + .arg(probefile); + + if let Some(target) = env::var_os("TARGET") { + cmd.arg("--target").arg(target); + } + + // If Cargo wants to set RUSTFLAGS, use that. + if let Ok(rustflags) = env::var("CARGO_ENCODED_RUSTFLAGS") { + if !rustflags.is_empty() { + for arg in rustflags.split('\x1f') { + cmd.arg(arg); + } + } + } + + cmd.status().ok() +} From 460396e8f3ed2f6615372961920fb32daa7376c9 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Mon, 5 Sep 2022 14:44:44 -0700 Subject: [PATCH 4/6] Add trait with method that won't collide between Provider and Error --- src/lib.rs | 5 +++++ src/provide.rs | 15 +++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 src/provide.rs diff --git a/src/lib.rs b/src/lib.rs index 6666584..3d6ad81 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -208,9 +208,12 @@ clippy::return_self_not_must_use, clippy::wildcard_imports, )] +#![cfg_attr(provide_any, feature(provide_any))] mod aserror; mod display; +#[cfg(provide_any)] +mod provide; pub use thiserror_impl::*; @@ -219,4 +222,6 @@ pub use thiserror_impl::*; pub mod __private { pub use crate::aserror::AsDynError; pub use crate::display::{DisplayAsDisplay, PathAsDisplay}; + #[cfg(provide_any)] + pub use crate::provide::ThiserrorProvide; } diff --git a/src/provide.rs b/src/provide.rs new file mode 100644 index 0000000..524e743 --- /dev/null +++ b/src/provide.rs @@ -0,0 +1,15 @@ +use std::any::{Demand, Provider}; + +pub trait ThiserrorProvide: Sealed { + fn thiserror_provide<'a>(&'a self, demand: &mut Demand<'a>); +} + +impl ThiserrorProvide for T { + #[inline] + fn thiserror_provide<'a>(&'a self, demand: &mut Demand<'a>) { + self.provide(demand); + } +} + +pub trait Sealed {} +impl Sealed for T {} From aaf8449dcb25b31a24c39056c420afc99416e7b0 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Mon, 5 Sep 2022 14:45:05 -0700 Subject: [PATCH 5/6] Use ThiserrorProvide to disambiguate 'provide' method calls --- impl/src/expand.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/impl/src/expand.rs b/impl/src/expand.rs index f2f7116..4352209 100644 --- a/impl/src/expand.rs +++ b/impl/src/expand.rs @@ -67,12 +67,12 @@ fn impl_struct(input: Struct) -> TokenStream { let source_provide = if type_is_option(source_field.ty) { quote_spanned! {source.span()=> if let std::option::Option::Some(source) = &self.#source { - source.as_dyn_error().provide(#demand); + source.thiserror_provide(#demand); } } } else { quote_spanned! {source.span()=> - self.#source.as_dyn_error().provide(#demand); + self.#source.thiserror_provide(#demand); } }; let self_provide = if source == backtrace { @@ -89,7 +89,7 @@ fn impl_struct(input: Struct) -> TokenStream { }) }; quote! { - use thiserror::__private::AsDynError; + use thiserror::__private::ThiserrorProvide; #source_provide #self_provide } @@ -259,12 +259,12 @@ fn impl_enum(input: Enum) -> TokenStream { let source_provide = if type_is_option(source_field.ty) { quote_spanned! {source.span()=> if let std::option::Option::Some(source) = #varsource { - source.as_dyn_error().provide(#demand); + source.thiserror_provide(#demand); } } } else { quote_spanned! {source.span()=> - #varsource.as_dyn_error().provide(#demand); + #varsource.thiserror_provide(#demand); } }; let self_provide = if type_is_option(backtrace_field.ty) { @@ -284,7 +284,7 @@ fn impl_enum(input: Enum) -> TokenStream { #source: #varsource, .. } => { - use thiserror::__private::AsDynError; + use thiserror::__private::ThiserrorProvide; #source_provide #self_provide } @@ -298,17 +298,17 @@ fn impl_enum(input: Enum) -> TokenStream { let source_provide = if type_is_option(source_field.ty) { quote_spanned! {backtrace.span()=> if let std::option::Option::Some(source) = #varsource { - source.as_dyn_error().provide(#demand); + source.thiserror_provide(#demand); } } } else { quote_spanned! {backtrace.span()=> - #varsource.as_dyn_error().provide(#demand); + #varsource.thiserror_provide(#demand); } }; quote! { #ty::#ident {#backtrace: #varsource, ..} => { - use thiserror::__private::AsDynError; + use thiserror::__private::ThiserrorProvide; #source_provide } } From 01e7c183100b63945f8d079c05b3bcc1aa674d60 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Tue, 13 Sep 2022 11:33:54 -0700 Subject: [PATCH 6/6] Temporarily disable AnyhowBacktrace test --- tests/test_backtrace.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_backtrace.rs b/tests/test_backtrace.rs index 43f68b8..6103df9 100644 --- a/tests/test_backtrace.rs +++ b/tests/test_backtrace.rs @@ -140,7 +140,8 @@ pub mod structs { let error = AnyhowBacktrace { source: anyhow::Error::msg("..."), }; - assert!(any::request_ref::(&error).is_some()); + // FIXME: change back to is_some after `impl Provider for anyhow::Error` exists. + assert!(any::request_ref::(&error).is_none()); let error = BoxDynErrorBacktrace { source: Box::new(PlainBacktrace {