-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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-85283: Extending Argument Clinic to support to use the limited C API. #26080
Conversation
The test of Argument Clinic in: https://github.com/shihai1991/cpython/compare/main...shihai1991:clinic_support_limited_capi?expand=1 |
@larryhastings Hi, Larry. Would you mind to take a look? cc @vstinner |
Before I look at the patch: what is the purpose of it? Argument Clinic is an internal tool for CPython, it can use the full API. |
Yes. The Argument Clinic can use the full API. So this PR add an option to allow developer to use the limited C API or not. More details in: |
This PR is stale because it has been open for 30 days with no activity. |
@@ -887,7 +895,7 @@ def parser_body(prototype, *fields, declarations=''): | |||
""", indent=4)] | |||
else: | |||
parser_code = [normalize_snippet(""" | |||
if (!PyArg_ParseTuple(args, "{format_units}:{name}", | |||
if (!_PyArg_ParseTupleAndKeywordsFast(args, kwargs, &_parser, |
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.
Why?
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.
Use the limited C API when using the use_limited_api flag, otherwise it'll use the unlimited C API.
Sorry for my delay.
else: | ||
declarations = ( | ||
'static const char * const _keywords[] = {{{keywords} NULL}};\n' | ||
'static _PyArg_Parser _parser = {{"{format_units}:{name}", _keywords, 0}};') |
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.
This is not in the Limited API.
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.
OK, you are right. Thanks.
After PEP 620 canceled, so I decide close this PR. I will reopen this PR when we continue update PEP 620, ref: python/peps#2923 |
@vstinner would of course know best, but my impression of the intent of withdrawing PEP 620 was that neither would it be re-opened and updated later or nor would the remaining changes be abandoned (as most of them were completed or mostly completed already), but rather they would be broken up into smaller separate PEPs. |
@shihai1991, are you interested in reviving this effort? |
Sure, of course. Sorry for my late replay ;) |
https://bugs.python.org/issue41111