Argument Clinic: generated code contains redundant gotos

[eltoder]

Bug report

Bug description:

Generated code for argument parsing contains redundant gotos when there are both positional and keyword-only arguments with default values. Take this example:

/*[clinic input]
test.func -> int

    x: object
    y: object = None
    *
    z: object = None

Test function
[clinic start generated code]*/

This produces

    args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 1, 2, 0, argsbuf);
    if (!args) {
        goto exit;
    }
    x = args[0];
    if (!noptargs) {
        goto skip_optional_pos;
    }
    if (args[1]) {
        y = args[1];
        if (!--noptargs) {
            goto skip_optional_pos;
        }
    }
skip_optional_pos:
    if (!noptargs) {
        goto skip_optional_kwonly;
    }
    z = args[2];
skip_optional_kwonly:
    _return_value = test_func_impl(module, x, y, z);

Note that all gotos to skip_optional_pos happen only when !noptargs, but skip_optional_pos first checks the exact same condition and immediately jumps to skip_optional_kwonly. We can remove this double jump and extra check and instead generate simpler code:

    x = args[0];
    if (!noptargs) {
        goto skip_optional;
    }
    if (args[1]) {
        y = args[1];
        if (!--noptargs) {
            goto skip_optional;
        }
    }
    z = args[2];
skip_optional:

This code is generated if z is not keyword-only but a regular positional argument.

In general, it does not seem to make sense to have separate skip_optional_pos and skip_optional_kwonly, since there is no way to tell them apart. We only have the combined number of optional arguments (noptargs), which includes both positional and keyword only. (This is unlike positional-only arguments, where we can also check nargs.)

CC @erlend-aasland @AlexWaygood @serhiy-storchaka

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

• gh-113367: Avoid redundant gotos and labels in Argument Clinic #158284

[serhiy-storchaka]

Two labels are needed if there are required keyword-only parameters.

/*[clinic input]
test.func -> int

    x: object
    y: object = None
    *
    z: object
    t: object = None

Test function
[clinic start generated code]*/

It is not currently supported by PyArg_ParseTupleWithKeywords(), but there are plans to support it.

If it is possible to make Argument Clinic to generate simpler code in special cases without significant complication of the generator code -- it would be good. The current code is a compromise between simplicity of the generator and efficiency of the generated code. I believe that modern C compilers are smart enough to eliminate unneeded tests and jumps, so it only affects human readability.

[eltoder]

Two labels are needed if there are required keyword-only parameters.

Can you elaborate why is it required? If there are required keyword-only arguments, noptargs will never reach 0 until we get to the keyword-only arguments, so all gotos before that can be omitted.

I believe that modern C compilers are smart enough to eliminate unneeded tests and jumps, so it only affects human readability.

C compilers can optimize the jumps, but there's no way for them to remove the extra check. This relies on the contract of _PyArg_UnpackKeywords() function that is not known to compilers.

[eltoder]

Can you elaborate why is it required? If there are required keyword-only arguments, noptargs will never reach 0 until we get to the keyword-only arguments, so all gotos before that can be omitted.

Ah, I take it back. Yes, in this case you can make use of 2 labels.

[eltoder]

It seems straightforward to use 2 labels when there are required keyword-only arguments and 1 label when there are only optional keyword-only arguments. The biggest issue seems to be that the former case is not actually supported yet, so there is no way to test that it works :-)