GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Argument Clinic includes internal headers in generated output unconditionally · Issue #107603 · python/cpython

Argument Clinic includes internal headers in generated output unconditionally #107603

New issue
Copy link
New issue
Copy link
Closed
Closed
Argument Clinic includes internal headers in generated output unconditionally#107603
Copy link
Labels
topic-argument-clinictype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

Description

@erlend-aasland
erlend-aasland
opened on Aug 3, 2023
Issue body actions

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

  • 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

Activity

  1. erlend-aasland
    added
    type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
    topic-argument-clinic
    on Aug 3, 2023
  2. erlend-aasland commented on Aug 3, 2023

    @erlend-aasland
    erlend-aasland
    on Aug 3, 2023
    ContributorAuthor
    More actions

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

  3. kumaraditya303 commented on Aug 5, 2023

    @kumaraditya303
    kumaraditya303
    on Aug 5, 2023
    Contributor
    More actions

    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.

  4. erlend-aasland commented on Aug 5, 2023

    @erlend-aasland
    erlend-aasland
    on Aug 5, 2023
    ContributorAuthor
    More actions

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

  5. vstinner
    added 2 commits that reference this issue on Aug 25, 2023

    pythongh-107603: Argument Clinic can emit includes

    743d8ff

    pythongh-107603: Argument Clinic can emit includes

    cdadfa1
  6. vstinner commented on Aug 25, 2023

    @vstinner
    vstinner
    on Aug 25, 2023
    Member
    More actions

    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.

  7. vstinner
    added 2 commits that reference this issue on Aug 25, 2023

    pythongh-107603: Argument Clinic can emit includes

    830fb64

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

    73d33c1
  8. AlexWaygood
    added a commit that references this issue on Aug 28, 2023

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

    0bd2ba5
  9. vstinner
    added 11 commits that reference this issue on Aug 31, 2023

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

    669a72a

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

    65a4548

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

    e8ea3f9

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

    b8efef7

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

    f989d08

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

    750b40b

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

    df2c754

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

    48e7081

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

    46d3ff0

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

    a3f0c76

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

    ad73674
  10. hugovk commented on Nov 9, 2023

    @hugovk
    hugovk
    on Nov 9, 2023
    Member
    More actions

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

  11. vstinner commented on Nov 9, 2023

    @vstinner
    vstinner
    on Nov 9, 2023
    Member
    More actions

    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.

    Reacted by Hugo van Kemenade
  12. vstinner
    closed this as completedon Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    topic-argument-clinictype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions

      You are visitor 01402503 — GeezerVison does not store the pages you look up.
      Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.