Derby #17: Convert 49 sites to Argument Clinic across 13 files

[larryhastings]

BPO	20185
Nosy	@terryjreedy, @vstinner, @larryhastings, @vadmium, @zware, @serhiy-storchaka, @vajrasky, @MojoVampire, @JulienPalard
PRs	bpo-20185: Convert the marshal module to Argument Clinic. #541 bpo-20185: Convert list object implementation to Argument Clinic. #542 bpo-20185: Convert float object implementation to Argument Clinic. #543 bpo-20185: Convert typeobject.c to Argument Clinic. #544 bpo-20185: Convert the resource module to Argument Clinic. #545 [3.6]bpo-20185: Adjust IDLE test to 3.7 Clinic change [GH-542] #2116 bpo-46852: Rename float.__set_format__() to float.__setformat__() #31558 [3.10] bpo-46852: Rename float.__set_format__() to float.__setformat__() (GH-31558) #31578 [3.9] bpo-46852: Rename float.__set_format__() to float.__setformat__() (GH-31558) #31581
Dependencies	bpo-28754: Argument Clinic for bisect.bisect_left
Files	issue20185_conglomerate_v4.diff clinic_gc_v4.patch clinic_longobject_v3.patch clinic_longobject_v4.patch clinic_longobject_v5.patch clinic_gc_v5.patch clinic_float_v5.diff clinic_list_v5.diff clinic_type_v5.diff clinic_marshal_v5.diff clinic_resource_v5.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-05-07.14:09:44.504>
created_at = <Date 2014-01-08.00:22:19.535>
labels = ['extension-modules', 'type-feature', '3.7', 'expert-argument-clinic']
title = 'Derby #17: Convert 49 sites to Argument Clinic across 13 files'
updated_at = <Date 2022-02-25.15:13:53.923>
user = 'https://github.com/larryhastings'

bugs.python.org fields:

activity = <Date 2022-02-25.15:13:53.923>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2019-05-07.14:09:44.504>
closer = 'mdk'
components = ['Extension Modules', 'Argument Clinic']
creation = <Date 2014-01-08.00:22:19.535>
creator = 'larry'
dependencies = ['28754']
files = ['33943', '33988', '33989', '46392', '46449', '46495', '46526', '46527', '46528', '46529', '46530']
hgrepos = []
issue_num = 20185
keywords = ['patch']
message_count = 65.0
messages = ['207643', '208124', '208140', '208150', '208154', '208246', '208247', '208269', '208271', '208529', '208632', '208762', '208768', '208774', '208877', '208883', '208884', '209048', '209419', '209432', '209516', '209517', '209520', '209548', '209570', '210142', '210168', '210235', '210326', '210327', '210330', '210350', '210388', '210389', '210393', '210434', '210441', '210594', '210595', '224766', '282110', '282790', '282893', '282894', '282906', '283579', '285963', '285980', '286093', '286436', '286698', '286817', '286936', '286942', '287064', '290164', '290205', '290215', '290227', '290228', '295725', '341729', '413954', '413998', '414006']
nosy_count = 11.0
nosy_names = ['terry.reedy', 'vstinner', 'larry', 'nadeem.vawda', 'python-dev', 'martin.panter', 'zach.ware', 'serhiy.storchaka', 'vajrasky', 'josh.r', 'mdk']
pr_nums = ['541', '542', '543', '544', '545', '2116', '31558', '31578', '31581']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue20185'
versions = ['Python 3.7']

[larryhastings]

This issue is part of the Great Argument Clinic Conversion Derby,
where we're trying to convert as much of Python 3.4 to use
Argument Clinic as we can before Release Candidate 1 on January 19.

This issue asks you to change the following bundle of files:
Objects/typeobject.c: 5 sites
Objects/longobject.c: 5 sites
Objects/listobject.c: 5 sites
Objects/floatobject.c: 4 sites
Modules/resource.c: 4 sites
Modules/_threadmodule.c: 4 sites
Modules/_bz2module.c: 4 sites
Modules/_bisectmodule.c: 4 sites
Python/marshal.c: 3 sites
Objects/exceptions.c: 3 sites
Modules/nismodule.c: 3 sites
Modules/gcmodule.c: 3 sites
Python/_warnings.c: 2 sites
Modules/_cryptmodule.c: 1 sites

Talk to me (larry) if you only want to attack part of a bundle.

For instructions on how to convert a function to work with Argument
Clinic, read the "howto":
http://docs.python.org/dev/howto/clinic.html

[larryhastings]

Antoine Pitrou took care of cryptmodule.c.

[vajrasky]

Here is the patch for Modules/resource.c.

However, I can not convert this method signature:

if (!PyArg_ParseTuple(args, _Py_PARSE_PID "i|(OO):prlimit",
                          &pid, &resource, &curobj, &maxobj))

_Py_PARSE_PID can be 'i' or 'l' or 'L'.

[vajrasky]

Here is the patch for Modules/gcmodule.c.

[vajrasky]

Here is the patch for Python/marshal.c.

A couple of issues:

1. I can not have bytes as argument.

   bytes: Py_buffer -> not possible

So I changed it to byte.

2. I can not give default value, marshal.version, to argument.

version: int(c_default="Py_MARSHAL_VERSION") = marshal.version -> not possible (unless I put "import marshal" in Tools/clinic/clinic.py).

So I gave it a raw value, 2.

