iterator-related cleanup

[thestinger]

The extra::iter module wasn't actually included in extra.rs when it was moved from std... I assume no one is going to miss it.

[bstrie]

I'm obviously very much against removing .times(). The method is intended for dead-simple repetition, and to provide trivial examples of do notation, higher-order functions, and implementing methods on "builtin" types. It's great for tutorials and small examples. In short, it's good PR, and good PR is very important. for _ in range(0, x) is a poor replacement for these purposes. times is also 12 lines of code total: in other words, no maintenance burden whatsoever.

Constant allegations of cuteness aside, .times is found in hundreds of places thoughout the compiler. We'd always had the ability to write for range(0, x) |_| {, and yet people still elected to use .times instead. That alone should be evidence of its utility.

r- from me

[thestinger]

I fully intend to support the same zero-parameter sugar in for loops as for iterator {} instead of for pattern in iterator {} for when pattern is _. It won't be any longer than do is with Times right now. You could consider this to a temporary bit of ugliness like advance was.

There's nothing to gain from having an alternate way of looping lacking a way to break and making you encounter closure capture errors. There is a cognitive overhead to have many different looping conventions in the standard library - Times doesn't even implement the old internal iterator protocol that's still cluttered around.

[alexcrichton]

@thestinger, could times be modified to just return an iterator over () for now? That way once there's sugar for not capturing any patterns you'd still have the same syntax for 4.times { ... }? That would mean that all current users would be rewritten to for _ in 5.times {} which looks silly but it's on its way out.

[thestinger]

@alexcrichton: yeah, it could just be a wrapper around range(start, stop) doing range(0, stop) but it would be a function rather than a method. It would be odd to stick a times method on anything implementing Add + Ord - Times isn't implemented generically and doesn't work on anything but uint (including unsigned big integers).

We used used to have a repeat internal iterator doing the same thing as Times, but repeat is going to repeat a value forever now.

I decided just to copy Python's convention for now because times(i) doesn't make sense in the way that i.times does.

[alexcrichton]

I'm confused why this can't just be

pub trait Times {
    fn times(self) -> SomeIteratorOverUnit;
}

impl Times for uint {
    fn times(self) -> SomeIteratorOverUnit {
        range(0, self).transform(|_| ())
    }
}

[thestinger]

@alexcrichton: As a function, it can be implemented once generically for all types. As a trait, it adds yet another numeric trait every unsigned numeric type will have to implement. You need an implementation for i8, i16, i32, i64, uint, unsigned big integers, and any alternate numeric types such as overflow-checked ones in the future.

If we use traits that way, they're actually doing the opposite of code reuse. There will be third-party numeric types, so it makes the language less friendly to them by increasing the size of a minimal implementation.

[thestinger]

There's also no benefit from turning the result from an integer into (). It might as well just be a Range<T>.

[alexcrichton]

I'm still confused what the problem is, if you want it generically why won't this work

impl<T: Required + Traits + For + Range> Times for T {
    fn times(self) -> SomeIteratorOverUnit {
        range(0, self)
    }
}

[thestinger]

@alexcrichton: it will work, but if it's going to be in prelude it will reserve that name as a method (for types it can satisfy), which may not be wanted

[alexcrichton]

I wouldn't necessarily be in favor of it being in the prelude. This is more of a "cute" language feature than something core which should be present in every module's namespace in my opinion.

[thestinger]

I would really rather just have for range(0, n) { } as the one consistent way to do it.

[bstrie]

I'm happy at the prospect of being able to omit the _ in, but using range(0, x) to repeat something x times still feels like a kludge to me (I'm aware that this is what Python does, and I still disapprove). It runs the risk of the same off-by-one errors that C's for is vulnerable to.

As for implementation, I really don't care. My only concern is the API. I agree that times(i) would be pointless, as one of the driving forces for the x.times approach was to avoid having to add a name to the universal namespace.

Honestly I don't care if it gets used in the compiler, so I'm still indifferent to the bulk of this PR. But I do insist that this API be made available, whether with for or do.

[bluss]

The main commit regarding Times is no longer in the PR, but there are a few times → range conversions in the commit "remove extra::iter"

[thestinger]

I plan on opening the Times commit as a separate pull request. The compiler is not going to be using it regardless of any decision, because it's slow to compile closure-based loops.

An Iterator-based times would still be range, just behind a wrapper.