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

[Sean Farley]

Martin von Zweigbergk <martinvonz at google.com> writes:

> On Wed, Jun 21, 2017 at 10:35 AM, Sean Farley <sean at farley.io> wrote:
>> Yuya Nishihara <yuya at tcha.org> writes:
>>
>>> 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.
>>
>> The reason I brought it up is that python3.6 now emits a warning about
>> 'raise StopIteration' being deprecated:
>>
>> https://www.python.org/dev/peps/pep-0479/
>
> But that's *inside generators* and Yuya was saying that the remaining
> few cases in our code base are not inside generator (functions). Maybe
> I misunderstood, but my impression was that that would still be
> allowed.

Yeah, I was confused; sorry!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170621/a97fe63e/attachment.sig>