[Python-Dev] cpython: Implement PEP 380 (original) (raw)

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

Nick Coghlan ncoghlan at gmail.com
Sat Jan 14 07:53:39 CET 2012

On Sat, Jan 14, 2012 at 1:17 AM, Georg Brandl <g.brandl at gmx.net> wrote:

On 01/13/2012 12:43 PM, nick.coghlan wrote:

diff --git a/Doc/reference/expressions.rst b/Doc/reference/expressions.rst There should probably be a "versionadded" somewhere on this page.

Good catch, I added versionchanged notes to this page, simple_stmts and the StopIteration entry in the library reference.

 PEP 3155: Qualified name for classes and functions  ================================================== This looks like a spurious (and syntax-breaking) change.

Yeah, it was an error I introduced last time I merged from default. Fixed.

diff --git a/Grammar/Grammar b/Grammar/Grammar -argument: test [compfor] | test '=' test  # Really [keyword '='] test +argument: (test) [compfor] | test '=' test  # Really [keyword '='] test This looks like a change without effect?

Fixed.

It was a lingering after-effect of Greg's original patch (which also modified the function call syntax to allow "yield from" expressions with extra parens). I reverted the change to the function call syntax, but forgot to ditch the added parens while doing so.

diff --git a/Include/genobject.h b/Include/genobject.h

-     /* List of weak reference. */ -     PyObject *giweakreflist; +        /* List of weak reference. */ +        PyObject *giweakreflist;  } PyGenObject; While these change tabs into spaces, it should be 4 spaces, not 8.

Fixed.

+PyAPIFUNC(int) PyGenFetchStopIterationValue(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)

-#define CALLFUNCTION        131     /* #args + (#kwargs<<8) */ -#define MAKEFUNCTION        132     /* #defaults + #kwdefaults<<8 + #annotations<<16 */ -#define BUILDSLICE  133     /* Number of items */ +#define CALLFUNCTION   131     /* #args + (#kwargs<<8) */ +#define MAKEFUNCTION   132     /* #defaults + #kwdefaults<<8 + #annotations<<16 */ +#define BUILDSLICE     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

diff --git a/Objects/abstract.c b/Objects/abstract.c --- a/Objects/abstract.c +++ b/Objects/abstract.c @@ -2267,7 +2267,6 @@

 func = PyObjectGetAttrString(o, name);  if (func == NULL) { -        PyErrSetString(PyExcAttributeError, name);  return 0;  } @@ -2311,7 +2310,6 @@  func = PyObjectGetAttrString(o, name);  if (func == NULL) { -        PyErrSetString(PyExcAttributeError, name);  return 0;  }  vastart(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'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)

diff --git a/Objects/genobject.c b/Objects/genobject.c +        } else { +            PyObject *e = PyStopIterationCreate(result); +            if (e != NULL) { +                PyErrSetObject(PyExcStopIteration, e); +                PyDECREF(e); +            } Wouldn't PyErrSetObject(PyExcStopIteration, value) suffice here anyway?

I think you're right - so noted in the tracker issue about the C API additions.

Thanks for the thorough review, a fresh set of eyes is very helpful :)

Cheers, Nick.

-- Nick Coghlan   |   ncoghlan at gmail.com   |   Brisbane, Australia

More information about the Python-Dev mailing list