Add CythonLibcoroTaskWrapperCallback type alias for the asyncio Future-resolving callback - #1040
Conversation
Matt711
added
non-breaking
| */ | ||
| coro::task<void> cython_libcoro_task_wrapper( | ||
| void (*cpp_set_py_future)(void*, const char *), | ||
| PyFutureCallback cpp_set_py_future, |
There was a problem hiding this comment.
Or maybe AsyncIOFutureCallback?
There was a problem hiding this comment.
What about cython_libcoro_task_wrapper_callback to avoid the name clash? Since this is in a .pxd, it will effectively be copy-pasted into all Cython files importing it.
Alternatively, we could wrap the whole thing in a namespace.
There was a problem hiding this comment.
What about cython_libcoro_task_wrapper_callback to avoid the name clash?
Is there a current name clash or are you thinking of protecting against another .pxd using the same name with a different type?
There was a problem hiding this comment.
Let's put the whole thing in an anonymous namespace I think.
Although then I would worry about ODR-use violations...
There was a problem hiding this comment.
Is there a current name clash or are you thinking of protecting against another
.pxdusing the same name with a different type?
There is no current name clash. This is mostly about avoiding a future one.
Since this lives under _detail, we can always rename it later without a
deprecation period. Still, I think it is good style to use a unique name because this is
declared in a .pxd and may be pulled into different generated C++ files.
I do not think an anonymous namespace helps here. A cdef extern from * block
is effectively copied into the generated C++ source of each module that imports
it. So if a .pyx file also defines or imports a PyFutureCallback with a
different type, the names can still collide within that generated translation
unit.
That is why I prefer a more specific name like cython_libcoro_task_wrapper_callback.
There was a problem hiding this comment.
CythonLibcoroTaskWrapperCallback or no 😄 ?
There was a problem hiding this comment.
Heh, yeah I guess it should be CythonLibcoroTaskWrapperCallback :)
Since this is only used in one place, a large name is fine.
0acbfeb to
2edc16e
Compare
Matt711
changed the title
CythonLibcoroTaskWrapperCallback type alias for the asyncio Future-resolving callback
dfa86d7 to
b34f2d3
Compare
8de8c0d to
357c842
Compare
|
/merge |
3 participants
Because function pointers are confusing to read. And this helps some.