std::error::Error: change the error iterator producer

[haraldh]

To produce an error iterator std::error::Chain one had to call
<dyn Error>::chain(), which was not very ergonomic, because you have to
manually cast to a trait object, if you didn't already have one with
the type erased.

    let mut iter = (&my_error as &(dyn Error)).chain();
    // or
    let mut iter = <dyn Error>::chain(&my_error);
    // or
    let mut iter = Error::chain(&my_error);

The chain() method can't be implemented on the Error trait, because of
#69161

Chain::new() replaces <dyn Error>::chain() as a good alternative
without confusing users, why they can't use my_error.chain() directly.

The Error::sources() method doesn't have this problem, so implement
it more efficiently, than one could achieve this with Chain::new().skip(1).

Related: #58520

[rust-highfive]

r? @KodrAus

(rust-highfive has picked a reviewer for you, use r? to override)

[rust-log-analyzer]

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    Checking addr2line v0.14.0
error: associated function has missing stability attribute
   --> library/std/src/error.rs:139:5
    |
139 | /     fn sources(&self) -> Chain<'_> {
140 | |         Chain { current: self.source() }
    | |_____^

error: aborting due to previous error


error: could not compile `std`

To learn more, run the command again with --verbose.
command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "16" "--release" "--color" "always" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "/checkout/library/test/Cargo.toml" "--message-format" "json-render-diagnostics"
failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check
Build completed unsuccessfully in 0:01:31

[KodrAus]

Thanks for pushing this forward @haraldh!

If the only thing blocking us from an ergonomic chain method on the Error trait itself is #69161 that seems pretty point-in-time to me so I think keeping the .chain() method on dyn Error and adding it to the Error trait later seems fine to me. We don't seem to have made progress on #69161 but I don't think that's a reason to expose a Chain::new, which isn't an approach used by iterators generally (array::IntoIter::new is a temporary exception). If we wanted this approach I imagine we'd offer a free chain function in std::error instead.

[haraldh]

If we wanted this approach I imagine we'd offer a free chain function in std::error instead.

@KodrAus yeah, a pub fn error_chain() was my first approach... then again I thought, that it really is a Chain constructor in reality.

Feel free to close this PR, if it doesn't fit.

[KodrAus]

Since this is all unstable I don’t think there’s any harm in at least re-introducing .sources().

If we didn’t want to do that yet then adding an example on using iter::successors to iterate the chain of sources would be nice.

What do you think?

[haraldh]

TIL about std::iter::successors.

<dyn Error>::chain() is

std::iter::successors::<&(dyn Error), _>(Some(&my_error), |e| e.source())

Error::sources() is

std::iter::successors::(my_error.source(), |e| e.source())

So, Error::Chain is basically obsolete

[KodrAus]

I think the subtle difference that makes an example worth adding is that we need to manually dereference the successor error in that closure, otherwise we run afoul of lifetime mismatches. So it looks a bit more like:

std::iter::successors::(my_error.source(), |&e| e.source())