Stream: project-error-handling

Topic: `impl<E: Error> Error for Arc<E>`


view this post on Zulip Richard Dodd (Dec 31 2020 at 14:41):

I'm using an error type that I need to be cheaply cloneable, but which contains types that aren't cloneable. The simple solution is to stick the inner types in an Arc (I don't need to mutate the error once its created). I noticed that there isn't an error impl on Arc, but there is for Box. There is already a Display impl for Arc, so adding an Error seems like an innocuous addition. Anyways I made a PR - let me know if you think there are any issues/problems with adding it.

view this post on Zulip Poliorcetics (Dec 31 2020 at 17:05):

I cannot tell you if it will be accepted or not, but if it is, doing it for Rc would be logical no ?

view this post on Zulip Richard Dodd (Dec 31 2020 at 17:12):

Good point.

view this post on Zulip Richard Dodd (Dec 31 2020 at 17:12):

I'll wait for feedback before doing that.

view this post on Zulip Richard Dodd (Dec 31 2020 at 17:40):

One problem could be that if someone impl Error for Arc<MyError> where MyError: Error, then their code would stop compiling. I would be surprised if anyone had actually done this.

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:25):

Richard Dodd said:

One problem could be that if someone impl Error for Arc<MyError> where MyError: Error, then their code would stop compiling. I would be surprised if anyone had actually done this.

as far as I can tell the orphan rules ban impls like this so that shouldn't be an issue

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:25):

   Compiling playground v0.0.1 (/playground)
error[E0117]: only traits defined in the current crate can be implemented for arbitrary types
 --> src/lib.rs:6:1
  |
6 | impl std::error::Error for Arc<Error> {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^----------
  | |                          |
  | |                          `Arc` is not defined in the current crate
  | impl doesn't use only types from inside the current crate
  |
  = note: define and implement a trait or new type instead

view this post on Zulip Richard Dodd (Dec 31 2020 at 18:29):

@Jane Lusby it will build if the Error type there is defined by you. This is to allow things like impl From<MyType> for TypeInStd.

view this post on Zulip Richard Dodd (Dec 31 2020 at 18:29):

^^ I think. It's a recent change (about 6 months ago?)

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:29):

I think it depends on if its "covered" or not

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:29):

i know what you mean

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:29):

i bookmarked the release notes that document the rules because they're so convoluted

view this post on Zulip Richard Dodd (Dec 31 2020 at 18:30):

Haha oh then you've taught me something! :)

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:30):

use std::sync::Arc;

#[derive(Debug)]
struct Error;

impl std::error::Error for Arc<Error> {

}

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:30):

this is how i tested it

view this post on Zulip Richard Dodd (Dec 31 2020 at 18:30):

Amazing, thankyou!

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:30):

https://blog.rust-lang.org/2020/01/30/Rust-1.41.0.html#relaxed-restrictions-when-implementing-traits

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:30):

extremely glad I bookmarked this one now, lol

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:31):

While it's still true that both From and Vec were foreign, the trait (in this case From) was parameterized by a local type. Therefore, Rust 1.41.0 allows this impl.

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:31):

https://rust-lang.github.io/rfcs/2451-re-rebalancing-coherence.html

view this post on Zulip Richard Dodd (Dec 31 2020 at 18:32):

Ahh so the trait has to be parameterised by the local type, not the thing its implemented on?

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:32):

Fundamental Type: A type for which you cannot add a blanket impl backwards compatibly. This includes &, &mut, and Box. Any time a type T is considered local, &T, &mut T, and Box<T> are also considered local. Fundamental types cannot cover other types. Any time the term "covered type" is used, &T, &mut T, and Box<T> are not considered covered.

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:33):

so Box is fundamental but Arc is not

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:33):

so this would have been an issue for Box, ironically, lol

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:33):

I'm not sure yet if its specifically the trait

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:33):

I think it is tho

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:33):

rereading the RFC rn

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:36):

When implementing a foreign trait for a foreign type, the trait must have one or more type parameters. A type local to your crate must appear before any use of any type parameters. This means that impl<T> ForeignTrait<LocalType<T>, T> for ForeignType is valid, but impl<T> ForeignTrait<T, LocalType<T>> for ForeignType is not.

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:36):

