Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use internal iteration in the Sum and Product impls of Result and Option #62459

Merged
merged 1 commit into from Aug 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
146 changes: 37 additions & 109 deletions src/libcore/iter/adapters/mod.rs
Expand Up @@ -2063,137 +2063,65 @@ impl<I: ExactSizeIterator, F> ExactSizeIterator for Inspect<I, F>
impl<I: FusedIterator, F> FusedIterator for Inspect<I, F>
where F: FnMut(&I::Item) {}

/// An iterator adapter that produces output as long as the underlying
/// iterator produces `Option::Some` values.
pub(crate) struct OptionShunt<I> {
iter: I,
exited_early: bool,
}

impl<I, T> OptionShunt<I>
where
I: Iterator<Item = Option<T>>,
{
/// Process the given iterator as if it yielded a `T` instead of a
/// `Option<T>`. Any `None` value will stop the inner iterator and
/// the overall result will be a `None`.
pub fn process<F, U>(iter: I, mut f: F) -> Option<U>
where
F: FnMut(&mut Self) -> U,
{
let mut shunt = OptionShunt::new(iter);
let value = f(shunt.by_ref());
shunt.reconstruct(value)
}

fn new(iter: I) -> Self {
OptionShunt {
iter,
exited_early: false,
}
}

/// Consume the adapter and rebuild a `Option` value.
fn reconstruct<U>(self, val: U) -> Option<U> {
if self.exited_early {
None
} else {
Some(val)
}
}
}

impl<I, T> Iterator for OptionShunt<I>
where
I: Iterator<Item = Option<T>>,
{
type Item = T;

fn next(&mut self) -> Option<Self::Item> {
match self.iter.next() {
Some(Some(v)) => Some(v),
Some(None) => {
self.exited_early = true;
None
}
None => None,
}
}

fn size_hint(&self) -> (usize, Option<usize>) {
if self.exited_early {
(0, Some(0))
} else {
let (_, upper) = self.iter.size_hint();
(0, upper)
}
}
}

/// An iterator adapter that produces output as long as the underlying
/// iterator produces `Result::Ok` values.
///
/// If an error is encountered, the iterator stops and the error is
/// stored. The error may be recovered later via `reconstruct`.
pub(crate) struct ResultShunt<I, E> {
/// stored.
pub(crate) struct ResultShunt<'a, I, E> {
iter: I,
error: Option<E>,
error: &'a mut Result<(), E>,
}

impl<I, T, E> ResultShunt<I, E>
where I: Iterator<Item = Result<T, E>>
/// Process the given iterator as if it yielded a `T` instead of a
/// `Result<T, _>`. Any errors will stop the inner iterator and
/// the overall result will be an error.
pub(crate) fn process_results<I, T, E, F, U>(iter: I, mut f: F) -> Result<U, E>
where
I: Iterator<Item = Result<T, E>>,
for<'a> F: FnMut(ResultShunt<'a, I, E>) -> U,
{
/// Process the given iterator as if it yielded a `T` instead of a
/// `Result<T, _>`. Any errors will stop the inner iterator and
/// the overall result will be an error.
pub fn process<F, U>(iter: I, mut f: F) -> Result<U, E>
where F: FnMut(&mut Self) -> U
{
let mut shunt = ResultShunt::new(iter);
let value = f(shunt.by_ref());
shunt.reconstruct(value)
}

fn new(iter: I) -> Self {
ResultShunt {
iter,
error: None,
}
}

/// Consume the adapter and rebuild a `Result` value. This should
/// *always* be called, otherwise any potential error would be
/// lost.
fn reconstruct<U>(self, val: U) -> Result<U, E> {
match self.error {
None => Ok(val),
Some(e) => Err(e),
}
}
let mut error = Ok(());
let shunt = ResultShunt {
iter,
error: &mut error,
};
let value = f(shunt);
error.map(|()| value)
}

impl<I, T, E> Iterator for ResultShunt<I, E>
impl<I, T, E> Iterator for ResultShunt<'_, I, E>
where I: Iterator<Item = Result<T, E>>
{
type Item = T;

fn next(&mut self) -> Option<Self::Item> {
match self.iter.next() {
Some(Ok(v)) => Some(v),
Some(Err(e)) => {
self.error = Some(e);
None
}
None => None,
}
self.find(|_| true)
}

fn size_hint(&self) -> (usize, Option<usize>) {
if self.error.is_some() {
if self.error.is_err() {
(0, Some(0))
} else {
let (_, upper) = self.iter.size_hint();
(0, upper)
}
}

fn try_fold<B, F, R>(&mut self, init: B, mut f: F) -> R
where
F: FnMut(B, Self::Item) -> R,
R: Try<Ok = B>,
{
let error = &mut *self.error;
self.iter
.try_fold(init, |acc, x| match x {
Ok(x) => LoopState::from_try(f(acc, x)),
Err(e) => {
*error = Err(e);
LoopState::Break(Try::from_ok(acc))
}
})
.into_try()
}
}
2 changes: 1 addition & 1 deletion src/libcore/iter/mod.rs
Expand Up @@ -360,7 +360,7 @@ pub use self::adapters::Flatten;
#[stable(feature = "iter_copied", since = "1.36.0")]
pub use self::adapters::Copied;

pub(crate) use self::adapters::{TrustedRandomAccess, OptionShunt, ResultShunt};
pub(crate) use self::adapters::{TrustedRandomAccess, process_results};

mod range;
mod sources;
Expand Down
10 changes: 5 additions & 5 deletions src/libcore/iter/traits/accum.rs
@@ -1,6 +1,6 @@
use crate::ops::{Mul, Add};
use crate::num::Wrapping;
use crate::iter::adapters::{OptionShunt, ResultShunt};
use crate::iter;

/// Trait to represent types that can be created by summing up an iterator.
///
Expand Down Expand Up @@ -139,7 +139,7 @@ impl<T, U, E> Sum<Result<U, E>> for Result<T, E>
fn sum<I>(iter: I) -> Result<T, E>
where I: Iterator<Item = Result<U, E>>,
{
ResultShunt::process(iter, |i| i.sum())
iter::process_results(iter, |i| i.sum())
}
}

