Simplify Iterator::next for GenericShunt used in iter::try_process

[JustForFun88]

This pull request simplifies the iterator implementation for the GenericShunt structure used in iter::try_process. Although the old implementation commands respect and admiration, but:

1. To be honest, it is not clear not at first sight, nor from the second. And although it is delightful in its conciseness, I spent half a day trying to get through the wilds of mutual function calls.
2. Uses too many unnecessary branches and function calls (see code below). Also, using an implicit for + return loop instead of a direct iter.next() seems odd. While the compiler will likely optimize all of this, the implementation proposed in this pull request will probably compile faster.

In any case, feel free to just close this PR if you think it's redundant.

cc @scottmcm.

P.S. More explicit old code:

impl<I, R> Iterator for GenericShunt<'_, I, R>
where
    I: Iterator<Item: Try<Residual = R>>,
{
    type Item = <I::Item as Try>::Output;

    fn next(&mut self) -> Option<Self::Item> {
        let result = self.iter.try_fold((), |acc, x| match <I::Item as Try>::branch(x) {
            ControlFlow::Continue(x) => ControlFlow::Break(ControlFlow::Break(x)),
            ControlFlow::Break(r) => {
                *self.residual = Some(r);
                ControlFlow::Break(ControlFlow::Continue(acc))
            }
        });

        let out = match result {
            ControlFlow::Continue(val) => return None,
            ControlFlow::Break(val) => match val {
                ControlFlow::Continue(_) => return None,
                ControlFlow::Break(val) => val,
            },
        };
        Some(out)
    }
}

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @scottmcm (or someone else) soon.

Please see the contribution instructions for more information.

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[scottmcm]

This is a fairly standard way to not need to manually implement next -- notice that the same pattern is used here

rust/library/core/src/iter/adapters/array_chunks.rs

Lines 49 to 51 in 3de7d7f

	fn next(&mut self) -> Option<Self::Item> {
	self.try_for_each(ControlFlow::Break).break_value()
	}

There are lots of other ways it can be written, if any of these help you follow what it's doing:

self.try_for_each(Err).err()
self.find(|_| true)
self.find_map(Some)

but they're all using the same core idea, which is a loop that always returns the first value.

And if you look at the callers of try_process (https://github.com/rust-lang/rust/search?q=try_process), you'll see that they're doing things like sum or collect over the iterator, which tend to use the loop forms, not next:

rust/library/core/src/iter/traits/accum.rs

Lines 48 to 56 in a000811

	impl Sum for $a {
	fn sum<I: Iterator<Item=Self>>(iter: I) -> Self {
	iter.fold(
	$zero,
	#[rustc_inherit_overflow_checks]
	|a, b| a + b,
	)
	}
	}

So my instinct here is to stay with the concise code for now.

That said, if you have some benchmarks showing that doing next this way makes collecting into, say, a Result<Vec<_>, _> faster, then I'd be happy to reconsider.

@rustbot author

[JustForFun88]

That said, if you have some benchmarks showing that doing next this way makes collecting into, say, a Result<Vec<_>, _> faster, then I'd be happy to reconsider.

I tried as you said to compare the compiled code (old: https://rust.godbolt.org/z/K3WaKP4z7, new: https://rust.godbolt.org/z/cWjPj781r). With optimization level 3, the code in both versions is compiled into the same asm. The difference appears only when the optimization level is less than or equal to 1 and reaches 10%. That is, there should be an improvement only in compile time. Should I close the PR?

[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)

........................................................................................ 1496/1532
....................................
failures:

---- iter::traits::iterator::iter_try_collect_uses_try_fold_not_next stdout ----
thread 'iter::traits::iterator::iter_try_collect_uses_try_fold_not_next' panicked at 'Iterator::next should not be called!', library/core/tests/iter/traits/iterator.rs:561:13
error: test failed, to rerun pass `-p core --test coretests`

failures:
    iter::traits::iterator::iter_try_collect_uses_try_fold_not_next