gh-127124: Pass optional state to context watcher callback #127140
Conversation
rhansen
force-pushed
the
gh-127124-context-watcher-opaque-state
branch
from
f6ffc28
to
3130a2f
Compare
rhansen
changed the title
There was a problem hiding this comment.
I'll open a separate PR for the callable idea I mentioned in #127124 (comment) so that we can compare the two and decide which we like better.
Edit: Done; see #127247.
| PyContext_WatchCallback context_watchers[CONTEXT_MAX_WATCHERS]; | ||
| struct { | ||
| PyContext_WatchCallback *callback; | ||
| PyObject *arg; |
There was a problem hiding this comment.
Does this need a Py_VISIT somewhere? I didn't see any for the other fields in this struct, but maybe the GC does something special with this struct?
This enables users to associate state with the callback without
relying on globals.
Also:
* Refactor the tests for improved readability and extensibility, and
to cover the new state object.
* Drop the pointer from the `PyContext_WatchCallback` typedef. This
de-obfuscates the fact that pointers are involved, and makes it
possible to forward-declare functions to improve readability:
static PyContext_WatchCallback my_callback;
int
my_callback(PyObject *cbarg, PyContextEvent event, PyObject *obj)
{
...
}
rhansen
force-pushed
the
gh-127124-context-watcher-opaque-state
branch
from
3130a2f
to
c9e95d4
Compare
|
FWIW, no need to force push changes--it makes reviewing harder. It's fine to just commit normally, and then everything gets squashed. |
|
Force pushing is the only way to update the commit message. |
|
Yes, but it makes reviewing more difficult. FWIW, nobody really cares about the commit messages in the PR itself, the end message that gets squashed is really what matters. (There's a section in the devguide about this, IIRC.) |
This enables users to associate state with the callback without relying on globals.
Also:
Refactor the tests for improved readability and extensibility, and to cover the new state object.
Drop the pointer from the
PyContext_WatchCallbacktypedef. This de-obfuscates the fact that pointers are involved, and makes it possible to forward-declare functions to improve readability:This will conflict with #124741; if one is merged I'll rebase the other.
📚 Documentation preview 📚: https://cpython-previews--127140.org.readthedocs.build/