[PATCH] py3: catch StopIteration from next() in generatorset

[Yuya Nishihara]

On Tue, 20 Jun 2017 15:27:56 -0700, Sean Farley wrote:
> Martin von Zweigbergk <martinvonz at google.com> writes:
> 
> > On Tue, Jun 20, 2017 at 2:53 PM, Sean Farley <sean at farley.io> wrote:
> >> Martin von Zweigbergk via Mercurial-devel
> >> <mercurial-devel at mercurial-scm.org> writes:
> >>
> >>> # HG changeset patch
> >>> # User Martin von Zweigbergk <martinvonz at google.com>
> >>> # Date 1497992441 25200
> >>> #      Tue Jun 20 14:00:41 2017 -0700
> >>> # Node ID f86d21c457209f5f5a139bcc808be33fbd930424
> >>> # Parent  0906385672d754f13a512fe70759f0463a069f1e
> >>> py3: catch StopIteration from next() in generatorset
> >>>
> >>> IIUC, letting the StopIteration through would not cause any bugs, but
> >>> not doing it makes the test-py3-commands.t pass.
> >>>
> >>> I have also diligently gone through all uses of next() in our code
> >>> base. They either:
> >>>
> >>>  * are not called from a generator
> >>>  * pass a default value to next()
> >>>  * catch StopException
> >>>  * work on infinite iterators
> >>>  * request a fixed number of items that matches the generated number
> >>>  * are about batching in wireproto which I didn't quite follow
> >>>
> >>> I'd appreciate if Augie or someone else could take a look at the
> >>> wireproto batching and convince themselves that the next(batchable)
> >>> calls there will not raise a StopIteration.
> >>
> >> I was just thinking of doing something like this. Shouldn't we just
> >> 'return None' in Mercurial instead of 'raise StopIteration'?
> >
> > I thought "return" was just short for "return None". Are you just
> > saying that we prefer "return None" in Mercurial? Should I send a v2?
> 
> Sorry, no, I was just being sloppy ;-P I meant going through and
> replacing 'raise StopIteration' with 'return' (e.g. return None) and
> therefore we shouldn't need to catch StopIteration, correct? Here are
> the calls I found:
> 
> ../mercurial/commandserver.py
> 142:            raise StopIteration
> 
> ../mercurial/manifest.py
> 124:            raise StopIteration
> 146:            raise StopIteration
> 
> ../mercurial/patch.py
> 151:                raise StopIteration

IIUC, these StopIteration are valid since they are iterator/generator types.
On the other hand, generatorset._iterator.gen() is a generator function, which
should just end (without raising StopIteration) when all items are yielded.