[Python-Dev] cpython: Implement PEP 380 - 'yield from' (closes #11682)

[Georg Brandl]

On 01/14/2012 07:53 AM, Nick Coghlan wrote:

>>> +PyAPI_FUNC(int) PyGen_FetchStopIterationValue(PyObject **);
>>
>> Does this API need to be public? If yes, it needs to be documented.
> 
> Hmm, good point - that one needs a bit of thought, so I've put it on
> the tracker: http://bugs.python.org/issue13783
> 
> (that issue also covers your comments regarding the docstring for this
> function and whether or not we even need the StopIteration instance
> creation API)

Great.

>>> -#define CALL_FUNCTION        131     /* #args + (#kwargs<<8) */
>>> -#define MAKE_FUNCTION        132     /* #defaults + #kwdefaults<<8 + #annotations<<16 */
>>> -#define BUILD_SLICE  133     /* Number of items */
>>> +#define CALL_FUNCTION   131     /* #args + (#kwargs<<8) */
>>> +#define MAKE_FUNCTION   132     /* #defaults + #kwdefaults<<8 + #annotations<<16 */
>>> +#define BUILD_SLICE     133     /* Number of items */
>>
>> Not sure putting these and all the other cosmetic changes into an already
>> big patch is such a good idea...
> 
> I agree, but it's one of the challenges of a long-lived branch like
> the PEP 380 one (I believe some of these cosmetic changes started life
> in Greg's original patch and separating them out would have been quite
> a pain). Anyone that wants to see the gory details of the branch
> history can take a look at my bitbucket repo:
> 
> https://bitbucket.org/ncoghlan/cpython_sandbox/changesets/tip/branch%28%22pep380%22%29

I see.  I hadn't followed the development of PEP 380 closely before.

In any case, it is probably a good idea to mention this branch URL in the
commit message in case it is meant to be kept permanently  (it would also be
possible to put only that branch of your sandbox into another clone at
hg.python.org).

>>> diff --git a/Objects/abstract.c b/Objects/abstract.c
>>> --- a/Objects/abstract.c
>>> +++ b/Objects/abstract.c
>>> @@ -2267,7 +2267,6 @@
>>>
>>>      func = PyObject_GetAttrString(o, name);
>>>      if (func == NULL) {
>>> -        PyErr_SetString(PyExc_AttributeError, name);
>>>          return 0;
>>>      }
>>>
>>> @@ -2311,7 +2310,6 @@
>>>
>>>      func = PyObject_GetAttrString(o, name);
>>>      if (func == NULL) {
>>> -        PyErr_SetString(PyExc_AttributeError, name);
>>>          return 0;
>>>      }
>>>      va_start(va, format);
>>
>> These two changes also look suspiciously unrelated?
> 
> IIRC, I removed those lines while working on the patch because the
> message they produce (just the attribute name) is worse than the one
> produced by the call to PyObject_GetAttrString (which also includes
> the type of the object being accessed). Leaving the original
> exceptions alone helped me track down some failures I was getting at
> the time.

I agree that it's useful.

> I've now made the various CallMethod helper APIs in abstract.c (1
> public, 3 private) consistently leave the GetAttr exception alone and
> added an explicit C API note to NEWS.
> 
> (Vaguely related tangent: the new code added by the patch probably has
> a few parts that could benefit from the new GetAttrId private API)

Maybe another candidate for an issue, so that we don't forget?

cheers,
Georg