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

Martin von Zweigbergk martinvonz at google.com
Wed Jun 21 02:13:02 EDT 2017


On Tue, Jun 20, 2017 at 11:09 PM, Martin von Zweigbergk
<martinvonz at google.com> wrote:
> On Tue, Jun 20, 2017 at 8:38 PM, Gregory Szorc <gregory.szorc at gmail.com> wrote:
>> On Tue, Jun 20, 2017 at 3:24 PM, Martin von Zweigbergk via Mercurial-devel
>> <mercurial-devel at mercurial-scm.org> wrote:
>>>
>>> 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?
>>
>>
>> I'm not sure what the Python language itself specifies, but from the CPython
>> API, you need to return a PyObject from every function and NULL is
>> interpreted as an exception has been raised. If you have nothing to return,
>> you Py_RETURN_NONE to return None as a PyObject.
>>
>> Yes, the return value of "return" is None. However, "return" and "return
>> None" aren't semantically identical. For generators (functions containing
>> "yield"), it is illegal to have "yield" and "return x" in the same function:
>> you can only use bareword "return" for control flow purposes. So for this
>> patch, "return None" will raise SyntaxError since the function has "yield."
>
> So, in summary, you both think the patch is good? If so, could one of
> you queue it? :-)

Sorry, I realized after sending that that may have sounded pushy. I
also realize you may simply not have had time. I just meant to check
if either of you still head reservations about the patch (potential
wireproto changes is out of scope for this patch, IMO).


More information about the Mercurial-devel mailing list