Expand All @@ -153,7 +153,7 @@ impl<T, U, E> Product<Result<U, E>> for Result<T, E>
fn product<I>(iter: I) -> Result<T, E>
where I: Iterator<Item = Result<U, E>>,
{
ResultShunt::process(iter, |i| i.product())
iter::process_results(iter, |i| i.product())
}
}

Expand All @@ -180,7 +180,7 @@ where
where
I: Iterator<Item = Option<U>>,
{
OptionShunt::process(iter, |i| i.sum())
iter.map(|x| x.ok_or(())).sum::<Result<_, _>>().ok()
shepmaster marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -196,6 +196,6 @@ where
where
I: Iterator<Item = Option<U>>,
{
OptionShunt::process(iter, |i| i.product())
iter.map(|x| x.ok_or(())).product::<Result<_, _>>().ok()
}
}
7 changes: 5 additions & 2 deletions src/libcore/option.rs
Expand Up @@ -135,7 +135,7 @@

#![stable(feature = "rust1", since = "1.0.0")]

use crate::iter::{FromIterator, FusedIterator, TrustedLen, OptionShunt};
use crate::iter::{FromIterator, FusedIterator, TrustedLen};
use crate::{convert, fmt, hint, mem, ops::{self, Deref, DerefMut}};
use crate::pin::Pin;

Expand Down Expand Up @@ -1499,7 +1499,10 @@ impl<A, V: FromIterator<A>> FromIterator<Option<A>> for Option<V> {
// FIXME(#11084): This could be replaced with Iterator::scan when this
// performance bug is closed.

OptionShunt::process(iter.into_iter(), |i| i.collect())
iter.into_iter()
.map(|x| x.ok_or(()))
.collect::<Result<_, _>>()
.ok()
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/libcore/result.rs
Expand Up @@ -231,7 +231,7 @@
#![stable(feature = "rust1", since = "1.0.0")]

use crate::fmt;
use crate::iter::{FromIterator, FusedIterator, TrustedLen, ResultShunt};
use crate::iter::{self, FromIterator, FusedIterator, TrustedLen};
use crate::ops::{self, Deref, DerefMut};

/// `Result` is a type that represents either success ([`Ok`]) or failure ([`Err`]).
Expand Down Expand Up @@ -1343,7 +1343,7 @@ impl<A, E, V: FromIterator<A>> FromIterator<Result<A, E>> for Result<V, E> {
// FIXME(#11084): This could be replaced with Iterator::scan when this
// performance bug is closed.

ResultShunt::process(iter.into_iter(), |i| i.collect())
iter::process_results(iter.into_iter(), |i| i.collect())
}
}

Expand Down
17 changes: 17 additions & 0 deletions src/libcore/tests/iter.rs
Expand Up @@ -1082,6 +1082,23 @@ fn test_iterator_sum_result() {
assert_eq!(v.iter().cloned().sum::<Result<i32, _>>(), Ok(10));
let v: &[Result<i32, ()>] = &[Ok(1), Err(()), Ok(3), Ok(4)];
assert_eq!(v.iter().cloned().sum::<Result<i32, _>>(), Err(()));

#[derive(PartialEq, Debug)]
struct S(Result<i32, ()>);

impl Sum<Result<i32, ()>> for S {
fn sum<I: Iterator<Item = Result<i32, ()>>>(mut iter: I) -> Self {
// takes the sum by repeatedly calling `next` on `iter`,
// thus testing that repeated calls to `ResultShunt::try_fold`
// produce the expected results
Self(iter.by_ref().sum())
}
}

let v: &[Result<i32, ()>] = &[Ok(1), Ok(2), Ok(3), Ok(4)];
assert_eq!(v.iter().cloned().sum::<S>(), S(Ok(10)));
let v: &[Result<i32, ()>] = &[Ok(1), Err(()), Ok(3), Ok(4)];
assert_eq!(v.iter().cloned().sum::<S>(), S(Err(())));
}

#[test]
Expand Down