Right now the thiserror macro will generate code that invokes the hidden AsDynError trait. However it doesn't use a fully qualified path, but instead uses method-call resolution, where an inherent method on the field type shadows the trait method.
Is this an intentional choice to allow external implementations of the method?
Context: In rootcause we got a feature request to support placing a rootcause report inside enums with thiserror impls using #[error(transparent)].
While we don't recommend this way of combining thiserror and rootcause, we do consider it a helpful feature to make migrations of large codebases easier.
We added the feature using a Deref impl, but we had to roll back the change.
However after rolling back the change, I noticed that the #[error(transparent)] code still worked. It turns out, this is because we also named our method .as_dyn_error(), so everything happened to still work.
This makes me a bit worried, as "internal" changes in thiserror can now cause accidental breakage in consumers that use both thiserror and rootcause.
It'd be good to know where you land on this. If external as_dyn_error() impls are fair game, then we will document that this is a supported (but non-recommended) way to combine rootcause with thiserror. Otherwise we'd rather you make the call fully-qualified so the behavior breaks now instead of by accident later.
Right now the thiserror macro will generate code that invokes the hidden
AsDynErrortrait. However it doesn't use a fully qualified path, but instead uses method-call resolution, where an inherent method on the field type shadows the trait method.Is this an intentional choice to allow external implementations of the method?
Context: In rootcause we got a feature request to support placing a rootcause report inside enums with thiserror impls using
#[error(transparent)].While we don't recommend this way of combining thiserror and rootcause, we do consider it a helpful feature to make migrations of large codebases easier.
We added the feature using a
Derefimpl, but we had to roll back the change.However after rolling back the change, I noticed that the
#[error(transparent)]code still worked. It turns out, this is because we also named our method.as_dyn_error(), so everything happened to still work.This makes me a bit worried, as "internal" changes in thiserror can now cause accidental breakage in consumers that use both thiserror and rootcause.
It'd be good to know where you land on this. If external
as_dyn_error()impls are fair game, then we will document that this is a supported (but non-recommended) way to combine rootcause with thiserror. Otherwise we'd rather you make the call fully-qualified so the behavior breaks now instead of by accident later.