Fallible extend with fallible iterator

language design
1

Disclaimer: This topic is not related to Fallible alternative to Extend or others talking about fallible allocation.

Currently, there is no simple way to extend a collection with fallible iterator, i.e. iterator that yield Result<T, E>. It can be implemented manually, reproducing core::iter::try_process, but it's cumbersome. Instead, I would like to propose the following non-breaking change to Extend trait:

pub trait Extend<A, R = ()> {
    fn extend<T: IntoIterator<Item = A>>(&mut self, iter: T) -> R;
}

impl<A, E, C: Extend<A>> Extend<Result<A, E>, Result<(), E>> for C {
    fn extend<T: IntoIterator<Item = A>>(&mut self, iter: T) -> Result<(), E> {
        iter::try_process(iter.into_iter(), |i| self.extend(i))
    }
}
1 Like
2

This is the part you are proposing?

3

As long as that generic

impl<A, E, C: Extend<A>> Extend<Result<A, E>, Result<(), E>> for C

implementation is the only implementation that offers Extent<A, Something> implementation with Something != (), then this could perhaps be served just as well with a helper function.

Also note that you can have a stable version of (something like) try_process by using itertools: process_results in itertools - Rust

Finally, one could debate what the best / correct behavior here is. In case of an error, is it expected & desired that the method did still end up extending the collection by all iterator values before the Err was encountered? If not, how to undo it? Or avoid any modification to begin with somehow? (So pre-collect into a Vec instead? But that's inefficient..) Perhaps make it from a fn(&mut Self) into a fn(Self) -> Self style API that can just discard the whole collection on error, as to not leave any half-modified state accessible? (So that’s a signature like fn(C, impl IntoIter<Item = Result<A, E>>) -> Result<C, E> then.)

4

Rather than a generic parameter, I'm fairly sure R should be an associated type, unless you intend that it should be possible to implement Extend to return different output types. Associated type defaults are unstable, but have the correct behavior to make adding a defaulted one non-breaking.

Extend::extend (potentially) having a non-() return type also influences what Iterator::collect_into (also unstable) should return.

Since we already have impl<A, E, V: FromIterator<A>> FromIterator<Result<A, E>> for Result<V, E> which discards any Oks before the first Err, it's a fair precedent to say that Extend should match FromIterator here. (Unstable collect_into makes this even more relevant a correspondence.) Which I think would actually be just impl<A, E, V: Extend<A>> Extend<Result<A, E>> for Result<V, E> instead.

If we had a &mut-forwarding impl for Extend (unfortunately we don't, and it'd be breaking to add, since references are #[fundamental]), it'd still enable non-intermediate-result-losing usage as let mut r = Ok(&mut c); r.extend(iter.by_ref());

1 Like
5

You're right, I was too much influenced by the FromIterator implementation on Result. We could rather add a default method to the trait, like:

trait Extend<A> {
    fn extend<T: IntoIterator<Item = A>>(&mut self, iter: T);
    fn try_extend<T: IntoIterator<Item = Result<A, E>>, E>(&mut self, iter: T) -> Result<(), E> {}
}

It seems better like this in fact – by the way, I also think a try_collect method would have been better than the pattern collect::<Result<_, _>>().

In my opinion, collection should be extended with the first Ok(_) values. It can be impossible for some collections to drop the extension, e.g. with HashMap, and as you said, pre-collecting in a Vec would kill the performance. Some collections like Vec would still be able to truncate to the state before the extension.

6

My intuition would be for it to consume the entire iterator and add the Ok variants to the collection— If you want the iterator to stop after the first error, use an adaptor for that and keep the concerns separate.

(I understand that this is inconsistent with collecting into Result, and therefore is unlikely to be added to std, but I never use that feature myself for the same reason.)

Edit: Speaking of an adaptor, something like this might be a useful addition to solidify the pattern established by Result as FromIter:

trait IteratorExt: Iterator {
    fn fuse_err<'a,R,E>(self, err: &'a mut Option<E>)->FuseErr<'a, Self, E> where Self: Sized+Iterator<Item=Result<R,E>> {
        FuseErr { err, iter:self }
    }
}

impl<I:Iterator> IteratorExt for I {}

struct FuseErr<'a, I, E> {
    err: &'a mut Option<E>,
    iter: I
}

impl<'a, I, R, E> Iterator for FuseErr<'a, I, E> where I:Iterator<Item=Result<R,E>> {
    type Item = R;
    fn next(&mut self)->Option<R> {
        if self.err.is_some() { return None; }
        match self.iter.next() {
            None => None,
            Some(Ok(r)) => Some(r),
            Some(Err(e)) => {
                *self.err = Some(e);
                None
            }
        }
    }
}