libs: stabilize iter module

[aturon]

This is an initial pass at stabilizing the iter module. The module is
fairly large, but is also pretty polished, so most of the stabilization
leaves things as they are.

Some changes:

• Due to the new object safety rules, various traits needs to be split
  into object-safe traits and extension traits. This includes Iterator
  itself. While splitting up the traits adds some complexity, it will
  also increase flexbility: once we have automatic impls of Trait for
  trait objects over Trait, then things like the iterator adapters
  will all work with trait objects.
• Iterator adapters that use up the entire iterator now take it by
  value, which makes the semantics more clear and helps catch bugs. Due
  to the splitting of Iterator, this does not affect trait objects. If
  the underlying iterator is still desired for some reason, by_ref can
  be used. (Note: this change had no fallout in the Rust distro except
  for the useless mut lint.)
• In general, extension traits new and old are following an in-progress
  convention. As such, they
  are marked unstable.
• As usual, anything involving closures is unstable pending unboxed
  closures.
• A few of the more esoteric/underdeveloped iterator forms (like
  RandomAccessIterator and MutableDoubleEndedIterator, along with
  various unfolds) are left experimental for now.
• The order submodule is left experimental because it will hopefully
  be replaced by generalized comparison traits.
• "Leaf" iterators (like Repeat and Counter) are uniformly
  constructed by free fns at the module level. That's because the types
  are not otherwise of any significance (if we had impl Trait, you
  wouldn't want to define a type at all).

Closes #17701

Due to renamings and splitting of traits, this is a:

[breaking-change]

[tbu-]

Iterator becoming object safe \o/

[alexcrichton]

We've had concerns in the past about peekable iterators, perhaps this could be #[unstable] for now?

[aturon]

Sure thing. What were the concerns?

[alexcrichton]

The example I've seen before is along the lines of:

fn main() {
    let vec = vec![1, 2, 3, 4, 5];
    let mut iter = vec.into_iter().peekable(); // peekable makes no difference
    let a: Vec<int> = iter.by_ref().take_while(is_odd).collect(); // takes 1, drops 2
    let b: Vec<int> = iter.by_ref().take_while(is_even).collect(); // drops 3
    let c: Vec<int> = iter.by_ref().take_while(is_odd).collect();  // drops 4
    let rest: Vec<int> = iter.collect();
    println!("a={} b={} c={} rest={}", a, b, c, rest);
} // prints a=[1], b=[], c=[], rest=[5]

fn is_odd(&x: &int) -> bool { 
    x % 2 == 1
}

fn is_even(&x: &int) -> bool {
    x % 2 == 0
}

The problem here being that the methods like skip_while and take_while consume elements even though they in theory can preserve the elements of the underlying iterator if its peekable. Which is to say that SkipWhile<Peekable<T>> can actually not consume an element when the SkipWhile starts to return None, but it has to perform like SkipWhile<T> where it always consumes the element that returns None.

This may be a problem with skip_while or take_while as well, just wanted to make sure that these concerns were here.

I know that I've personally found it somewhat clunky to use. I generally only have an Iterator<T> instead of a Peekable<Iterator<T>> and then having to pass a Peekable all over the place is pretty painful. That's pretty fundamental though, and I'm not sure there's anything that we can do to alleviate my personal concern here.

[aturon]

This sounds like an argument that skip_while and take_while should use peek internally, so really a complaint about those adapters rather than peekable itself. Is that right?

[alexcrichton]

Kind of I think, more that skip_while and take_while should only be defined on peekable iterators. The other suggestion was to make peek-ability a trait rather than an iterator.

[alexcrichton]

@aturon just one design question about self vs &mut self on iterator methods, but other than that looks good to me.

[aturon]

Thanks @alexcrichton! I'm going to look into the by-value convention.

@thestinger, given that the splitting of traits here (necessitated by the new object safety rules) now makes all these adapters usable on trait objects in principle, is the only downside you see the additional by_ref calls that would be needed?

[aturon]

@alexcrichton I've pushed an update that switches to by-value where it makes sense. There was literally no fallout in all of the Rust distro (other than useless mut warnings). Given that we're splitting the trait (so these will be usable on trait objects), and that you can use by_ref (which should be very rare when using a consuming operation) this seems like a pure win from a clarify/bug catching perspective.

I've also tweaked the stability attributes of MinMaxResult since we'll probably want to revisit the naming in an enum sweep.

I'm leaving peekable/Peekable stable, since the changes you talked about would actually only affect take_while. (Note that skip_while works fine.) There are a few options for revising the semantics, but I want to visit that issue in a second pass; I've left a note so we remember to do so.

[Gankra]

Can/should we decouple these? ExactSize makes sense for HashMap but DoubleEndedIterator doesn't really. Although granted ExactSize is only used for DoubledEndedIteration. CC #19327

[bluss]

I concur. ExactSize has become something more than it was when introduced (used for backwards enumeration).