GeezerVison

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

GeezerVison is showing github.com — gh-110964: Remove private _PyArg functions by vstinner · Pull Request #110966 · python/cpython

Conversation

vstinner commented Oct 17, 2023 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

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.

Issue: [C API] Move undocumented private _PyArg C API to pycore_modsupport.h internal C API #110964

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

vstinner requested review from a team, 1st1, AlexWaygood, abalkin, asvetlov, berkerpeksag, brettcannon, corona10, ericsnowcurrently, erlend-aasland, ethanfurman, gvanrossum, iritkatriel, kumaraditya303, markshannon, ncoghlan, pganssle, rhettinger, tiran, warsaw and willingc as code owners

October 17, 2023 08:13

bedevere-app Bot mentioned this pull request

Oct 17, 2023

[C API] Move undocumented private _PyArg C API to pycore_modsupport.h internal C API #110964

Closed

bedevere-app Bot added the awaiting core review label

Oct 17, 2023

vstinner commented Oct 17, 2023

Copy link

Copy Markdown

Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner added the skip news label

Oct 17, 2023

This was referenced Oct 17, 2023

Remove unused _PyArg_VaParseTupleAndKeywordsFast() #107196

Closed

gh-85283: Fix Argument Clinic for md5 extension #110976

Merged

AlexWaygood reviewed

Oct 17, 2023

View reviewed changes

AlexWaygood left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this 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.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          pythongh-110964: Remove private _PyArg functions

46a846d

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 force-pushed the pycore_modsupport branch from 1c53867 to 46a846d Compare

October 17, 2023 11:47


          Fix mypy --config-file Tools/clinic/mypy.ini

8a4f9af

vstinner commented Oct 17, 2023

Copy link

Copy Markdown

Member Author

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.

AlexWaygood reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood reviewed

Oct 17, 2023

View reviewed changes

Comment thread

Tools/clinic/clinic.py

Comment on lines +3523 to +3524

		if clinic is not None:
		clinic.add_include('pycore_modsupport.h', '_PyArg_BadArgument()')

AlexWaygood Oct 17, 2023

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

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

	if clinic is not None:
	clinic.add_include('pycore_modsupport.h', '_PyArg_BadArgument()')
	assert clinic is not None
	clinic.add_include('pycore_modsupport.h', '_PyArg_BadArgument()')

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

vstinner Oct 17, 2023

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

AlexWaygood Oct 17, 2023

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

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 :-)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland reacted with thumbs up emoji

All reactions

👍 1 reaction

vstinner Oct 17, 2023

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

vstinner merged commit be5e8a0 into python:main

Oct 17, 2023

vstinner deleted the pycore_modsupport branch

October 17, 2023 12:30

bedevere-app Bot removed the awaiting core review label

Oct 17, 2023

vstinner commented Oct 17, 2023

Copy link

Copy Markdown

Member Author

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

I wrote PR #110984 for that.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented Oct 17, 2023

Copy link

Copy Markdown

Member Author

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

AlexWaygood reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aisk pushed a commit to aisk/cpython that referenced this pull request

Feb 11, 2024


          pythongh-110964: Remove private _PyArg functions (python#110966)

27cadf1

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


          Inline _PyArg_NoKeywords from Python 3.13 in case it is missing

7f36fde

Python 3.13 made this private:
python/cpython#110966

Fixes pygame#4099

hroncok mentioned this pull request

Jun 18, 2024

Inline _PyArg_NoKeywords from Python 3.13 in case it is missing pygame/pygame#4269

Merged

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request

Sep 2, 2024


          pythongh-110964: Remove private _PyArg functions (python#110966)

2fc8127

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

Reviewers

AlexWaygood

AlexWaygood left review comments

erlend-aasland

Awaiting requested review from erlend-aasland

erlend-aasland is a code owner

corona10

Awaiting requested review from corona10

corona10 is a code owner

ethanfurman

Awaiting requested review from ethanfurman

rhettinger

Awaiting requested review from rhettinger

rhettinger is a code owner

berkerpeksag

Awaiting requested review from berkerpeksag

pganssle

Awaiting requested review from pganssle

abalkin

Awaiting requested review from abalkin

brettcannon

Awaiting requested review from brettcannon

ericsnowcurrently

Awaiting requested review from ericsnowcurrently

ericsnowcurrently is a code owner

ncoghlan

Awaiting requested review from ncoghlan

warsaw

Awaiting requested review from warsaw

tiran

Awaiting requested review from tiran

iritkatriel

Awaiting requested review from iritkatriel

iritkatriel is a code owner

markshannon

Awaiting requested review from markshannon

markshannon is a code owner

1st1

Awaiting requested review from 1st1

1st1 is a code owner

asvetlov

Awaiting requested review from asvetlov

asvetlov is a code owner

gvanrossum

Awaiting requested review from gvanrossum

kumaraditya303

Awaiting requested review from kumaraditya303

kumaraditya303 is a code owner

willingc

Awaiting requested review from willingc

willingc is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — 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.

GeezerVison

Conversation

vstinner commented Oct 17, 2023 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner commented Oct 17, 2023

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Oct 17, 2023

Uh oh!

AlexWaygood Oct 17, 2023

Choose a reason for hiding this comment

Uh oh!

vstinner Oct 17, 2023

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Oct 17, 2023

Choose a reason for hiding this comment

Uh oh!

vstinner Oct 17, 2023

Choose a reason for hiding this comment

Uh oh!

vstinner commented Oct 17, 2023

Uh oh!

vstinner commented Oct 17, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vstinner commented Oct 17, 2023 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.