GH-137630: Convert _interpchannels to use Argument Clinic
#139141
Conversation
| /*[python end generated code: output=da39a3ee5e6b4b0d input=78e17eef989afa6b]*/ | ||
|
|
||
| static int | ||
| channel_id_arg_converter(PyObject *module, PyObject *arg, void *ptr) |
There was a problem hiding this comment.
You don't need to use a void pointer here. (Implying you can also get rid of the casting and temporary cid_ptr stuff below.
| channel_id_arg_converter(PyObject *module, PyObject *arg, void *ptr) | |
| channel_id_arg_converter(PyObject *module, PyObject *arg, int64_t *cid) |
| broken_limited_capi = True | ||
|
|
||
| def parse_arg(self, argname, displayname, *, limited_capi): | ||
| assert not limited_capi | ||
| return self.format_code(""" | ||
| if (!{converter}(module, {argname}, &{paramname})) {{{{ | ||
| goto exit; | ||
| }}}} | ||
| """, | ||
| argname=argname, | ||
| converter=self.converter) |
There was a problem hiding this comment.
Why is all this needed?
There was a problem hiding this comment.
Is it because of the "module" argument?
| unboundop as unboundarg: int = -1 | ||
| fallback as fallbackarg: int = -1 | ||
| * | ||
| blocking: bool = True |
There was a problem hiding this comment.
The default for blocking depends on timeout, rather than just being True.
| unboundop as unboundarg: int = -1 | ||
| fallback as fallbackarg: int = -1 | ||
| * | ||
| blocking: bool = True |
There was a problem hiding this comment.
Likewise, the default for blocking depends on timeout.
| * recv is None and send is None: | ||
| - raise ChannelNotEmptyError |
There was a problem hiding this comment.
Hmm, None isn't actually allowed is it? That really isn't something you've changed and I'm not sure I'd worry about it.
There was a problem hiding this comment.
is None converted to False for p? Does that continue to work under argument clinic?
| send: bool = False | ||
| recv: bool = False |
There was a problem hiding this comment.
FWIW, it isn't exactly just True or False. The default is "close both" (the same as when both are True), which is why I described it as None in the original docstring. Saying the default is both False could be confusing. Then again, this is a private, low-level module so I suppose it doesn't matter much.
| send: bool = False | ||
| recv: bool = False |
There was a problem hiding this comment.
Likewise about True/False.
| send: object(subclass_of='&PyType_Type', type='PyTypeObject *') | ||
| recv: object(subclass_of='&PyType_Type', type='PyTypeObject *') |
cc @ericsnowcurrently.
The implementation of
channel_id_arg_converteris a bit hacky, but I took advice from @erlend-aasland here, and it does work. Perhaps we could use a different mechanism to obtain module state inchannel_id_converter()rather than stuffing the module object intochannel_id_converter_data, but that is existing code & may have other design considerations, so I haven't proposed anything in this direction.A
_interpretersto Argument Clinic #137630