Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-122943: Add the varpos parameter in _PyArg_UnpackKeywords#126564
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.
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
gh-122943: Add the varpos parameter in _PyArg_UnpackKeywords #126564
Uh oh!
There was an error while loading. Please reload this page.
Conversation
serhiy-storchaka commented Nov 8, 2024 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
Remove _PyArg_UnpackKeywordsWithVararg. Add comments for integer arguments of _PyArg_UnpackKeywords.
0d808e4 to a511bc2Compare
erlend-aasland left a comment
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.
Neat. Consider this style amendment for IMO slightly more readable generated code:
diff
diff --git a/Tools/clinic/libclinic/parse_args.py b/Tools/clinic/libclinic/parse_args.py index 8b7afbcf3e4..8d1714b1ebe 100644 --- a/Tools/clinic/libclinic/parse_args.py+++ b/Tools/clinic/libclinic/parse_args.py@@ -695,13 +695,21 @@ def parse_general(self, clang: CLanguage) -> None: if has_optional_kw: self.declarations += "\nPy_ssize_t noptargs = %s + (kwargs ? PyDict_GET_SIZE(kwargs) : 0) - %d;" % (nargs, self.min_pos + self.min_kw_only) unpack_args = '_PyTuple_CAST(args)->ob_item, nargs, kwargs, NULL' - parser_code = [libclinic.normalize_snippet(f"""-{argsname} = _PyArg_UnpackKeywords({unpack_args}, &_parser,- /*minpos*/{self.min_pos}, /*maxpos*/{self.max_pos}, /*minkw*/{self.min_kw_only}, /*varpos*/{1 if self.varpos else 0}, argsbuf);+ snippet = f"""+{argsname} = _PyArg_UnpackKeywords(+{unpack_args}, &_parser,+ /*minpos*/{self.min_pos},+ /*maxpos*/{self.max_pos},+ /*minkw*/{self.min_kw_only},+ /*varpos*/{1 if self.varpos else 0},+ argsbuf+ ); if (!{argsname}){{{{goto exit}}}} - """, indent=4)]+ """+ snippet = libclinic.normalize_snippet(snippet, indent=4)+ parser_code = [snippet] if self.requires_defining_class: self.flags = 'METH_METHOD|' + self.flagsskirpichev commented Nov 8, 2024
(This is why auto-formatting for clinic output (with indent-like tool) might be not a bad idea.) But if we are going for multi-line code snippet, lets add also spaces to comments: |
erlend-aasland commented Nov 8, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
We already do auto-formatting of clinic output1; see Footnotes
|
serhiy-storchaka commented Nov 8, 2024
It would be less readable to me. I have vision problems so I get lost in very scattered code.
I initially wrote it as |
…ythonGH-126564) Remove _PyArg_UnpackKeywordsWithVararg. Add comments for integer arguments of _PyArg_UnpackKeywords.
…ythonGH-126564) Remove _PyArg_UnpackKeywordsWithVararg. Add comments for integer arguments of _PyArg_UnpackKeywords.
Remove _PyArg_UnpackKeywordsWithVararg.
Add comments for integer arguments of _PyArg_UnpackKeywords.
This is a follow up of #122945. It was not included in #122945 to minimize the diff.