Clone error in closure to prevent compiler lifetime errors.

[cr1901]

Presently, if you build simple-eyre from crates.io, you get the following compiler error:

william@xubuntu-dtrain:~/Projects/embedded/simple-eyre$ cargo build
    Updating crates.io index
   Compiling eyre v0.6.5
   Compiling indenter v0.3.3
   Compiling once_cell v1.7.2
   Compiling simple-eyre v0.3.0 (/home/william/Projects/embedded/simple-eyre)
error: lifetime may not live long enough
  --> src/lib.rs:86:65
   |
86 |             let errors = std::iter::successors(Some(cause), |e| e.source());
   |                                                              -- ^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
   |                                                              ||
   |                                                              |return type of closure is Option<&'2 dyn std::error::Error>
   |                                                              has type `&'1 &dyn std::error::Error`

error: aborting due to previous error

error: could not compile `simple-eyre`

To learn more, run the command again with --verbose.

I don't actually understand why clone() fixes this, but it does. I was test-driving simple-eyre to see if it fits my needs, and while it doesn't atm (plain eyre actually uses less .text :o), I figured I should make a PR :).

[yaahc]

There's a bit more of an explanation of what's going on here in this issue rust-lang/rust#81460. The gist is we added the impl impl<E> Error for &'_ E where E: Error to std and it breaks some code because the reference implementing error means it introduces a new reference and a new lifetime, instead of derefing in order to get to the inner error which it used to do before.

I've known about this issue since it popped up in crater but haven't fixed it yet because I mistakenly assumed it was in test code and wouldn't affect anyone, apologies for that! 😬

I was test-driving simple-eyre to see if it fits my needs, and while it doesn't atm (plain eyre actually uses less .text :o), I figured I should make a PR :).

I'm guessing this is because the reporting logic built into eyre doesn't end up being dead code, so you end up with two implementations of reporting. I can look into adding feature flags to eyre to disable it's own default reporter completely so that you must install a reporter or it panics, which would hopefully reduce the .text. Or maybe not, I'm kinda guessing here, this is a bit lower level than I normally go.

[cr1901]

The thing I don't get about issue rust-lang/rust#81460 (or rather, the error) is the following:

successors takes a function F that implements F: FnMut(&T) -> Option<T>. Okay, since we are passing in an Option<&(dyn Error + 'static)>, based on the type bounds for successors, &T must be of type &'1 &dyn std::error::Error. I.e. an extra layer of reference is added. So far so good.

As of 1.51 we have the following implementation for Error: impl<'a, T: Error + ?Sized> Error for &'a T:

• dyn Error will implement Error.
• Thanks to the ?Sized bound, &'a dyn Error will also implement Error.
• Since &'a dyn Error implements Error, so will &'b &'a dyn Error, etc.

Still no problems.

The signature of source is: fn source(&self) -> Option<&(dyn Error + 'static)>. Without lifetime elision, &'a self, and Option<&'a (dyn Error + 'static)>. I don't understand why the error message refers to two lifetimes '1 and '2.

That said, I think I get your explanation: because a new Error impl exists for references, deref coercion no longer occurs. The lifetime of the source return value in |e| e.source() becomes tied to a lifetime that begins and ends inside successors rather than a lifetime tied to the inner error type? I haven't looked at the body of successors, but I'm guessing the fact that the return value of the closure escapes is the problem?

[cr1901]

I can look into adding feature flags to eyre to disable it's own default reporter completely so that you must install a reporter or it panics, which would hopefully reduce the .text. Or maybe not, I'm kinda guessing here, this is a bit lower level than I normally go.

Here is a link to the relevant repo, which branches main and simple-eyre to compare. The code is meant for a MIPS target, but you can run just build-local on a x86_64 Linux system to do a comparison of binary sizes without creating a MIPS toolchain.

[yaahc]

That said, I think I get your explanation: because a new Error impl exists for references, deref coercion no longer occurs. The lifetime of the source return value in |e| e.source() becomes tied to a lifetime that begins and ends inside successors rather than a lifetime tied to the inner error type? I haven't looked at the body of successors, but I'm guessing the fact that the return value of the closure escapes is the problem?

I think you hit the nail on the head with this explanation, but successors doesn't really play into the error, I think its an issue with how closures bind lifetimes.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=1f796e6dacfebc9faf3fdf33679fac84

Though I don't really understand why the function version here works... my best guess is that either the elided lifetime influences auto deref behavior with resolving the method or I really still don't understand how lifetimes work 🥴

[cr1901]

FWIW, it works when I add explicit lifetime annotations to the closure: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=2a0f2f0e40b8c2942d58d4ffdaaadcf6

I really still don't understand how lifetimes work

Preaching to the choir here :).