rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.62k stars 12.48k forks source link

Generators should implement either Iterator or IntoIterator. #60062

Open lachlansneff opened 5 years ago

lachlansneff commented 5 years ago

Generators (currently unstable) are used as an implementation detail for async/await support in rust, but they can also be useful for writing Iterators and similar.

If you want to turn a Generator into an Iterator, you must wrap it in a new type, but it'd be useful to implement Iterator directory on Generator.

Here's an example of how that could work:

impl<G> Iterator for G where G: Generator<Return = ()> + Unpin {
    type Item = G::Yield;

    fn next(&mut self) -> Option<Self::Item> {
        match Pin::new(&mut self.generator).resume() {
            GeneratorState::Yielded(item) => Some(item),
            GeneratorState::Complete(_) => None,
        }
    }
}
DutchGhost commented 5 years ago

I've experiment with this in the past, and also talked to @Nemo157 about this. Basically we came up with an implementation using a little wrapper: https://gist.github.com/DutchGhost/5a6712f3d8b611ccbed6a9f120929e90

An interesting point is, do you allow panic's to happen? Your implementation does panic when .next() is called, and the previous call already finished the generator.

Another interesting point is, what to do with generators that are !Unpin? You can actually make an Iterator out of them directly using macro's, which handle's all the unsafe parts correctly. But that requires a macro, which may not be ideal.

lachlansneff commented 5 years ago

I don't have a problem with a panic occurring if .next() continues to be called. If the user wants the iterator to allows return None after that, they can just call .fuse() to turn it into a FusedIterator. Unpin generators are a more niche usecase and it wouldn't be a breaking change to support them in the future if it becomes possible.

I've seen the gen_iter crate and it's cool, but I'd rather just have Generators be iterators instead of having to wrap them.

Nemo157 commented 5 years ago

I don't have a problem with a panic occurring if .next() continues to be called. If the user wants the iterator to allows return None after that, they can just call .fuse() to turn it into a FusedIterator.

This is my position as well, but when discussing it on the community Discord there was a lot of pushback against it. The common opinion appears to be that the Iterator::next API guarantees one of the two options it mentions, either returning None forever the same as a FusedIterator, or restarting from the beginning (or some combination of the two, e.g. return None 10 times then restart from the beginning).

timvermeulen commented 5 years ago

@Nemo157 To be fair, the Iterator::next docs don't mention any possible panics, but other than that they don't really guarantee anything:

calling next() again may or may not eventually start returning Some(Item) again at some point.

Iterator::next isn't required to either be fused or restart at some point by any means (or the Chain adaptor, and I'm sure others, would currently be implemented incorrectly).

alexreg commented 5 years ago

@DutchGhost That looks like a good approach to me. I actually quickly hacked together something similar myself in the past. What do you mean about making an iterator out of !Unpin generators? How is that possible?

Either way, I think we definitely need this in the stdlib, just needs a bit more planning first.

tema3210 commented 4 years ago

If we had pin_cell in std then it could look like:

impl<G> Iterator for G where G: PinCell< Generator<Return = ()> > {
    type Item = G::Content::Yield;

    fn next(&mut self) -> Option<Self::Item> {
        match (self.borrow_pin_mut().unwrap()).resume() {
            GeneratorState::Yielded(item) => Some(item),
            GeneratorState::Complete(_) => None,
        }
    }
}