version: int(c_default="Py_MARSHAL_VERSION") = 2

[larryhastings]

Someone else came up with this hack using a custom converter:

class PID_converter(int_converter):
  format_unit = '" _Py_PARSE_PID "'

Also, for marshal.version, soon you will be able to. This syntax won't work now but it will soon:

version: int(c_default="Py_MARSHAL_VERSION") = version

[larryhastings]

Keep track of bpo-20226, that's the issue where I'm adding improved support for simple expressions like "version".

[vajrasky]

With little modification:

class PID_converter(int_converter):
  type = 'pid_t'
  format_unit = '" _Py_PARSE_PID "'

It works. Thanks! But I got unnecessary empty string:

if (!PyArg_ParseTuple(args, "" _Py_PARSE_PID "i:prlimit", &pid, &resource))

It should be:

if (!PyArg_ParseTuple(args, _Py_PARSE_PID "i:prlimit", &pid, &resource))

Anyway, that is trivial problem. Now I hit a hard one. How do you convert this signature?

• if (!PyArg_ParseTuple(args, _Py_PARSE_PID "i|(OO):prlimit",

•                      &pid, &resource, &curobj, &maxobj))
  

I create custom converters:

+class TupleOpen_converter(object_converter):
+ format_unit = '(O'
+class TupleClose_converter(object_converter):
+ format_unit = 'O)'

and

+/[clinic input]
+resource.prlimit
+
+ pid: PID
+ resource: int
+ [
+ curobj: TupleOpen
+ maxobj: TupleClose
+ ]
+ /
+[clinic start generated code]/

But I got invalid argument size in the generated code.

+ case 2:
+ if (!PyArg_ParseTuple(args, "" _Py_PARSE_PID "i:prlimit", &pid, &resource))
+ return NULL;
+ break;
+ case 4: ========> should be "case 3:"
+ if (!PyArg_ParseTuple(args, "" _Py_PARSE_PID "i(OO):prlimit", &pid, &resource, &curobj, &maxobj))
+ return NULL;
+ group_right_1 = 1;
+ break;
+ default:
+ PyErr_SetString(PyExc_TypeError, "resource.prlimit requires 2 to 4 arguments"); =======> should be "2 to 3 arguments"
+ return NULL;

Any idea, Larry?

[serhiy-storchaka]

This is known bug in prlimit (bpo-20191).

[vajrasky]

Here is the patch for typeobject. I didn't convert the new method. It's so complicated. It counts whether how many arguments or keywords you pass. It asserts the args before parsing the args. I don't think clinic supports this.

This is the code:
assert(args != NULL && PyTuple_Check(args));
assert(kwds == NULL || PyDict_Check(kwds));

    /* Special case: type(x) should return x->ob_type */
    {
        const Py_ssize_t nargs = PyTuple_GET_SIZE(args);
        const Py_ssize_t nkwds = kwds == NULL ? 0 : PyDict_Size(kwds);

        if (PyType_CheckExact(metatype) && nargs == 1 && nkwds == 0) {
            PyObject *x = PyTuple_GET_ITEM(args, 0);
            Py_INCREF(Py_TYPE(x));
            return (PyObject *) Py_TYPE(x);
        }

        /* SF bug 475327 -- if that didn't trigger, we need 3
           arguments. but PyArg_ParseTupleAndKeywords below may give
           a msg saying type() needs exactly 3. */
        if (nargs + nkwds != 3) {
            PyErr_SetString(PyExc_TypeError,
                            "type() takes 1 or 3 arguments");
            return NULL;
        }
    }

/* Check arguments: (name, bases, dict) */
if (!PyArg_ParseTupleAndKeywords(args, kwds, "UO!O!:type", kwlist,
                                 &name,
                                 &PyTuple_Type, &bases,
                                 &PyDict_Type, &orig_dict))
    return NULL;

[vajrasky]

Here is the patch for longobject. There are two sites which I couldn't convert. The first is the constructor which is complicated. The other one is __round__ which clinic explicitly does not support.

[vajrasky]

Here is the patch for listobject. A couple of thoughts:

1. I can not convert this signature:

    if (!PyArg_ParseTuple(args, "O|O&O&:index", &v,
                                _PyEval_SliceIndex, &start,
                                _PyEval_SliceIndex, &stop))
        return NULL;

It's bloody difficult. If I make start (and stop) as PyObject, I get a lot of pointer warning conversion. The closest I can get is making start (and stop) as Py_ssize_t but it changes the behaviour. a.index(0, -4*sys.maxsize, 4*sys.maxsize) will throw OverflowError.

2. for tp_init, it forces me to give wrong return signature.
   "__init__([sequence=None])\n"

This is the clinic input:

list.__init__

self: PyListObject
[
sequence: object(c_default="NULL") = None
]
/

Without None, it will be an error in clinic process.

3. The init function returns integer but in error case, it returns NULL which will throws pointer warning conversion.

static int
list___init__(PyObject *self, PyObject *args, PyObject *kwargs)
{
    int return_value = -1;
    int group_right_1 = 0;
    PyObject *sequence = NULL;

    if (!_PyArg_NoKeywords("__init__", kwargs))
        goto exit;
    switch (PyTuple_GET_SIZE(args)) {
        case 0:
            break;
        case 1:
            if (!PyArg_ParseTuple(args, "O:__init__", &sequence))
                return NULL;
            group_right_1 = 1;
            break;
....