okay yea, just the trait

view this post on Zulip Richard Dodd (Dec 31 2020 at 18:37):

It feels like its basically been designed to cover From and nothing else :P

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:40):

wait no im confused again

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:40):

Given impl<P1..=Pn> Trait<T1..=Tn> for T0, an impl is valid only if at least one of the following is true:

* Trait is a local trait
* All of
        * At least one of the types T0..=Tn must be a local type. Let Ti be the first such type.
        * No uncovered type parameters P1..=Pn may appear in T0..Ti (excluding Ti)

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:41):

hmm

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:41):

this seems to imply that Arc<Error> should be implementable

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:41):

given the example of impl Error for Arc<MyError>

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:42):

wait no

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:42):

T0 = Arc<MyError> not MyError

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:42):

okay so that doesn't count as a local type, and the trait isn't local

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:42):

and there are no type parameters

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:42):

so it's invalid

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:42):

okay cool

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:43):

Richard Dodd said:

It feels like its basically been designed to cover From and nothing else :P

It's probably pretty useful for all the std::ops traits too

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:43):

just about anything with a type parameter on the trait itself

view this post on Zulip Jane Lusby (Dec 31 2020 at 18:43):

which makes sense

view this post on Zulip Notification Bot (Jan 04 2021 at 16:19):

This topic was moved here from #t-libs > impl<E: Error> Error for Arc<E> by simulacrum

view this post on Zulip Jane Lusby (Jan 06 2021 at 03:26):

@Ashley Mannix regarding https://github.com/rust-lang/rust/pull/80553#discussion_r552291239

view this post on Zulip Jane Lusby (Jan 06 2021 at 05:16):

actually nvm

view this post on Zulip Jane Lusby (Jan 06 2021 at 05:16):

i was gonna disagree but I think I changed my mind

view this post on Zulip Ashley Mannix (Jan 06 2021 at 07:25):

I was a bit back-and-forth on that comment myself :smile: But settled on thinking we should try make our future impls ?Sized and treat Box as an oddity for now with its common use with ? (I am still thinking an anyhow/eyre-like type should replace Box<dyn Error> someday)

view this post on Zulip Jane Lusby (Jan 06 2021 at 21:09):

yea

view this post on Zulip Jane Lusby (Jan 06 2021 at 21:09):

my take on it is that we should make the decision based on their relation to crates like eyre and anyhow

view this post on Zulip Jane Lusby (Jan 06 2021 at 21:09):

those crates are reporting types, and they mostly exist for convenience which is why they opt for From over Error

view this post on Zulip Jane Lusby (Jan 06 2021 at 21:09):

but Arc<dyn Error> and friends are not reporters

view this post on Zulip Jane Lusby (Jan 06 2021 at 21:10):

and the fact that box is implemented like one is probably not a good precident to follow

view this post on Zulip Jane Lusby (Jan 06 2021 at 21:10):

and I dont think Arc<dyn Error> and Box<dyn Error> should be thought of as alternatives to eyre and anyhow. If you want that it should be trivial to just wrap a box and add the from impl but not the error impl

view this post on Zulip Jane Lusby (Jan 06 2021 at 21:11):

so we should optimize for the smart pointer usecase not the dyn error usecase

view this post on Zulip Jane Lusby (Jan 06 2021 at 21:11):

its gonna be a gross inconsistency

view this post on Zulip Jane Lusby (Jan 06 2021 at 21:11):

but I think its for the best

view this post on Zulip Jane Lusby (Jan 06 2021 at 21:11):

the sooner we quarantine the weird behaviour the better

view this post on Zulip Jane Lusby (Jan 06 2021 at 21:12):

we should probably end up documenting the discrepancy somewhere

view this post on Zulip Ashley Mannix (Jan 06 2021 at 23:38):

Yeh, I guess Box<dyn Error> is our current standard library reporter but that's not really what we want. Box and Arc should just be details of the error's storage, not of its role

view this post on Zulip Jane Lusby (Jan 06 2021 at 23:38):

Exactly


Last updated: Jan 29 2022 at 10:51 UTC