Argument Clinic includes internal headers in generated output unconditionally

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:

Details

/*[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:

Details

/*[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

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

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.

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

pythongh-107603: Argument Clinic can emit includes

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.

pythongh-107603: Argument Clinic can emit includes

gh-107603: Argument Clinic can emit includes (#108486)

GH-107603: Clinic: Pass specific attributes to ``print_block()`` (#10…

pythongh-107603: AC only includes pycore_gc.h if needed

gh-107603: Argument Clinic: Only include pycore_gc.h if needed (#108726)

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

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.

Navigation Menu

Repository navigation

Argument Clinic includes internal headers in generated output unconditionally #107603

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Argument Clinic includes internal headers in generated output unconditionally #107603

Description

Linked PRs

Activity

erlend-aasland commented on Aug 3, 2023

kumaraditya303 commented on Aug 5, 2023

erlend-aasland commented on Aug 5, 2023

vstinner commented on Aug 25, 2023

hugovk commented on Nov 9, 2023

vstinner commented on Nov 9, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions