Description

noamcohen97

opened on Jul 19, 2022

Issue body actions

Bug report

/*[clinic input]
function -> NoneType
[clinic start generated code]*/

will produce this function:

static PyObject *
function(PyObject *module, PyObject *Py_UNUSED(ignored))
{
    return function_impl(module);
}

while function_impl returns Py_None (as it should, if using NoneType return converter), refcount is not increased.

here is a working example of NoneType return converter:

/*[clinic input]
function -> NoneType

    a: int
    /
[clinic start generated code]*/

static PyObject *
function(PyObject *module, PyObject *arg)
{
    PyObject *return_value = NULL;
    int a;
    PyObject *_return_value;

    a = _PyLong_AsInt(arg);
    if (a == -1 && PyErr_Occurred()) {
        goto exit;
    }
    _return_value = function_impl(module, a);
    if (_return_value != Py_None) {
        goto exit;
    }
    return_value = Py_None;
    Py_INCREF(Py_None);

exit:
    return return_value;
}

Generation also fails when generating a function with a single object parameter.

It seems like the problem is with default_return_converter in clinic.py, being set to True

Reacted by Shantanu

Activity

1. noamcohen97

   added

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Jul 19, 2022
2. erlend-aasland

   added

   topic-argument-clinic

   on Jul 19, 2022
3. serhiy-storchaka

   added a commit that references this issue on Jul 19, 2022

   pythongh-95007: Remove the NoneType return converter

   f7cbbe7
4. serhiy-storchaka commented on Jul 19, 2022

   

   serhiy-storchaka

   on Jul 19, 2022

   Member

   More actions

   Good catch. This converter is not used, so the bug was not exposed. And since it has dubious semantic (requires returning a weak reference to None on success), I think that it is better to remove it.

   Reacted by Erlend E. Aasland and Noam Cohen
5. noamcohen97 commented on Jul 19, 2022

   

   noamcohen97

   on Jul 19, 2022

   ContributorAuthor

   More actions

   How about we'll get rid of the callback return value, and use PyErr_Occurred() to chose whether to return NULL or Py_None?
6. arhadthedev commented on Jul 19, 2022

   

   arhadthedev

   on Jul 19, 2022

   Member

   More actions

   How about we'll get rid of the callback return value, and use PyErr_Occurred() to chose whether to return NULL or Py_None?

   When I did the same in gh-91284, I found that PyErr_Occurred takes GIL. Currently (until nogil is implemented, at least) it's cheaper to deal with PyErr_Occurred only on NULL returned from a callback.
7. serhiy-storchaka

   added a commit that references this issue on Jul 20, 2022

   gh-95007: Remove the NoneType return converter (GH-95019)

   74b5e4c
8. serhiy-storchaka

   closed this as completedon Jul 20, 2022
9. erlend-aasland

   added a commit that references this issue on Aug 1, 2022

   gh-95007: Remove the NoneType return converter from Argument Clinic D…

   347c783
10. erlend-aasland commented on Aug 1, 2022

    

    erlend-aasland

    on Aug 1, 2022

    Contributor

    More actions

    Resolved with:

    • gh-95007: Remove the NoneType return converter #95019
    • gh-95007: Remove the NoneType return converter from Argument Clinic Doc #95529
11. AA-Turner

    added a commit that references this issue on Sep 13, 2023

    python/cpython#95007: Remove the NoneType return converter from Argum…

    d6ff211
12. erlend-aasland

    added a commit that references this issue on Sep 26, 2023

    python/cpython#95007: Remove the NoneType return converter from Argum…

    2f81c6b

Issue actions