Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

Make &dyn Error and Box<dyn Error> implement Error #16

Open
KodrAus opened this issue Oct 14, 2020 · 3 comments
Open

Make &dyn Error and Box<dyn Error> implement Error #16

KodrAus opened this issue Oct 14, 2020 · 3 comments

Comments

@KodrAus
Copy link
Contributor

KodrAus commented Oct 14, 2020

This is a tracking issue to pursue the following trait impls:

impl<E: ?Sized + Error> Error for &'_ E;
impl<E: ?Sized + Error> Error for Box<E>;

Each of these impls comes with caveats that make them non-trivial to just introduce. We should try explore these caveats to see what it would take to make them possible.

@KodrAus
Copy link
Contributor Author

KodrAus commented Oct 14, 2020

The first impl can be added, but does cause a small amount of breakage: rust-lang/rust#75180

@KodrAus
Copy link
Contributor Author

KodrAus commented Oct 26, 2020

Here's what I've been thinking so far. I thought we could discuss it at our next meeting:

We somewhat hitched our wagon to dyn Error over impl Error when we stabilized the Error trait. It's not really useful to use generically, because the two standard methods for passing dynamic values, &dyn Error and Box<dyn Error> don't implement the Error trait, so can't be passed to methods accepting impl Error. We can make &dyn Error satisfy impl Error, but not Box<dyn Error>. For completeness, I think we should still pursue impl<'a, E: Error + ?Sized + 'a> Error for E independently of impl<'a, E: Error + ?Sized + 'a> Error for Box<E>. It breaks some cases where you want to call methods on a &&dyn Error, but those are currently very niche (only an example from eyre broke in a crater run).

Even without Box<dyn Error> implementing the Error trait, you can still manually deref it to get a value that satisfies impl Error through &dyn Error.

So now the question is should we recommend impl Error or dyn Error? Ideally it would be up to consumers to pick between impl and dyn based on their own cases, but I think errors are a bit special, since they imply you're off the happy path (it also doesn't help that for reasons we looked at above impl Error isn't very useful). I think that makes us lean more towards recommending dyn Error over impl Error when you want to abstract over error types.

What bound should we recommend users accept when they want to accept dynamic errors? Is it dyn Error or dyn Error + 'static?

I think the recommendation should be:

  • Prefer &dyn Error + 'static for borrowed dynamic errors.
  • Prefer Box<dyn Error + Send + Sync + 'static> for owned dynamic errors.

If you do really want impl Error then don't add a 'static bound unless you're taking ownership of the error. That's because impl Error + 'static prevents anybody with a Box<dyn Error + 'static> from being able to call your method. All they could get is a &(dyn Error + 'static), which will be impl Error, but not impl Error + 'static. If you do want to take ownership, then prefer impl Error + Send + Sync + 'static.

@KodrAus
Copy link
Contributor Author

KodrAus commented Oct 26, 2020

For a recommendation on how to accept errors abstractly, we discussed this at the recent project group meeting and settled on:

  • &dyn Error + 'static for borrowed abstract errors and
  • impl Error + Send + Sync + 'static for owned abstract errors.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant