gh-120834: Use PyGenObject for generators, coroutines and async generators#120976
gh-120834: Use PyGenObject for generators, coroutines and async generators#120976iritkatriel wants to merge 1 commit intopython:mainfrom
Conversation
| /* --- Asynchronous Generators -------------------------------------------- */ | ||
|
|
||
| typedef struct _PyAsyncGenObject PyAsyncGenObject; | ||
| typedef PyGenObject PyAsyncGenObject; |
There was a problem hiding this comment.
PyCoroObject and PyAsyncGenObject are mentioned in the docs, so I think we need to leave this even though we're not using these names.
markshannon
left a comment
markshannon
left a comment
There was a problem hiding this comment.
A few pedantic type suggestions, otherwise LGTM.
| struct _PyCoroObject { | ||
| _PyGenObject_HEAD(cr) | ||
| /* The gi_ prefix is for generator-iterator. */ | ||
| PyObject_HEAD \ |
There was a problem hiding this comment.
We don't need the line extension \s any more.
|
|
||
| void | ||
| _PyErr_WarnUnawaitedAgenMethod(PyAsyncGenObject *agen, PyObject *method) | ||
| _PyErr_WarnUnawaitedAgenMethod(PyGenObject *agen, PyObject *method) |
There was a problem hiding this comment.
This seems a less correct type. agen should be an async generator, so PyAsyncGenObject * seems the correct type.
There was a problem hiding this comment.
Are we going to keep PyAsyncGenObject and PyCoroObject, now that they are just aliases?
It's not like using them in a signature will give compile-time type checks.
I was assuming we will want to stop using them altogether and deprecate them from the API.
|
|
||
| void | ||
| _PyErr_WarnUnawaitedCoroutine(PyObject *coro) | ||
| _PyErr_WarnUnawaitedCoroutine(PyGenObject *coro) |
There was a problem hiding this comment.
This should only be called with a coroutine, so I think PyCoroObject * is correct type.
Maybe add an assert that Py_TYPE(coro) == &PyCoro_Type?
| extern struct _PyInterpreterFrame* _PyEval_GetFrame(void); | ||
|
|
||
| PyAPI_FUNC(PyObject *)_Py_MakeCoro(PyFunctionObject *func); | ||
| PyAPI_FUNC(PyGenObject *)_Py_MakeCoro(PyFunctionObject *func); |
There was a problem hiding this comment.
We generally have a convention that there is a correspondence between the C type and the Python class.
e.g. if PyXXX_Check[Exact](obj) is true, then it is safe to cast (PyXXXObject *)obj and vice-versa.
This sort of breaks that for coroutines and async generators.
So, I think it best to leave this as PyObject *. Does that make sense to you?
|
When you're done making the requested changes, leave the comment: |
|
We decided not to do this after all. |
2 participants
Uh oh!
There was an error while loading. Please reload this page.