Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-47128: Enhance Argument Clinic's NoneType return converter to give void #32126

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions Doc/howto/clinic.rst
Expand Up @@ -1046,15 +1046,18 @@ 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:

.. code-block:: none

NoneType
bool
int
unsigned int
Expand Down
@@ -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.
21 changes: 20 additions & 1 deletion Modules/_io/clinic/iobase.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 8 additions & 9 deletions Modules/_io/iobase.c
Expand Up @@ -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
/

Expand All @@ -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 */
Expand All @@ -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"
Expand Down
30 changes: 16 additions & 14 deletions Tools/clinic/clinic.py
Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -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())
Expand Down