-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
bpo-42085: Introduce dedicated entry in PyAsyncMethods for sending values #22780
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
cdd612e
7fb80fd
d11ba24
4635bf1
6559a1e
0269d6c
5006b27
016f3e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Add dedicated entry to PyAsyncMethods for sending values |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1484,7 +1484,8 @@ future_cls_getitem(PyObject *cls, PyObject *type) | |
static PyAsyncMethods FutureType_as_async = { | ||
(unaryfunc)future_new_iter, /* am_await */ | ||
0, /* am_aiter */ | ||
0 /* am_anext */ | ||
0, /* am_anext */ | ||
0, /* am_send */ | ||
}; | ||
|
||
static PyMethodDef FutureType_methods[] = { | ||
|
@@ -1602,37 +1603,58 @@ FutureIter_dealloc(futureiterobject *it) | |
} | ||
} | ||
|
||
static PyObject * | ||
FutureIter_iternext(futureiterobject *it) | ||
static PySendResult | ||
FutureIter_am_send(futureiterobject *it, PyObject *Py_UNUSED(arg), PyObject **result) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to keep lines under 79 characters |
||
{ | ||
/* arg is unused, see the comment on FutureIter_send for clarification */ | ||
|
||
PyObject *res; | ||
FutureObj *fut = it->future; | ||
|
||
*result = NULL; | ||
if (fut == NULL) { | ||
return NULL; | ||
return PYGEN_ERROR; | ||
serhiy-storchaka marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
if (fut->fut_state == STATE_PENDING) { | ||
if (!fut->fut_blocking) { | ||
fut->fut_blocking = 1; | ||
Py_INCREF(fut); | ||
return (PyObject *)fut; | ||
*result = (PyObject *)fut; | ||
return PYGEN_NEXT; | ||
} | ||
PyErr_SetString(PyExc_RuntimeError, | ||
"await wasn't used with future"); | ||
return NULL; | ||
return PYGEN_ERROR; | ||
serhiy-storchaka marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
it->future = NULL; | ||
res = _asyncio_Future_result_impl(fut); | ||
if (res != NULL) { | ||
/* The result of the Future is not an exception. */ | ||
(void)_PyGen_SetStopIterationValue(res); | ||
Py_DECREF(res); | ||
*result = res; | ||
return PYGEN_RETURN; | ||
} | ||
|
||
Py_DECREF(fut); | ||
return NULL; | ||
return PYGEN_ERROR; | ||
} | ||
|
||
static PyObject * | ||
FutureIter_iternext(futureiterobject *it) | ||
{ | ||
PyObject *result; | ||
switch (FutureIter_am_send(it, Py_None, &result)) { | ||
case PYGEN_RETURN: | ||
(void)_PyGen_SetStopIterationValue(result); | ||
Py_DECREF(result); | ||
return NULL; | ||
case PYGEN_NEXT: | ||
return result; | ||
case PYGEN_ERROR: | ||
return NULL; | ||
default: | ||
Py_UNREACHABLE(); | ||
} | ||
} | ||
|
||
static PyObject * | ||
|
@@ -1721,14 +1743,23 @@ static PyMethodDef FutureIter_methods[] = { | |
{NULL, NULL} /* Sentinel */ | ||
}; | ||
|
||
static PyAsyncMethods FutureIterType_as_async = { | ||
0, /* am_await */ | ||
0, /* am_aiter */ | ||
0, /* am_anext */ | ||
(sendfunc)FutureIter_am_send, /* am_send */ | ||
}; | ||
|
||
|
||
static PyTypeObject FutureIterType = { | ||
PyVarObject_HEAD_INIT(NULL, 0) | ||
"_asyncio.FutureIter", | ||
.tp_basicsize = sizeof(futureiterobject), | ||
.tp_itemsize = 0, | ||
.tp_dealloc = (destructor)FutureIter_dealloc, | ||
.tp_as_async = &FutureIterType_as_async, | ||
.tp_getattro = PyObject_GenericGetAttr, | ||
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, | ||
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_HAVE_SEND, | ||
.tp_traverse = (traverseproc)FutureIter_traverse, | ||
.tp_iter = PyObject_SelfIter, | ||
.tp_iternext = (iternextfunc)FutureIter_iternext, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -268,30 +268,10 @@ gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult, | |
return result ? PYGEN_RETURN : PYGEN_ERROR; | ||
} | ||
|
||
PySendResult | ||
PyIter_Send(PyObject *iter, PyObject *arg, PyObject **result) | ||
static PySendResult | ||
PyGen_am_send(PyGenObject *gen, PyObject *arg, PyObject **result) | ||
{ | ||
_Py_IDENTIFIER(send); | ||
assert(arg != NULL); | ||
assert(result != NULL); | ||
|
||
if (PyGen_CheckExact(iter) || PyCoro_CheckExact(iter)) { | ||
return gen_send_ex2((PyGenObject *)iter, arg, result, 0, 0); | ||
} | ||
|
||
if (arg == Py_None && PyIter_Check(iter)) { | ||
*result = Py_TYPE(iter)->tp_iternext(iter); | ||
} | ||
else { | ||
*result = _PyObject_CallMethodIdOneArg(iter, &PyId_send, arg); | ||
} | ||
if (*result != NULL) { | ||
return PYGEN_NEXT; | ||
} | ||
if (_PyGen_FetchStopIterationValue(result) == 0) { | ||
return PYGEN_RETURN; | ||
} | ||
return PYGEN_ERROR; | ||
return gen_send_ex2(gen, arg, result, 0, 0); | ||
} | ||
|
||
static PyObject * | ||
|
@@ -788,6 +768,14 @@ static PyMethodDef gen_methods[] = { | |
{NULL, NULL} /* Sentinel */ | ||
}; | ||
|
||
static PyAsyncMethods gen_as_async = { | ||
0, /* am_await */ | ||
0, /* am_aiter */ | ||
0, /* am_anext */ | ||
(sendfunc)PyGen_am_send, /* am_send */ | ||
}; | ||
|
||
|
||
PyTypeObject PyGen_Type = { | ||
PyVarObject_HEAD_INIT(&PyType_Type, 0) | ||
"generator", /* tp_name */ | ||
|
@@ -798,7 +786,7 @@ PyTypeObject PyGen_Type = { | |
0, /* tp_vectorcall_offset */ | ||
0, /* tp_getattr */ | ||
0, /* tp_setattr */ | ||
0, /* tp_as_async */ | ||
&gen_as_async, /* tp_as_async */ | ||
(reprfunc)gen_repr, /* tp_repr */ | ||
0, /* tp_as_number */ | ||
0, /* tp_as_sequence */ | ||
|
@@ -809,7 +797,8 @@ PyTypeObject PyGen_Type = { | |
PyObject_GenericGetAttr, /* tp_getattro */ | ||
0, /* tp_setattro */ | ||
0, /* tp_as_buffer */ | ||
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags */ | ||
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | | ||
Py_TPFLAGS_HAVE_SEND, /* tp_flags */ | ||
0, /* tp_doc */ | ||
(traverseproc)gen_traverse, /* tp_traverse */ | ||
0, /* tp_clear */ | ||
|
@@ -1031,7 +1020,8 @@ static PyMethodDef coro_methods[] = { | |
static PyAsyncMethods coro_as_async = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait, with this PR regular generators no longer benefit from the faster |
||
(unaryfunc)coro_await, /* am_await */ | ||
0, /* am_aiter */ | ||
0 /* am_anext */ | ||
0, /* am_anext */ | ||
(sendfunc)PyGen_am_send, /* am_send */ | ||
}; | ||
|
||
PyTypeObject PyCoro_Type = { | ||
|
@@ -1055,7 +1045,8 @@ PyTypeObject PyCoro_Type = { | |
PyObject_GenericGetAttr, /* tp_getattro */ | ||
0, /* tp_setattro */ | ||
0, /* tp_as_buffer */ | ||
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags */ | ||
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | | ||
Py_TPFLAGS_HAVE_SEND, /* tp_flags */ | ||
0, /* tp_doc */ | ||
(traverseproc)gen_traverse, /* tp_traverse */ | ||
0, /* tp_clear */ | ||
|
@@ -1413,7 +1404,8 @@ static PyMethodDef async_gen_methods[] = { | |
static PyAsyncMethods async_gen_as_async = { | ||
0, /* am_await */ | ||
PyObject_SelfIter, /* am_aiter */ | ||
(unaryfunc)async_gen_anext /* am_anext */ | ||
(unaryfunc)async_gen_anext, /* am_anext */ | ||
(sendfunc)PyGen_am_send, /* am_send */ | ||
}; | ||
|
||
|
||
|
@@ -1438,7 +1430,8 @@ PyTypeObject PyAsyncGen_Type = { | |
PyObject_GenericGetAttr, /* tp_getattro */ | ||
0, /* tp_setattro */ | ||
0, /* tp_as_buffer */ | ||
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags */ | ||
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | | ||
Py_TPFLAGS_HAVE_SEND, /* tp_flags */ | ||
0, /* tp_doc */ | ||
(traverseproc)async_gen_traverse, /* tp_traverse */ | ||
0, /* tp_clear */ | ||
|
@@ -1676,7 +1669,8 @@ static PyMethodDef async_gen_asend_methods[] = { | |
static PyAsyncMethods async_gen_asend_as_async = { | ||
PyObject_SelfIter, /* am_await */ | ||
0, /* am_aiter */ | ||
0 /* am_anext */ | ||
0, /* am_anext */ | ||
0, /* am_send */ | ||
}; | ||
|
||
|
||
|
@@ -2084,7 +2078,8 @@ static PyMethodDef async_gen_athrow_methods[] = { | |
static PyAsyncMethods async_gen_athrow_as_async = { | ||
PyObject_SelfIter, /* am_await */ | ||
0, /* am_aiter */ | ||
0 /* am_anext */ | ||
0, /* am_anext */ | ||
0, /* am_send */ | ||
}; | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -5409,6 +5409,13 @@ PyType_Ready(PyTypeObject *type) | |||||
_PyObject_ASSERT((PyObject *)type, type->tp_vectorcall_offset > 0); | ||||||
_PyObject_ASSERT((PyObject *)type, type->tp_call != NULL); | ||||||
} | ||||||
/* Consistency check for Py_TPFLAGS_HAVE_SEND - flag requires | ||||||
* type->tp_as_async->am_send to be present. | ||||||
*/ | ||||||
if (type->tp_flags & Py_TPFLAGS_HAVE_SEND) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd be more specific with the flag name.
Suggested change
|
||||||
_PyObject_ASSERT((PyObject *)type, type->tp_as_async != NULL); | ||||||
_PyObject_ASSERT((PyObject *)type, type->tp_as_async->am_send != NULL); | ||||||
} | ||||||
|
||||||
type->tp_flags |= Py_TPFLAGS_READYING; | ||||||
|
||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not we add corresponding
Py_TPFLAGS_HAVE_
flag for binary compatibility?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typeslots.h
should be updated.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we support extensions compiled for higher versions of Python in lower versions of Python? If I compile something for 3.10 I wouldn't expect it to work in 3.9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we don't need a
Py_TPFLAGS_HAVE_
flag and I don't remember adding one for theas_async
slot when we implemented PEP 492.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if you compile something for 3.9 it is expected to work in 3.10 and do not read uninitialized slot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps there is a bug in the PEP 492 implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, I see now, thanks for explaining! Yeah, in this case we do need a flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I don't think we need to do anything now that
as_async
has been there since 3.5. But looking back, maybe we should have added a flag.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We used an existing slot for PEP-492, so the ABI did not change.
I don't think we necessarily need a new flag. See https://bugs.python.org/issue32388