Skip to content

Conversation

@vstinner
Copy link
Member

@vstinnervstinner commented Oct 17, 2023

Move the following private functions and structures to pycore_modsupport.h internal C API:

  • _PyArg_BadArgument()
  • _PyArg_CheckPositional()
  • _PyArg_NoKeywords()
  • _PyArg_NoPositional()
  • _PyArg_ParseStack()
  • _PyArg_ParseStackAndKeywords()
  • _PyArg_Parser structure
  • _PyArg_UnpackKeywords()
  • _PyArg_UnpackKeywordsWithVararg()
  • _PyArg_UnpackStack()
  • _Py_ANY_VARARGS()

Changes:

  • Python/getargs.h now includes pycore_modsupport.h to export functions.

  • clinic.py now adds pycore_modsupport.h when one of these functions is used.

  • Add pycore_modsupport.h includes when a C extension uses one of these functions.

  • Define Py_BUILD_CORE_MODULE in C extensions which now include directly or indirectly (via code generated by Argument Clinic) pycore_modsupport.h:

    • _csv
    • _curses_panel
    • _dbm
    • _gdbm
    • _multiprocessing.posixshmem
    • _sqlite.row
    • _statistics
    • grp
    • resource
    • syslog
  • _testcapi: bad_get() no longer uses METH_FASTCALL calling convention but METH_VARARGS. Replace _PyArg_UnpackStack() with PyArg_ParseTuple().

  • _testcapi: add PYTESTCAPI_NEED_INTERNAL_API macro which is defined by _testcapi sub-modules which need the internal C API (pycore_modsupport.h): exceptions.c, float.c, vectorcall.c, watchers.c.

  • Remove Include/cpython/modsupport.h header file. Include/modsupport.h no longer includes the removed header file.

@vstinner
Copy link
MemberAuthor

This change is big because it changes 122 files generated by Argument Clinic:

$ git show --stat|grep 'clinic/.*.c.h'|wc -l 122 

I prefer to write a single PR to move all private _PyArg functions at once, because each moved function touch many files generated by AC.

Copy link
Member

@AlexWaygoodAlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AC changes look fine to me! (I'll leave the question of whether the changes are worth making to those more skilled in C.)

Move the following private functions and structures to pycore_modsupport.h internal C API: * _PyArg_BadArgument() * _PyArg_CheckPositional() * _PyArg_NoKeywords() * _PyArg_NoPositional() * _PyArg_ParseStack() * _PyArg_ParseStackAndKeywords() * _PyArg_Parser structure * _PyArg_UnpackKeywords() * _PyArg_UnpackKeywordsWithVararg() * _PyArg_UnpackStack() * _Py_ANY_VARARGS() Changes: * Python/getargs.h now includes pycore_modsupport.h to export functions. * clinic.py now adds pycore_modsupport.h when one of these functions is used. * Add pycore_modsupport.h includes when a C extension uses one of these functions. * Define Py_BUILD_CORE_MODULE in C extensions which now include directly or indirectly (via code generated by Argument Clinic) pycore_modsupport.h: * _csv * _curses_panel * _dbm * _gdbm * _multiprocessing.posixshmem * _sqlite.row * _statistics * grp * resource * syslog * _testcapi: bad_get() no longer uses METH_FASTCALL calling convention but METH_VARARGS. Replace _PyArg_UnpackStack() with PyArg_ParseTuple(). * _testcapi: add PYTESTCAPI_NEED_INTERNAL_API macro which is defined by _testcapi sub-modules which need the internal C API (pycore_modsupport.h): exceptions.c, float.c, vectorcall.c, watchers.c. * Remove Include/cpython/modsupport.h header file. Include/modsupport.h no longer includes the removed header file. * Fix mypy clinic.py
@vstinner
Copy link
MemberAuthor

For clinic.py, I was lazy and used the clinic global variable in bad_argument(). The correct fix is to pass a clinic argument to add bad_argument() methods and calling sites. Problem: there are many. Second problem: I wrote such change many times, but I lost my work since the overall change was blocked for different reasons. Also, I tried to keep this PR as small as possible.

In short, clinic.py should be reworked later, once this change lands.

Comment on lines +3523 to +3524
ifclinicisnotNone:
clinic.add_include('pycore_modsupport.h', '_PyArg_BadArgument()')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or perhaps this?

Suggested change
ifclinicisnotNone:
clinic.add_include('pycore_modsupport.h', '_PyArg_BadArgument()')
assertclinicisnotNone
clinic.add_include('pycore_modsupport.h', '_PyArg_BadArgument()')

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As explained in my previous comment, I plan to write a follow-up for this code. We should not use the global variable, but pass an argument which cannot be None.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree that's the more principled action in the longer term, and I'm fine with using the easier solution for now. But this is the same number of lines, and I think makes it clearer that we never expect clinic to be None here (if it is None, something has gone horribly wrong :-)

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR to refactor this code, to avoid if clinic is not None: PR #110982.

@vstinnervstinner merged commit be5e8a0 into python:mainOct 17, 2023
@vstinnervstinner deleted the pycore_modsupport branch October 17, 2023 12:30
@vstinner
Copy link
MemberAuthor

For clinic.py, I was lazy and used the clinic global variable in bad_argument().

I wrote PR #110984 for that.

@vstinner
Copy link
MemberAuthor

I merged my PR. @AlexWaygood: thanks for reviewing clinic.py changes.

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
Move the following private functions and structures to pycore_modsupport.h internal C API: * _PyArg_BadArgument() * _PyArg_CheckPositional() * _PyArg_NoKeywords() * _PyArg_NoPositional() * _PyArg_ParseStack() * _PyArg_ParseStackAndKeywords() * _PyArg_Parser structure * _PyArg_UnpackKeywords() * _PyArg_UnpackKeywordsWithVararg() * _PyArg_UnpackStack() * _Py_ANY_VARARGS() Changes: * Python/getargs.h now includes pycore_modsupport.h to export functions. * clinic.py now adds pycore_modsupport.h when one of these functions is used. * Add pycore_modsupport.h includes when a C extension uses one of these functions. * Define Py_BUILD_CORE_MODULE in C extensions which now include directly or indirectly (via code generated by Argument Clinic) pycore_modsupport.h: * _csv * _curses_panel * _dbm * _gdbm * _multiprocessing.posixshmem * _sqlite.row * _statistics * grp * resource * syslog * _testcapi: bad_get() no longer uses METH_FASTCALL calling convention but METH_VARARGS. Replace _PyArg_UnpackStack() with PyArg_ParseTuple(). * _testcapi: add PYTESTCAPI_NEED_INTERNAL_API macro which is defined by _testcapi sub-modules which need the internal C API (pycore_modsupport.h): exceptions.c, float.c, vectorcall.c, watchers.c. * Remove Include/cpython/modsupport.h header file. Include/modsupport.h no longer includes the removed header file. * Fix mypy clinic.py
hroncok added a commit to hroncok/pygame that referenced this pull request Jun 18, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Move the following private functions and structures to pycore_modsupport.h internal C API: * _PyArg_BadArgument() * _PyArg_CheckPositional() * _PyArg_NoKeywords() * _PyArg_NoPositional() * _PyArg_ParseStack() * _PyArg_ParseStackAndKeywords() * _PyArg_Parser structure * _PyArg_UnpackKeywords() * _PyArg_UnpackKeywordsWithVararg() * _PyArg_UnpackStack() * _Py_ANY_VARARGS() Changes: * Python/getargs.h now includes pycore_modsupport.h to export functions. * clinic.py now adds pycore_modsupport.h when one of these functions is used. * Add pycore_modsupport.h includes when a C extension uses one of these functions. * Define Py_BUILD_CORE_MODULE in C extensions which now include directly or indirectly (via code generated by Argument Clinic) pycore_modsupport.h: * _csv * _curses_panel * _dbm * _gdbm * _multiprocessing.posixshmem * _sqlite.row * _statistics * grp * resource * syslog * _testcapi: bad_get() no longer uses METH_FASTCALL calling convention but METH_VARARGS. Replace _PyArg_UnpackStack() with PyArg_ParseTuple(). * _testcapi: add PYTESTCAPI_NEED_INTERNAL_API macro which is defined by _testcapi sub-modules which need the internal C API (pycore_modsupport.h): exceptions.c, float.c, vectorcall.c, watchers.c. * Remove Include/cpython/modsupport.h header file. Include/modsupport.h no longer includes the removed header file. * Fix mypy clinic.py
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@vstinner@AlexWaygood