Argument Clinic includes internal headers in generated output unconditionally

[erlend-aasland]

It seems like Argument Clinic generates code that define Py_BUILD_CORE and includes internal headers unconditionally; for example, a file with a METH_O function will have no need for those:

/*[clinic input]
preserve
[clinic start generated code]*/

#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)
#  include "pycore_gc.h"            // PyGC_Head
#  include "pycore_runtime.h"       // _Py_ID()
#endif


PyDoc_STRVAR(func__doc__,
"func($module, a, /)\n"
"--\n"
"\n");

#define FUNC_METHODDEF    \
    {"func", (PyCFunction)func, METH_O, func__doc__},
/*[clinic end generated code: output=851b6645c29cfa0d input=a9049054013a1b77]*/

OTOH, a METH_KEYWORDS function needs those:

/*[clinic input]
preserve
[clinic start generated code]*/

#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)
#  include "pycore_gc.h"            // PyGC_Head
#  include "pycore_runtime.h"       // _Py_ID()
#endif


PyDoc_STRVAR(func__doc__,
"func($module, /, a, b)\n"
"--\n"
"\n");

#define FUNC_METHODDEF    \
    {"func", _PyCFunction_CAST(func), METH_FASTCALL|METH_KEYWORDS, func__doc__},

static PyObject *
func_impl(PyObject *module, PyObject *a, PyObject *b);

static PyObject *
func(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames)
{
    PyObject *return_value = NULL;
    #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)

    #define NUM_KEYWORDS 2
    static struct {
        PyGC_Head _this_is_not_used;
        PyObject_VAR_HEAD
        PyObject *ob_item[NUM_KEYWORDS];
    } _kwtuple = {
        .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS)
        .ob_item = { &_Py_ID(a), &_Py_ID(b), },
    };
    #undef NUM_KEYWORDS
    #define KWTUPLE (&_kwtuple.ob_base.ob_base)

    #else  // !Py_BUILD_CORE
    #  define KWTUPLE NULL
    #endif  // !Py_BUILD_CORE

    static const char * const _keywords[] = {"a", "b", NULL};
    static _PyArg_Parser _parser = {
        .keywords = _keywords,
        .fname = "func",
        .kwtuple = KWTUPLE,
    };
    #undef KWTUPLE
    PyObject *argsbuf[2];
    PyObject *a;
    PyObject *b;

    args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 2, 2, 0, argsbuf);
    if (!args) {
        goto exit;
    }
    a = args[0];
    b = args[1];
    return_value = func_impl(module, a, b);

exit:
    return return_value;
}
/*[clinic end generated code: output=e790cd95ffc517a0 input=a9049054013a1b77]*/

Argument Clinic should check if those defines/includes are needed before generating the output.

Linked PRs

• gh-107603: Argument Clinic can emit includes #108486
• GH-107603: Clinic: Pass specific attributes to print_block() #108581
• gh-107603: AC only includes pycore_gc.h if needed #108715
• gh-107603: Argument Clinic: Only include pycore_gc.h if needed #108726

[erlend-aasland]

'unconditionally' was a bit harsh, but it does generate core includes for some cases where it is not needed.

[kumaraditya303]

I don't think this is an important issue, this is more of a "nice to have" rather than necessary, it will probably add more conditional code. I certainly don't consider this as a bug though.

[erlend-aasland]

Yeah, it's not an important issue. But/feature? I'd say it is an undesirable feature, then :)

[vstinner]

I don't think this is an important issue, this is more of a "nice to have" rather than necessary, it will probably add more conditional code. I certainly don't consider this as a bug though.

I would like this feature to be able to remove private functions only used by AC from the public C API: issue #106320.

[hugovk]

Is there more to do for this, or can the issue be closed?

[vstinner]

There might be room for improvement: the #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) code is still generated for C code which doesn't define the Py_LIMITED_API macro.

Argument Clinic now generates #include for needed symbols. IMO the situation is less bad than before. Moreover, Argument Clinic now supports the limited C API: when targeting the limited C API, no pycore_ internal header is included, and so the problem is solved for these extensions.

While there is room for improvement, I close the issue: further changes can be done in a even more specific issue.