From b00fa1986e9056ba2275ec13bffcba457d3a93d5 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 26 Mar 2022 14:43:58 +0300 Subject: [PATCH 1/2] Map the Argument Clinic's NoneType return to `void` --- Doc/howto/clinic.rst | 1 + .../2022-03-26-15-29-25.bpo-47128.-48DIT.rst | 5 ++++ Modules/_io/clinic/iobase.c.h | 21 ++++++++++++- Modules/_io/iobase.c | 17 +++++------ Tools/clinic/clinic.py | 30 ++++++++++--------- 5 files changed, 50 insertions(+), 24 deletions(-) create mode 100644 Misc/NEWS.d/next/Tools-Demos/2022-03-26-15-29-25.bpo-47128.-48DIT.rst diff --git a/Doc/howto/clinic.rst b/Doc/howto/clinic.rst index 04b1a2cac0b042..09fbc6e954b7ab 100644 --- a/Doc/howto/clinic.rst +++ b/Doc/howto/clinic.rst @@ -1055,6 +1055,7 @@ Currently Argument Clinic supports only a few return converters: .. code-block:: none + NoneType bool int unsigned int diff --git a/Misc/NEWS.d/next/Tools-Demos/2022-03-26-15-29-25.bpo-47128.-48DIT.rst b/Misc/NEWS.d/next/Tools-Demos/2022-03-26-15-29-25.bpo-47128.-48DIT.rst new file mode 100644 index 00000000000000..cc45ea6b1b3669 --- /dev/null +++ b/Misc/NEWS.d/next/Tools-Demos/2022-03-26-15-29-25.bpo-47128.-48DIT.rst @@ -0,0 +1,5 @@ +Argument Clinic got enhanced :data:`NoneType` return converter. With it, a +generated impl header gets :c:type:`void` return type and the corresponding +generated wrapper returns :const:`NULL` on :c:func:`PyErr_Occurred` or +:c:macro:`Py_None` otherwise. Previously, the return type was +:c:type:`PyObject *`. Patch by Oleg Iarygin. diff --git a/Modules/_io/clinic/iobase.c.h b/Modules/_io/clinic/iobase.c.h index 4fd6e18c4efd25..8d2dee4150c17f 100644 --- a/Modules/_io/clinic/iobase.c.h +++ b/Modules/_io/clinic/iobase.c.h @@ -251,6 +251,25 @@ PyDoc_STRVAR(_io__IOBase_writelines__doc__, #define _IO__IOBASE_WRITELINES_METHODDEF \ {"writelines", (PyCFunction)_io__IOBase_writelines, METH_O, _io__IOBase_writelines__doc__}, +static void +_io__IOBase_writelines_impl(PyObject *self, PyObject *lines); + +static PyObject * +_io__IOBase_writelines(PyObject *self, PyObject *lines) +{ + PyObject *return_value = NULL; + + _io__IOBase_writelines_impl(self, lines); + if (PyErr_Occurred()) { + goto exit; + } + return_value = Py_None; + Py_INCREF(Py_None); + +exit: + return return_value; +} + PyDoc_STRVAR(_io__RawIOBase_read__doc__, "read($self, size=-1, /)\n" "--\n" @@ -310,4 +329,4 @@ _io__RawIOBase_readall(PyObject *self, PyObject *Py_UNUSED(ignored)) { return _io__RawIOBase_readall_impl(self); } -/*[clinic end generated code: output=83c1361a7a51ca84 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=93ac28e60a088847 input=a9049054013a1b77]*/ diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index 6ae43a8b3bd626..4b8fdcd3778d04 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -739,7 +739,7 @@ _io__IOBase_readlines_impl(PyObject *self, Py_ssize_t hint) } /*[clinic input] -_io._IOBase.writelines +_io._IOBase.writelines -> NoneType lines: object / @@ -749,25 +749,25 @@ Line separators are not added, so it is usual for each of the lines provided to have a line separator at the end. [clinic start generated code]*/ -static PyObject * -_io__IOBase_writelines(PyObject *self, PyObject *lines) -/*[clinic end generated code: output=976eb0a9b60a6628 input=cac3fc8864183359]*/ +static void +_io__IOBase_writelines_impl(PyObject *self, PyObject *lines) +/*[clinic end generated code: output=f3feca36db72dbd1 input=286ba711cb7291ad]*/ { PyObject *iter, *res; if (iobase_check_closed(self)) - return NULL; + return; iter = PyObject_GetIter(lines); if (iter == NULL) - return NULL; + return; while (1) { PyObject *line = PyIter_Next(iter); if (line == NULL) { if (PyErr_Occurred()) { Py_DECREF(iter); - return NULL; + return; } else break; /* Stop Iteration */ @@ -780,12 +780,11 @@ _io__IOBase_writelines(PyObject *self, PyObject *lines) Py_DECREF(line); if (res == NULL) { Py_DECREF(iter); - return NULL; + return; } Py_DECREF(res); } Py_DECREF(iter); - Py_RETURN_NONE; } #include "clinic/iobase.c.h" diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 14252b2514ea87..c23d9d20b324fe 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -759,7 +759,7 @@ def parser_body(prototype, *fields, declarations=''): # just imagine--your code is here in the middle fields.append(normalize_snippet(""" {modifications} - {return_value} = {c_basename}_impl({impl_arguments}); + {return_assignment}{c_basename}_impl({impl_arguments}); {return_conversion} {exit_label} @@ -1384,7 +1384,10 @@ def render_function(self, clinic, f): template_dict['impl_arguments'] = ", ".join(data.impl_arguments) template_dict['return_conversion'] = format_escape("".join(data.return_conversion).rstrip()) template_dict['cleanup'] = format_escape("".join(data.cleanup)) - template_dict['return_value'] = data.return_value + if data.return_value: + template_dict['return_assignment'] = f'{data.return_value} = ' + else: + template_dict['return_assignment'] = '' # used by unpack tuple code generator ignore_self = -1 if isinstance(converters[0], self_converter) else 0 @@ -3785,17 +3788,16 @@ def return_converter_init(self): pass def declare(self, data, name="_return_value"): - line = [] - add = line.append - add(self.type) - if not self.type.endswith('*'): - add(' ') - add(name + ';') - data.declarations.append(''.join(line)) - data.return_value = name + if self.type != 'void': + line = f"{self.type}{'' if self.type[-1] == '*' else ' '}{name};" + data.declarations.append(line) + data.return_value = name + else: + data.return_value = None def err_occurred_if(self, expr, data): - data.return_conversion.append('if (({}) && PyErr_Occurred()) {{\n goto exit;\n}}\n'.format(expr)) + dcond = f'({expr}) && ' if expr else '' + data.return_conversion.append(f'if ({dcond}PyErr_Occurred()) {{\n goto exit;\n}}\n') def err_occurred_if_null_pointer(self, variable, data): data.return_conversion.append('if ({} == NULL) {{\n goto exit;\n}}\n'.format(variable)) @@ -3810,12 +3812,12 @@ def render(self, function, data): add_c_return_converter(CReturnConverter, 'object') class NoneType_return_converter(CReturnConverter): + type = 'void' + def render(self, function, data): self.declare(data) + self.err_occurred_if(None, data) data.return_conversion.append(''' -if (_return_value != Py_None) { - goto exit; -} return_value = Py_None; Py_INCREF(Py_None); '''.strip()) From 61d8dfecee9ce4631f7ba16c71e2b91278c5226d Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 26 Mar 2022 18:04:09 +0300 Subject: [PATCH 2/2] Add a note on error indication --- Doc/howto/clinic.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Doc/howto/clinic.rst b/Doc/howto/clinic.rst index 09fbc6e954b7ab..5a1b701b170ec9 100644 --- a/Doc/howto/clinic.rst +++ b/Doc/howto/clinic.rst @@ -1046,10 +1046,12 @@ There's one additional complication when using return converters: how do you indicate an error has occurred? Normally, a function returns a valid (non-``NULL``) pointer for success, and ``NULL`` for failure. But if you use an integer return converter, all integers are valid. How can Argument Clinic detect an error? Its solution: each return -converter implicitly looks for a special value that indicates an error. If you return +converter except ``NoneType`` implicitly looks for a special value that indicates an error. If you return that value, and an error has been set (``PyErr_Occurred()`` returns a true value), then the generated code will propagate the error. Otherwise it will -encode the value you return like normal. +encode the value you return like normal. For ``NoneType``, +``PyErr_Occurred()`` only is checked because the ``void`` type cannot have a +special value. Currently Argument Clinic supports only a few return converters: