Skip to content

RFC: Split Iterator into Iterator and FiniteIterator #74

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

Sawyer47
Copy link

Describes my proposal to split existing Iterator trait into Iterator and FiniteIterator traits. Some functions from Iterator shouldn't be invoked on infinite iterators.
It's my first attempt to contribute to Rust. I hope this RFC makes sense.

@huonw
Copy link
Member

huonw commented May 11, 2014

A subtlety here is you can have iterators that can be infinite but are often used in a non-infinite fashion, e.g.

repeat(10).take_while(|x| x < 100); // infinite
count(0, 1).take_while(|x| x < 100); // finite

Obviously those two examples are silly, but they do indicate that TakeWhile cannot (correctly) implement FiniteIterator; however I imagine most uses of TakeWhile will be as a finite iterator. That is, this change would restrict a reasonably large amount of legitimate behaviour.

@huonw
Copy link
Member

huonw commented May 11, 2014

(I guess that adds to your Cycle edge case.)

Also, BTW, .collect isn't necessarily an infinite loop: the collection gets to decide how many elements of the iterator it takes, e.g. one could have VectorLimited20 that takes the first (at most) 20 elements of the iterator.


# Alternatives

Not doing this would mean that it's possible to write generic code that uses Iterator trait bound

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still possible with these changes. An example would be maybe_infinite.take_while(|x| x < 100). The TakeWhile iterator is not always infinite, but sometimes it is. It can prevent some of these cases but not all. Since iterators and iterator adaptors can have effects, infinite loops may be intentional.

@Sawyer47
Copy link
Author

Thanks for feedback! I have changed my proposal to include only these functions from Iterator trait that always result in an infinite loop. I hope it is better now.

@Thiez
Copy link

Thiez commented May 12, 2014

This doesn't seem very useful to me. Misuse of an infinite iterator should be easy to detect because a program will crash or just hang. And as @huonw said, it's not always clear what some functions should return.

@pczarn
Copy link

pczarn commented May 12, 2014

I think that functions which may return a finite iterator should do it anyway. Is creating an InfiniteIterator instead an alternative? It seems confusing.

Alternative name for FiniteIterator: BoundedIterator. These names are long.

One way to resolve this is to make it clear that the Iterator trait iterates over a finite number of elements[1]. Create a new ItemStream trait as a base for Iterator. Move most functions to ItemStream. Leave only these that would always (unconditionally) enter an infinite loop, given an infinite stream and any predicate, to be provided by Iterator.
[1] http://stackoverflow.com/questions/2622591/is-an-infinite-iterator-bad-design

@pczarn
Copy link

pczarn commented May 15, 2014

I will try to flesh out the details of a possible solution as if they were documented, also per @huonw's request.

Trait core::iter::ItemStream

pub trait ItemStream<A> {
    fn next(&mut self) -> Option<A>;

    fn chain<U: ItemStream<A>>(self, other: U) -> Chain<Self, U> { ... }
    fn zip<B, U: ItemStream<B>>(self, other: U) -> Zip<Self, U> { ... }
    fn map<'r, B>(self, f: |A|: 'r -> B) -> Map<'r, A, B, Self> { ... }
    fn filter<'r>(self, predicate: |&A|: 'r -> bool) -> Filter<'r, A, Self> { ... }
    fn filter_map<'r, B>(self, f: |A|: 'r -> Option<B>) -> FilterMap<'r, A, B, Self> { ... }
    fn enumerate(self) -> Enumerate<Self> { ... }
    fn peekable(self) -> Peekable<A, Self> { ... }
    fn skip_while<'r>(self, predicate: |&A|: 'r -> bool) -> SkipWhile<'r, A, Self> { ... }
    fn take_while<'r>(self, predicate: |&A|: 'r -> bool) -> TakeWhile<'r, A, Self> { ... }
    fn skip(self, n: uint) -> Skip<Self> { ... }
    fn take(self, n: uint) -> Take<Self> { ... }
    fn scan<'r, St, B>(self, initial_state: St, f: |&mut St, A|: 'r -> Option<B>) -> Scan<'r, A, B, Self, St> { ... }
    fn flat_map<'r, B, U: ItemStream<B>>(self, f: |A|: 'r -> U) -> FlatMap<'r, A, Self, U> { ... }
    fn fuse(self) -> Fuse<Self> { ... }
    fn inspect<'r>(self, f: |&A|: 'r) -> Inspect<'r, A, Self> { ... }
    fn by_ref<'r>(&'r mut self) -> ByRef<'r, Self> { ... }
    fn advance(&mut self, f: |A| -> bool) -> bool { ... }
    fn collect<B: FromItemStream<A>>(&mut self) -> B { ... }
    fn nth(&mut self, n: uint) -> Option<A> { ... }
    fn all(&mut self, f: |A| -> bool) -> bool { ... }
    fn any(&mut self, f: |A| -> bool) -> bool { ... }
    fn find(&mut self, predicate: |&A| -> bool) -> Option<A> { ... }
    fn position(&mut self, predicate: |A| -> bool) -> Option<uint> { ... }
}

