Repository navigation
gh-127124: Pass optional state to context watcher callback - #127140
rhansen wants to merge 3 commits into
Conversation
Sorry, something went wrong.
ZeroIntensity
left a comment
There was a problem hiding this comment.
Needs a NEWS entry (even though it's a change to a new feature, we want to show that we changed it in the a3 changelog)
Sorry, something went wrong.
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.
Sorry, something went wrong.
| 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?
Sorry, something went wrong.
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)
{
...
}
|
FWIW, no need to force push changes--it makes reviewing harder. It's fine to just commit normally, and then everything gets squashed. |
Sorry, something went wrong.
|
Force pushing is the only way to update the commit message. |
Sorry, something went wrong.
|
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.) |
Sorry, something went wrong.
|
This PR is stale because it has been open for 30 days with no activity. |
Sorry, something went wrong.
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:
This will conflict with #124741; if one is merged I'll rebase the other.
📚 Documentation preview 📚: https://cpython-previews--127140.org.readthedocs.build/