Functions that exhaust iterators should take them by value.

[Kimundi]

Functions that exhaust iterators should take them by value, even if they don't require ownership.

Example:

   foo.extend(&mut bar.move_iter());
-> foo.extend(bar.move_iter());

This optimizes the ergonomy of using them for the common case where you don't need access to the iterator anymore after it is empty, while still retaining the ability to do so with the by_ref() adapter:

let mut iter = bar.move_iter();
foo.extend(iter.by_ref());
iter.baz();

[japaric]

Are there any other methods that should be changed to take the iterator by value? I've seen that you already modified the extend and from_iterator methods.

[sfackler]

Are there any methods that take Iterators by reference at all? You can always use by_ref to get a "borrowed version" of an iterator if you don't want to give it away entirely to the function being called.

[Kimundi]

Last time I checked, most iterator methods that where not adapters took by reference.

[japaric]

Following the logic of the original issue, these Iterator methods should take the iterator by value, because they deplete it:

• collect
• count
• fold
• last
• max_by
• min_by

all, any and find are interesting because they may exhaust the iterator, or may leave it in some unknown state.

OTOH, advance, nth and position may exhaust the iterator, or may leave it in a known state (you know how many times it advanced).

As @sfackler said, if we change the methods to take the iterator by value, we can still reproduce the current behavior using the by_ref adapter. For example:

// move version of collect
fn collect<T, I: Iterator<T>, C: FromIterator<T>>(mut it: I) -> C {
    it.collect()
}

fn main() {
    let mut arr = [-1i, -2, -3];

    println!("move version");
    {
        let it1 = arr.iter_mut();  // non-copy iterator
        let it2 = range(0i, 3);  // copy iterator

        let c1: Vec<_> = collect(it1);
        let c2: Vec<_> = collect(it2);
        println!("{}", (c1, c2));
        //let c1: Vec<_> = collect(it1);  // ~error: moved
        let c2: Vec<_> = collect(it2);
        println!("{}", ((), c2));
    }

    println!("\n&mut version (is this behavior surprising?)");
    {
        let mut it1 = arr.iter_mut();
        let mut it2 = range(0i, 3);

        let c1: Vec<_> = it1.collect();
        let c2: Vec<_> = it2.collect();
        println!("{}", (c1, c2));
        let c1: Vec<_> = it1.collect();
        let c2: Vec<_> = it2.collect();
        println!("{}", (c1, c2));
    }

    println!("\n&mut version emulated via the move version");
    {
        let mut it1 = arr.iter_mut();
        let mut it2 = range(0i, 3);

        let c1: Vec<_> = collect(it1.by_ref());
        let c2: Vec<_> = collect(it2.by_ref());
        println!("{}", (c1, c2));
        let c1: Vec<_> = collect(it1.by_ref());
        let c2: Vec<_> = collect(it2.by_ref());
        println!("{}", (c1, c2));
    }
}

Prints

move version
([-1, -2, -3], [0, 1, 2])
((), [0, 1, 2])

&mut version (is this behavior surprising?)
([-1, -2, -3], [0, 1, 2])
([], [])

&mut version emulated via the move version
([-1, -2, -3], [0, 1, 2])
([], [])

IMO, I think the methods should take the iterator by value because depleted iterators are useless. Also, I think idiomatic code never calls these methods twice on the same iterator.

cc @aturon This should get sorted out as part of std::iter stabilization

[thestinger]

Taking the iterator by value for the non-adaptor methods is strictly less powerful than taking it by reference. It's not always possible to move a value.

[japaric]

It's not always possible to move a value.

Could you provide an example? (I thought this may be caused by the boxed closures, but the things I tried with a move collect worked)

[thestinger]

You can't move from &mut and partial moves from fields are quite restricted. There's no advantage to by-value for the non-adaptor methods so it's just going to add extra noise.

[thestinger]

Also, any method using by-value won't be available on a trait object. The adaptors use Self so that's an inherent problem with them anyway, but it's not for the non-adaptor methods.

[japaric]

You can't move from &mut and partial moves from fields are quite restricted.

Sounds reasonable

Also, any method using by-value won't be available on a trait object. The adaptors use Self so that's an inherent problem with them anyway, but it's not for the non-adaptor methods.

Can't really comment on traits objects, because I've hardly used dynamic dispatch. But I recall some discussion about this issue.

If making these Iterator methods take by value is going to add more problems, then it's better not do it.

Are there other methods that can take iterators by value but are taking them by reference? If not, then maybe this can be closed (?).

[thestinger]

I think it can be closed. It doesn't really apply to the methods on iterators themselves.

[aturon]

@japaric

To clarify a bit, the ergonomic tradeoff is different for adapter methods, because they will do the mutable borrowing automatically. The issue was talking about functions that take iterators as arguments; in that situation, you don't get automatic borrowing.