An item stream yields a (potentially-empty, potentially-infinite) sequence of values.

(Alternatively renamed to a different meaningful name.)

Implementors
Repeat<A>
Counter<A>
Chain<T, U>
Zip<T, U>
Cycle<T>
etc...

Trait core::iter::Iterator

trait<A> Iterator<A> : ItemStream<A> {
    fn size_hint(&self) -> (uint, Option<uint>) { ... } // doesn't it belong here?
    fn last(&mut self) -> Option<A> { ... }
    fn fold<B>(&mut self, init: B, f: |B, A| -> B) -> B { ... }
    fn len(&mut self) -> uint { ... }
    fn count(&mut self, predicate: |A| -> bool) -> uint { ... }
    fn max_by<B: TotalOrd>(&mut self, f: |&A| -> B) -> Option<A> { ... }
    fn min_by<B: TotalOrd>(&mut self, f: |&A| -> B) -> Option<A> { ... }
}

An interface for iterating over a collection or other (potentially-empty) finite sequence of values.

Implementors
Chain<T: Iterator<A>, U: Iterator<A>>
Zip<T: Iterator<A>, U: ItemStream<A>>
Zip<T: ItemStream<A>, U: Iterator<A>>
etc...

Yet another trait is possible, even though it has very little value:

trait<A> UnboundedStream<A> : ItemStream<A> {
    fn next_value(&mut self) -> A;
}

Traits AdditiveIterator, MultiplicativeIterator, OrdIterator are not implemented for T: ItemStream

@Sawyer47
Copy link
Author

I have realized that Zip<T, U> is also problematic. It's finite if either T or U are finite. It would need to be something like:

impl<A, B, T: ItemStream<A>, U: ItemStream<B>>
Iterator<(A, B)> for Zip<T, U> if T:Iterator<A> || U:Iterator<B>

@pczarn
Copy link

pczarn commented May 15, 2014

@Sawyer47 This alternation can be notated as two similar implementations. I'll add these to my comment.

Dzięki.

@Sawyer47
Copy link
Author

@pczarn Are you sure? This code doesn't compile:

trait ItemStream<A> {}
trait Iterator2<A> : ItemStream<A> {}

struct Zip<T, U> { a: T, b: U }

impl<A, B, T: ItemStream<A>, U: ItemStream<B>> ItemStream<(A, B)> for Zip<T, U> {}
impl<A, B, T: Iterator2<A>, U: ItemStream<B>> Iterator2<(A, B)> for Zip<T, U> {}
impl<A, B, T: ItemStream<A>, U: Iterator2<B>> Iterator2<(A, B)> for Zip<T, U> {}

fn main() { }

Compilation errors:

traits_zip.rs:7:1: 7:81 error: conflicting implementations for trait `Iterator2`
traits_zip.rs:7 impl<A, B, T: Iterator2<A>, U: ItemStream<B>> Iterator2<(A, B)> for Zip<T, U> {}
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
traits_zip.rs:8:1: 8:81 note: note conflicting implementation here
traits_zip.rs:8 impl<A, B, T: ItemStream<A>, U: Iterator2<B>> Iterator2<(A, B)> for Zip<T, U> {}
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
traits_zip.rs:8:1: 8:81 error: conflicting implementations for trait `Iterator2`
traits_zip.rs:8 impl<A, B, T: ItemStream<A>, U: Iterator2<B>> Iterator2<(A, B)> for Zip<T, U> {}
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
traits_zip.rs:7:1: 7:81 note: note conflicting implementation here
traits_zip.rs:7 impl<A, B, T: Iterator2<A>, U: ItemStream<B>> Iterator2<(A, B)> for Zip<T, U> {}
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
traits_zip.rs:7:1: 7:81 error: failed to find an implementation of trait Iterator2<B> for U
traits_zip.rs:7 impl<A, B, T: Iterator2<A>, U: ItemStream<B>> Iterator2<(A, B)> for Zip<T, U> {}

@pczarn
Copy link

pczarn commented May 16, 2014

@Sawyer47 Oh, multiple implementations of a trait in the current trait system are forbidden, because distinct parameters (<(A, B)> <T, U> in this case) are ignored. It could be issue rust-lang/rust#7590

@alexcrichton
Copy link
Member

Thanks for the RFC! The core team has decided, however, to not accept this for now. We believe that the increase in complexity isn't worth what' we're gaining from this RFC.

Closing.

withoutboats pushed a commit to withoutboats/rfcs that referenced this pull request Jan 15, 2017
added test for filter_map function of streams
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants