Implement advance_by, advance_back_by for iter::Chain

[timvermeulen]

Part of #77404.

This PR does two things:

• implement Chain::advance[_back]_by in terms of advance[_back]_by on self.a and advance[_back]_by on self.b
• change Chain::nth[_back] to use advance[_back]_by on self.a and nth[_back] on self.b

This ensures that Chain::nth can take advantage of an efficient nth implementation on the second iterator, in case it doesn't implement advance_by.

cc @scottmcm in case you want to review this

[rust-highfive]

r? @joshtriplett

(rust_highfive has picked a reviewer for you, use r? to override)

[scottmcm]

Sure, I'll take this one.
r? @scottmcm

[scottmcm]

The code here all looks right to me, but I've become paranoid about early-exits and fusing in &mut self iterator methods.

Could you please add some tests for the edge cases of non-fused iterators? You're self.a = None;ing in the right places, AFAICT, but I don't think the tests right now would catch it if you weren't (since slice::Iter is always fused anyway).

If you need a non-fused iterator, you could use something like

pub fn make_silly_iterator(mut x: u8) -> impl Iterator<Item = u8> {
    std::iter::from_fn(move || {
        let i = x;
        x = x.wrapping_sub(1);
        if i == 0 { None } else { Some(i) }
    })
}

[timvermeulen]

Good call, I've been meaning to add more tests with non-fused iterators across the board because the current tests are barely using any. It wouldn't surprise me if some of the iterators currently contain bugs related to that.

I'm adding a NonFuse iterator wrapper to the test file that simply panics when next/next_back is called after the iterator has been exhausted, that should catch everything. And libcore iterators probably shouldn't be calling next on an exhausted iterator anyway, even when the value isn't used.

[timvermeulen]

The tests now indeed do fail if I leave out self.a = None;. Mutation tests would be nice to have here 🙂

[scottmcm]

Thanks! Looks like a plausible helper to use in more tests (try_fold jumps to mind, obviously).

@bors r+

[bors]

📌 Commit 1d27a50 has been approved by scottmcm

[ecstatic-morse]

@bors rollup=never

[bors]

⌛ Testing commit 1d27a50 with merge 5849a7e...

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: scottmcm
Pushing 5849a7e to master...

[Mark-Simulacrum]

As mentioned in #76909 (comment), this was a pretty bad regression on the deeply-nested stress test. https://perf.rust-lang.org/compare.html?start=5ded394553296d56bb66e925d7001ab3271979ce&end=5849a7eca90582ee59b67eb09548a2aa424d7f52&stat=instructions:u