Caution, long review ahead.
On 01/13/2012 12:43 PM, nick.coghlan wrote:
> http://hg.python.org/cpython/rev/d64ac9ab4cd0
> changeset: 74356:d64ac9ab4cd0
> user: Nick Coghlan <ncogh...@gmail.com>
> date: Fri Jan 13 21:43:40 2012 +1000
> summary:
> Implement PEP 380 - 'yield from' (closes #11682)
> diff --git a/Doc/reference/expressions.rst b/Doc/reference/expressions.rst
> --- a/Doc/reference/expressions.rst
> +++ b/Doc/reference/expressions.rst
> @@ -318,7 +318,7 @@
There should probably be a "versionadded" somewhere on this page.
> .. productionlist::
> yield_atom: "(" `yield_expression` ")"
> - yield_expression: "yield" [`expression_list`]
> + yield_expression: "yield" [`expression_list` | "from" `expression`]
>
> The :keyword:`yield` expression is only used when defining a generator
> function,
> and can only be used in the body of a function definition. Using a
> @@ -336,7 +336,10 @@
> the generator's methods, the function can proceed exactly as if the
> :keyword:`yield` expression was just another external call. The value of the
> :keyword:`yield` expression after resuming depends on the method which
> resumed
> -the execution.
> +the execution. If :meth:`__next__` is used (typically via either a
> +:keyword:`for` or the :func:`next` builtin) then the result is :const:`None`,
> +otherwise, if :meth:`send` is used, then the result will be the value passed
> +in to that method.
>
> .. index:: single: coroutine
>
> @@ -346,12 +349,29 @@
> where should the execution continue after it yields; the control is always
> transferred to the generator's caller.
>
> -The :keyword:`yield` statement is allowed in the :keyword:`try` clause of a
> +:keyword:`yield` expressions are allowed in the :keyword:`try` clause of a
> :keyword:`try` ... :keyword:`finally` construct. If the generator is not
> resumed before it is finalized (by reaching a zero reference count or by
> being
> garbage collected), the generator-iterator's :meth:`close` method will be
> called, allowing any pending :keyword:`finally` clauses to execute.
>
> +When ``yield from expression`` is used, it treats the supplied expression as
> +a subiterator. All values produced by that subiterator are passed directly
> +to the caller of the current generator's methods. Any values passed in with
> +:meth:`send` and any exceptions passed in with :meth:`throw` are passed to
> +the underlying iterator if it has the appropriate methods. If this is not the
> +case, then :meth:`send` will raise :exc:`AttributeError` or :exc:`TypeError`,
> +while :meth:`throw` will just raise the passed in exception immediately.
> +
> +When the underlying iterator is complete, the :attr:`~StopIteration.value`
> +attribute of the raised :exc:`StopIteration` instance becomes the value of
> +the yield expression. It can be either set explicitly when raising
> +:exc:`StopIteration`, or automatically when the sub-iterator is a generator
> +(by returning a value from the sub-generator).
> +
> +The parentheses can be omitted when the :keyword:`yield` expression is the
> +sole expression on the right hand side of an assignment statement.
> +
> .. index:: object: generator
>
> The following generator's methods can be used to control the execution of a
> @@ -444,6 +464,10 @@
> The proposal to enhance the API and syntax of generators, making them
> usable as simple coroutines.
>
> + :pep:`0380` - Syntax for Delegating to a Subgenerator
> + The proposal to introduce the :token:`yield_from` syntax, making
> delegation
> + to sub-generators easy.
> +
>
> .. _primaries:
>
> PEP 3155: Qualified name for classes and functions
> ==================================================
>
> @@ -208,7 +224,6 @@
> how they might be accessible from the global scope.
>
> Example with (non-bound) methods::
> -
> >>> class C:
> ... def meth(self):
> ... pass
This looks like a spurious (and syntax-breaking) change.
> diff --git a/Grammar/Grammar b/Grammar/Grammar
> --- a/Grammar/Grammar
> +++ b/Grammar/Grammar
> @@ -121,7 +121,7 @@
> |'**' test)
> # The reason that keywords are test nodes instead of NAME is that using NAME
> # results in an ambiguity. ast.c makes sure it's a NAME.
> -argument: test [comp_for] | test '=' test # Really [keyword '='] test
> +argument: (test) [comp_for] | test '=' test # Really [keyword '='] test
This looks like a change without effect?
> diff --git a/Include/genobject.h b/Include/genobject.h
> --- a/Include/genobject.h
> +++ b/Include/genobject.h
> @@ -11,20 +11,20 @@
> struct _frame; /* Avoid including frameobject.h */
>
> typedef struct {
> - PyObject_HEAD
> - /* The gi_ prefix is intended to remind of generator-iterator. */
> + PyObject_HEAD
> + /* The gi_ prefix is intended to remind of generator-iterator. */
>
> - /* Note: gi_frame can be NULL if the generator is "finished" */
> - struct _frame *gi_frame;
> + /* Note: gi_frame can be NULL if the generator is "finished" */
> + struct _frame *gi_frame;
>
> - /* True if generator is being executed. */
> - int gi_running;
> + /* True if generator is being executed. */
> + int gi_running;
>
> - /* The code object backing the generator */
> - PyObject *gi_code;
> + /* The code object backing the generator */
> + PyObject *gi_code;
>
> - /* List of weak reference. */
> - PyObject *gi_weakreflist;
> + /* List of weak reference. */
> + PyObject *gi_weakreflist;
> } PyGenObject;
While these change tabs into spaces, it should be 4 spaces, not 8.
> @@ -34,6 +34,7 @@
>
> PyAPI_FUNC(PyObject *) PyGen_New(struct _frame *);
> PyAPI_FUNC(int) PyGen_NeedsFinalizing(PyGenObject *);
> +PyAPI_FUNC(int) PyGen_FetchStopIterationValue(PyObject **);
>
> #ifdef __cplusplus
> }
Does this API need to be public? If yes, it needs to be documented.
> diff --git a/Include/opcode.h b/Include/opcode.h
> --- a/Include/opcode.h
> +++ b/Include/opcode.h
> @@ -7,116 +7,117 @@
>
> /* Instruction opcodes for compiled code */
>
> -#define POP_TOP 1
> -#define ROT_TWO 2
> -#define ROT_THREE 3
> -#define DUP_TOP 4
> +#define POP_TOP 1
> +#define ROT_TWO 2
> +#define ROT_THREE 3
> +#define DUP_TOP 4
> #define DUP_TOP_TWO 5
> -#define NOP 9
> +#define NOP 9
>
> -#define UNARY_POSITIVE 10
> -#define UNARY_NEGATIVE 11
> -#define UNARY_NOT 12
> +#define UNARY_POSITIVE 10
> +#define UNARY_NEGATIVE 11
> +#define UNARY_NOT 12
>
> -#define UNARY_INVERT 15
> +#define UNARY_INVERT 15
>
> -#define BINARY_POWER 19
> +#define BINARY_POWER 19
>
> -#define BINARY_MULTIPLY 20
> +#define BINARY_MULTIPLY 20
>
> -#define BINARY_MODULO 22
> -#define BINARY_ADD 23
> -#define BINARY_SUBTRACT 24
> -#define BINARY_SUBSCR 25
> +#define BINARY_MODULO 22
> +#define BINARY_ADD 23
> +#define BINARY_SUBTRACT 24
> +#define BINARY_SUBSCR 25
> #define BINARY_FLOOR_DIVIDE 26
> #define BINARY_TRUE_DIVIDE 27
> #define INPLACE_FLOOR_DIVIDE 28
> #define INPLACE_TRUE_DIVIDE 29
>
> -#define STORE_MAP 54
> -#define INPLACE_ADD 55
> -#define INPLACE_SUBTRACT 56
> -#define INPLACE_MULTIPLY 57
> +#define STORE_MAP 54
> +#define INPLACE_ADD 55
> +#define INPLACE_SUBTRACT 56
> +#define INPLACE_MULTIPLY 57
>
> -#define INPLACE_MODULO 59
> -#define STORE_SUBSCR 60
> -#define DELETE_SUBSCR 61
> +#define INPLACE_MODULO 59
> +#define STORE_SUBSCR 60
> +#define DELETE_SUBSCR 61
>
> -#define BINARY_LSHIFT 62
> -#define BINARY_RSHIFT 63
> -#define BINARY_AND 64
> -#define BINARY_XOR 65
> -#define BINARY_OR 66
> -#define INPLACE_POWER 67
> -#define GET_ITER 68
> -#define STORE_LOCALS 69
> -#define PRINT_EXPR 70
> +#define BINARY_LSHIFT 62
> +#define BINARY_RSHIFT 63
> +#define BINARY_AND 64
> +#define BINARY_XOR 65
> +#define BINARY_OR 66
> +#define INPLACE_POWER 67
> +#define GET_ITER 68
> +#define STORE_LOCALS 69
> +#define PRINT_EXPR 70
> #define LOAD_BUILD_CLASS 71
> +#define YIELD_FROM 72
>
> -#define INPLACE_LSHIFT 75
> -#define INPLACE_RSHIFT 76
> -#define INPLACE_AND 77
> -#define INPLACE_XOR 78
> -#define INPLACE_OR 79
> -#define BREAK_LOOP 80
> +#define INPLACE_LSHIFT 75
> +#define INPLACE_RSHIFT 76
> +#define INPLACE_AND 77
> +#define INPLACE_XOR 78
> +#define INPLACE_OR 79
> +#define BREAK_LOOP 80
> #define WITH_CLEANUP 81
>
> -#define RETURN_VALUE 83
> -#define IMPORT_STAR 84
> +#define RETURN_VALUE 83
> +#define IMPORT_STAR 84
>
> -#define YIELD_VALUE 86
> -#define POP_BLOCK 87
> -#define END_FINALLY 88
> -#define POP_EXCEPT 89
> +#define YIELD_VALUE 86
> +#define POP_BLOCK 87
> +#define END_FINALLY 88
> +#define POP_EXCEPT 89
>
> -#define HAVE_ARGUMENT 90 /* Opcodes from here have an argument:
> */
> +#define HAVE_ARGUMENT 90 /* Opcodes from here have an argument: */
>
> -#define STORE_NAME 90 /* Index in name list */
> -#define DELETE_NAME 91 /* "" */
> -#define UNPACK_SEQUENCE 92 /* Number of sequence items */
> -#define FOR_ITER 93
> +#define STORE_NAME 90 /* Index in name list */
> +#define DELETE_NAME 91 /* "" */
> +#define UNPACK_SEQUENCE 92 /* Number of sequence items */
> +#define FOR_ITER 93
> #define UNPACK_EX 94 /* Num items before variable part +
> (Num items after variable part << 8) */
>
> -#define STORE_ATTR 95 /* Index in name list */
> -#define DELETE_ATTR 96 /* "" */
> -#define STORE_GLOBAL 97 /* "" */
> -#define DELETE_GLOBAL 98 /* "" */
> +#define STORE_ATTR 95 /* Index in name list */
> +#define DELETE_ATTR 96 /* "" */
> +#define STORE_GLOBAL 97 /* "" */
> +#define DELETE_GLOBAL 98 /* "" */
>
> -#define LOAD_CONST 100 /* Index in const list */
> -#define LOAD_NAME 101 /* Index in name list */
> -#define BUILD_TUPLE 102 /* Number of tuple items */
> -#define BUILD_LIST 103 /* Number of list items */
> -#define BUILD_SET 104 /* Number of set items */
> -#define BUILD_MAP 105 /* Always zero for now */
> -#define LOAD_ATTR 106 /* Index in name list */
> -#define COMPARE_OP 107 /* Comparison operator */
> -#define IMPORT_NAME 108 /* Index in name list */
> -#define IMPORT_FROM 109 /* Index in name list */
> +#define LOAD_CONST 100 /* Index in const list */
> +#define LOAD_NAME 101 /* Index in name list */
> +#define BUILD_TUPLE 102 /* Number of tuple items */
> +#define BUILD_LIST 103 /* Number of list items */
> +#define BUILD_SET 104 /* Number of set items */
> +#define BUILD_MAP 105 /* Always zero for now */
> +#define LOAD_ATTR 106 /* Index in name list */
> +#define COMPARE_OP 107 /* Comparison operator */
> +#define IMPORT_NAME 108 /* Index in name list */
> +#define IMPORT_FROM 109 /* Index in name list */
>
> -#define JUMP_FORWARD 110 /* Number of bytes to skip */
> -#define JUMP_IF_FALSE_OR_POP 111 /* Target byte offset from beginning of
> code */
> -#define JUMP_IF_TRUE_OR_POP 112 /* "" */
> -#define JUMP_ABSOLUTE 113 /* "" */
> -#define POP_JUMP_IF_FALSE 114 /* "" */
> -#define POP_JUMP_IF_TRUE 115 /* "" */
> +#define JUMP_FORWARD 110 /* Number of bytes to skip */
> +#define JUMP_IF_FALSE_OR_POP 111 /* Target byte offset from beginning
> of code */
> +#define JUMP_IF_TRUE_OR_POP 112 /* "" */
> +#define JUMP_ABSOLUTE 113 /* "" */
> +#define POP_JUMP_IF_FALSE 114 /* "" */
> +#define POP_JUMP_IF_TRUE 115 /* "" */
>
> -#define LOAD_GLOBAL 116 /* Index in name list */
> +#define LOAD_GLOBAL 116 /* Index in name list */
>
> -#define CONTINUE_LOOP 119 /* Start of loop (absolute) */
> -#define SETUP_LOOP 120 /* Target address (relative) */
> -#define SETUP_EXCEPT 121 /* "" */
> -#define SETUP_FINALLY 122 /* "" */
> +#define CONTINUE_LOOP 119 /* Start of loop (absolute) */
> +#define SETUP_LOOP 120 /* Target address (relative) */
> +#define SETUP_EXCEPT 121 /* "" */
> +#define SETUP_FINALLY 122 /* "" */
>
> -#define LOAD_FAST 124 /* Local variable number */
> -#define STORE_FAST 125 /* Local variable number */
> -#define DELETE_FAST 126 /* Local variable number */
> +#define LOAD_FAST 124 /* Local variable number */
> +#define STORE_FAST 125 /* Local variable number */
> +#define DELETE_FAST 126 /* Local variable number */
>
> -#define RAISE_VARARGS 130 /* Number of raise arguments (1, 2 or
> 3) */
> +#define RAISE_VARARGS 130 /* Number of raise arguments (1, 2 or 3) */
> /* CALL_FUNCTION_XXX opcodes defined below depend on this definition */
> -#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 */
>
> #define MAKE_CLOSURE 134 /* same as MAKE_FUNCTION */
> #define LOAD_CLOSURE 135 /* Load free variable from closure */
Not sure putting these and all the other cosmetic changes into an already
big patch is such a good idea...
> diff --git a/Include/pyerrors.h b/Include/pyerrors.h
> --- a/Include/pyerrors.h
> +++ b/Include/pyerrors.h
> @@ -51,6 +51,11 @@
> Py_ssize_t written; /* only for BlockingIOError, -1 otherwise */
> } PyOSErrorObject;
>
> +typedef struct {
> + PyException_HEAD
> + PyObject *value;
> +} PyStopIterationObject;
> +
> /* Compatibility typedefs */
> typedef PyOSErrorObject PyEnvironmentErrorObject;
> #ifdef MS_WINDOWS
> @@ -380,6 +385,8 @@
> const char *reason /* UTF-8 encoded string */
> );
>
> +/* create a StopIteration exception with the given value */
> +PyAPI_FUNC(PyObject *) PyStopIteration_Create(PyObject *);
About this API see below.
> 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?
> +PyObject *
> +PyStopIteration_Create(PyObject *value)
> +{
> + return PyObject_CallFunctionObjArgs(PyExc_StopIteration, value, NULL);
> +}
I think this function is rather questionable. It is only used once at all.
If kept, it should rather be named _PyE{rr,xc}_CreateStopIteration. But since
it's so trivial, it should be removed altogether.
> diff --git a/Objects/genobject.c b/Objects/genobject.c
> --- a/Objects/genobject.c
> +++ b/Objects/genobject.c
> @@ -5,6 +5,9 @@
> #include "structmember.h"
> #include "opcode.h"
>
> +static PyObject *gen_close(PyGenObject *gen, PyObject *args);
> +static void gen_undelegate(PyGenObject *gen);
> +
> static int
> gen_traverse(PyGenObject *gen, visitproc visit, void *arg)
> {
> @@ -90,12 +93,18 @@
>
> /* If the generator just returned (as opposed to yielding), signal
> * that the generator is exhausted. */
> - if (result == Py_None && f->f_stacktop == NULL) {
> - Py_DECREF(result);
> - result = NULL;
> - /* Set exception if not called by gen_iternext() */
> - if (arg)
> + if (result && f->f_stacktop == NULL) {
> + if (result == Py_None) {
> + /* Delay exception instantiation if we can */
> PyErr_SetNone(PyExc_StopIteration);
> + } else {
> + PyObject *e = PyStopIteration_Create(result);
> + if (e != NULL) {
> + PyErr_SetObject(PyExc_StopIteration, e);
> + Py_DECREF(e);
> + }
Wouldn't PyErr_SetObject(PyExc_StopIteration, value) suffice here
anyway?
> +/*
> + * If StopIteration exception is set, fetches its 'value'
> + * attribute if any, otherwise sets pvalue to None.
> + *
> + * Returns 0 if no exception or StopIteration is set.
> + * If any other exception is set, returns -1 and leaves
> + * pvalue unchanged.
> + */
> +
> +int
> +PyGen_FetchStopIterationValue(PyObject **pvalue) {
> + PyObject *et, *ev, *tb;
> + PyObject *value = NULL;
> +
> + if (PyErr_ExceptionMatches(PyExc_StopIteration)) {
> + PyErr_Fetch(&et, &ev, &tb);
> + Py_XDECREF(et);
> + Py_XDECREF(tb);
> + if (ev) {
> + value = ((PyStopIterationObject *)ev)->value;
> + Py_DECREF(ev);
> + }
PyErr_Fetch without PyErr_Restore clears the exception, that should be
mentioned in the docstring.
Georg
_______________________________________________
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
