Implement FromIterator for arrays up to [T; 32]

[ollie27]

I have used basically the same macro as for Default.
It will panic! if the iterator is not exactly the correct length.
This goes some way to solving rust-lang/rfcs#1109.

For example:

fn from_iter<I: IntoIterator<Item=T>>(iterator: I) -> [T; 3] {
    let mut i = iterator.into_iter();
    let a = [
        i.next().expect("iterator too short"),
        i.next().expect("iterator too short"),
        i.next().expect("iterator too short"),
    ];
    assert!(i.next().is_none(), "iterator too long");
    a
}

This relies on the evaluation order within array expressions which I don't know is well defined yet. If it's not it will be easy to fix.
I have added some simple tests to src/libcoretest/array.rs.

[rust-highfive]

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

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[Gankra]

So the usual problem applies here:

• this is an ugly hack because Arrays suck to talk about generically
• this will probably bloat up the size of std's rlib non-trivially

It seems like we've basically called a freeze on picking this wound until we get a better solution to "the array problem".

CC @rust-lang/libs

[bluss]

Point of discussion: Behavior when the iterator has more elements than needed. It's perfectly possible for FromIterator to not consume more than it needs. It's possible for the user to access the unconsumed elements after that. I think that behavior is the more natural and expected one; just don't ask for more elements than you need; if there are more, it's logical to leave them.

[ollie27]

@bluss from_iter takes it's argument by value so as far as I know you can't continue to use the iterator after calling it or collect. What you suggest sounds useful but I don't think it can be done with just FromIterator.

[bluss]

Yes it can, consider this:

let mut r = 0..10;
let array: [i32; 5] = (&mut r).collect();
assert_eq!(r.next(), Some(5));

&mut I where I: Iterator implements Iterator, so the “by value” argument to from_iter may be a mutable reference to an iterator.

[ollie27]

I see, you can change it to:

let array: [i32; 5] = (&mut r).take(5).collect();

which I think conveys intent better and is how you would collect the first 5 elements into any other collection.

[bluss]

Sure. I still think it's wrong for the FromIterator impl to ask for more elements than it needs and I want to dicuss the pros and cons of that approach.

[oli-obk]

@bluss: i think in this situation it's actually the less error-prone way. So I don't think there's a clear way forward.

Options:

1. always consume entire Iterator, error if the Iterator is too long
2. consume exactly the number of elements needed for the array
3. implement FromIterator for Result<[T, N], ArrayFillError> and return an Err(TooFew) or an Err(TooMany(array, last_elem)) so you never loose anything. Not sure how well .collect().unwrap() works with inference

[bluss]

Panic is appropriate for contract violation, and we use it for indexing errors for example.

Using it for not having enough elements, which could be a dynamic situation (you could filter more or less elements in an iterator chain for example) is not a good idea. I don't think it's a good fit for a contract violation. We say that “libraries shouldn't panic”, so libstd should live up to this as well.

An example of a better use is if an iterator promises to return n elements (with size_hint or ExactSizeIterator), and then it doesn't. Panic is appropriate, it's a contract violation.

Since the FromIterator trait cannot handle errors, I don't think it's a good idea for arrays.

This is derived from my reasoning around ArrayVec and FromIterator, which has the opposite conclusion due to different circumstances: The ArrayVec can fit 0 to N elements fine; since the trait does not have a way to handle errors; superfluous elements are skipped silently; this way the function is total without any panic cases. This is ok since they can still be reachable from the iterator.

[alexcrichton]

@gankro I'd actually think that there's a more fundamental problem here, as pointed out by @bluss there's a question of what if the iterator is too short or too long. All other implementors of FromIterator are dynamically sized and can hence work with an iterator of any size, but this would be the first instance which panics unless the iterator is of an exact length.

Tagging with T-libs to discuss during triage, but I agree with @bluss that this may not be appropriate for the standard library right now.

[alexcrichton]

The libs team discussed this during triage today, and the decision was that due to the panic concerns this isn't necessarily appropriate in the standard library for now, but it's certainly a neat idea!

Regardless, thanks again for the PR @ollie27!