Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Re-work support of var-positional parameters in Argument Clinic #122943

Closed
serhiy-storchaka opened this issue Aug 12, 2024 · 1 comment
Closed

Re-work support of var-positional parameters in Argument Clinic #122943

serhiy-storchaka opened this issue Aug 12, 2024 · 1 comment
Labels
topic-argument-clinic

Comments

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Aug 12, 2024 •
edited by bedevere-app bot
Loading

When fixing some bugs (#118814, #122688) I noticed also design problems in _PyArg_UnpackKeywordsWithVararg().

  • Its vararg parameter is always equal to maxpos, therefore it is redundant.
  • Inserting a new tuple between values for positional and keyword-only parameters adds complexity in the _PyArg_UnpackKeywordsWithVararg() code which caused bugs.
  • Since it is the only argument value which is a strong reference, it adds complexity for cleanup after calling _PyArg_UnpackKeywordsWithVararg().
  • This large code is mostly a duplication of _PyArg_UnpackKeywords().
  • But it lacks some microoptimizations.
  • And produces wrong error messages in some corner cases.
  • Also some cases (like var-positional parameter after optional parameters) were not supported.

So I re-worked it in several steps:

  • Removed the vararg parameter from _PyArg_UnpackKeywordsWithVararg().
  • Moved creation of the tuple for var-positional parameter out from _PyArg_UnpackKeywordsWithVararg().
  • Refactored Argument Clinic code: it now generates similar code for var-positional parameter in functions with and without keyword parameters. The generated code is now more optimal and more cases are now supported. This is a large step.
  • Finally, _PyArg_UnpackKeywordsWithVararg() was merged with _PyArg_UnpackKeywords() which now got a new flag.

Linked PRs
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Aug 12, 2024
@serhiy-storchaka
pythongh-122943: Rework support of var-positional parameter in Argume…
172352f 
…nt Clinic

Move creation of a tuple for var-positional parameter out of
_PyArg_UnpackKeywordsWithVararg().
Merge _PyArg_UnpackKeywordsWithVararg() with _PyArg_UnpackKeywords().
Add a new parameter in _PyArg_UnpackKeywords().

The "parameters" and "converters" attributes of ParseArgsCodeGen no
longer contain the var-positional parameter. It is now available as the
"varpos" attribute. Optimize code generation for var-positional
parameter and reuse the same generating code for functions with and without
keyword parameters.
@bedevere-app bedevere-app bot mentioned this issue Aug 12, 2024
Merged
@serhiy-storchaka
Copy link
Member Author

Changes in most of Argument Clinic generated files (which correspond to unchanged source files) can be ignored. Sorry, all these review requests are false.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Aug 12, 2024
@serhiy-storchaka
pythongh-122943: Rework support of var-positional parameter in Argume…
a7c627e 
…nt Clinic

Move creation of a tuple for var-positional parameter out of
_PyArg_UnpackKeywordsWithVararg().
Merge _PyArg_UnpackKeywordsWithVararg() with _PyArg_UnpackKeywords().
Add a new parameter in _PyArg_UnpackKeywords().

The "parameters" and "converters" attributes of ParseArgsCodeGen no
longer contain the var-positional parameter. It is now available as the
"varpos" attribute. Optimize code generation for var-positional
parameter and reuse the same generating code for functions with and without
keyword parameters.
serhiy-storchaka added a commit that referenced this issue Nov 7, 2024
@serhiy-storchaka
gh-122943: Rework support of var-positional parameter in Argument Cli…
1f77739 
…nic (GH-122945)

Move creation of a tuple for var-positional parameter out of
_PyArg_UnpackKeywordsWithVararg().
Merge _PyArg_UnpackKeywordsWithVararg() with _PyArg_UnpackKeywords().
Add a new parameter in _PyArg_UnpackKeywords().

The "parameters" and "converters" attributes of ParseArgsCodeGen no
longer contain the var-positional parameter. It is now available as the
"varpos" attribute. Optimize code generation for var-positional
parameter and reuse the same generating code for functions with and without
keyword parameters.

Add special converters for var-positional parameter. "tuple" represents it as
a Python tuple and "array" represents it as a continuous array of PyObject*.
"object" is a temporary alias of "tuple".
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Nov 7, 2024
@serhiy-storchaka
pythongh-122943: Remove the object converter for var-positional param…
7230dbb 
…eter
@bedevere-app bedevere-app bot mentioned this issue Nov 7, 2024
Merged
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Nov 7, 2024
@serhiy-storchaka
pythongh-122943: Remove the object converter for var-positional param…
19cf74e 
…eter
serhiy-storchaka added a commit that referenced this issue Nov 8, 2024
@serhiy-storchaka
gh-122943: Remove the object converter for var-positional parameter (G…
06a8b0b 
…H-126560)
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Nov 8, 2024
@serhiy-storchaka
pythongh-122943: Add the varpos parameter in _PyArg_UnpackKeywords
0d808e4 
Remove _PyArg_UnpackKeywordsWithVararg.
Add comments for integer arguments of _PyArg_UnpackKeywords.
@bedevere-app bedevere-app bot mentioned this issue Nov 8, 2024
Merged
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Nov 8, 2024
@serhiy-storchaka
pythongh-122943: Add the varpos parameter in _PyArg_UnpackKeywords
a511bc2 
Remove _PyArg_UnpackKeywordsWithVararg.
Add comments for integer arguments of _PyArg_UnpackKeywords.
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Nov 8, 2024
@serhiy-storchaka
pythongh-122943: Move code generation for var-positional parameter to…
d5d0134 
… converters
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Nov 8, 2024
@serhiy-storchaka
pythongh-122943: Move code generation for var-positional parameter to…
6f31305 
… converters
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Nov 8, 2024
@serhiy-storchaka
pythongh-122943: Move code generation for var-positional parameter to…
c45d4aa 
… converters
@bedevere-app bedevere-app bot mentioned this issue Nov 8, 2024
Merged
serhiy-storchaka added a commit that referenced this issue Nov 8, 2024
@serhiy-storchaka
gh-122943: Move code generation for var-positional parameter to conve…
ee0746a 
…rters (GH-126575)
serhiy-storchaka added a commit that referenced this issue Nov 8, 2024
@serhiy-storchaka
gh-122943: Add the varpos parameter in _PyArg_UnpackKeywords (GH-126564)
061e50f 
Remove _PyArg_UnpackKeywordsWithVararg.
Add comments for integer arguments of _PyArg_UnpackKeywords.
picnixz pushed a commit to picnixz/cpython that referenced this issue Dec 8, 2024
@serhiy-storchaka @picnixz
pythongh-122943: Rework support of var-positional parameter in Argume…
52441bf 
…nt Clinic (pythonGH-122945)

Move creation of a tuple for var-positional parameter out of
_PyArg_UnpackKeywordsWithVararg().
Merge _PyArg_UnpackKeywordsWithVararg() with _PyArg_UnpackKeywords().
Add a new parameter in _PyArg_UnpackKeywords().

The "parameters" and "converters" attributes of ParseArgsCodeGen no
longer contain the var-positional parameter. It is now available as the
"varpos" attribute. Optimize code generation for var-positional
parameter and reuse the same generating code for functions with and without
keyword parameters.

Add special converters for var-positional parameter. "tuple" represents it as
a Python tuple and "array" represents it as a continuous array of PyObject*.
"object" is a temporary alias of "tuple".
picnixz pushed a commit to picnixz/cpython that referenced this issue Dec 8, 2024
@serhiy-storchaka @picnixz
pythongh-122943: Remove the object converter for var-positional param…
edc9d23 
…eter (pythonGH-126560)
picnixz pushed a commit to picnixz/cpython that referenced this issue Dec 8, 2024
@serhiy-storchaka @picnixz
pythongh-122943: Move code generation for var-positional parameter to…
3617f36 
… converters (pythonGH-126575)
picnixz pushed a commit to picnixz/cpython that referenced this issue Dec 8, 2024
@serhiy-storchaka @picnixz
pythongh-122943: Add the varpos parameter in _PyArg_UnpackKeywords (p…
8e58fd4 
…ythonGH-126564)

Remove _PyArg_UnpackKeywordsWithVararg.
Add comments for integer arguments of _PyArg_UnpackKeywords.
ebonnal pushed a commit to ebonnal/cpython that referenced this issue Jan 12, 2025
@serhiy-storchaka @ebonnal
pythongh-122943: Rework support of var-positional parameter in Argume…
8051436 
…nt Clinic (pythonGH-122945)

Move creation of a tuple for var-positional parameter out of
_PyArg_UnpackKeywordsWithVararg().
Merge _PyArg_UnpackKeywordsWithVararg() with _PyArg_UnpackKeywords().
Add a new parameter in _PyArg_UnpackKeywords().

The "parameters" and "converters" attributes of ParseArgsCodeGen no
longer contain the var-positional parameter. It is now available as the
"varpos" attribute. Optimize code generation for var-positional
parameter and reuse the same generating code for functions with and without
keyword parameters.

Add special converters for var-positional parameter. "tuple" represents it as
a Python tuple and "array" represents it as a continuous array of PyObject*.
"object" is a temporary alias of "tuple".
ebonnal pushed a commit to ebonnal/cpython that referenced this issue Jan 12, 2025
@serhiy-storchaka @ebonnal
pythongh-122943: Remove the object converter for var-positional param…
0715b23 
…eter (pythonGH-126560)
ebonnal pushed a commit to ebonnal/cpython that referenced this issue Jan 12, 2025
@serhiy-storchaka @ebonnal
pythongh-122943: Move code generation for var-positional parameter to…
f5efe23 
… converters (pythonGH-126575)
ebonnal pushed a commit to ebonnal/cpython that referenced this issue Jan 12, 2025
@serhiy-storchaka @ebonnal
pythongh-122943: Add the varpos parameter in _PyArg_UnpackKeywords (p…
d1c045e 
…ythonGH-126564)

Remove _PyArg_UnpackKeywordsWithVararg.
Add comments for integer arguments of _PyArg_UnpackKeywords.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic-argument-clinic
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@serhiy-storchaka