NotificationsYou must be signed in to change notification settings
Fork32.4k
Star67.9k

gh-85283: Extending Argument Clinic to support to use the limited C API.#26080

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

shihai1991 wants to merge1 commit intopython:mainfromshihai1991:bpo_41111_3

Closed

gh-85283: Extending Argument Clinic to support to use the limited C API.#26080

shihai1991 wants to merge1 commit intopython:mainfromshihai1991:bpo_41111_3

Conversation

Copy link

Member

shihai1991 commentedMay 12, 2021•
edited by bedevere-bot
Loading

https://bugs.python.org/issue41111

Issue:[C API] Convert a few stdlib extensions to the limited C API (PEP 384) #85283

Extending Argument Clinic to support to use the limited C API.

e1238ef

bedevere-bot added the awaiting review label

May 12, 2021

the-knights-who-say-ni added the CLA signed label

May 12, 2021

shihai1991 added the skip news label

May 12, 2021

Copy link

MemberAuthor

shihai1991 commentedMay 12, 2021•
edited
Loading

python3 Tools/clinic/clinic.py --limited_api Modules/posixmodule.c

The test of Argument Clinic in:https://github.com/shihai1991/cpython/compare/main...shihai1991:clinic_support_limited_capi?expand=1

Copy link

MemberAuthor

shihai1991 commentedMay 13, 2021

@larryhastings Hi, Larry. Would you mind to take a look? cc@vstinner

shihai1991 requested review fromvstinner andlarryhastings

May 13, 2021 01:09

Copy link

Contributor

larryhastings commentedMay 14, 2021

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.

Copy link

MemberAuthor

shihai1991 commentedMay 14, 2021•
edited
Loading

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.
What's the benefit of using the limited C API? It can hide the implementation details of the C API and more easy to extend.

More details in：
PEP 620:https://www.python.org/dev/peps/pep-0620/#relationship-with-the-limited-c-api
some victor's comment:https://bugs.python.org/issue41111#msg372307

Copy link

github-actionsbot commentedJun 14, 2021

This PR is stale because it has been open for 30 days with no activity.

github-actionsbot added the staleStale PR or inactive for long period of time. label

Jun 14, 2021

This comment was marked as off-topic.

bedevere-bot added awaiting core review and removed awaiting review labels

Mar 24, 2022

serhiy-storchaka added the topic-argument-clinic label

Apr 30, 2022

serhiy-storchaka reviewed

Apr 30, 2022

View reviewed changes

Tools/clinic/clinic.py

		@@ -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,

Copy link

Member

serhiy-storchakaApr 30, 2022

Choose a reason for hiding this comment

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

Why?

Copy link

MemberAuthor

shihai1991Dec 18, 2022

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.

Tools/clinic/clinic.py

		else:
		declarations = (
		'static const char * const _keywords[] = {{{keywords} NULL}};\n'
		'static _PyArg_Parser _parser = {{"{format_units}:{name}", _keywords, 0}};')

Copy link

Member

serhiy-storchakaApr 30, 2022

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.

Copy link

MemberAuthor

shihai1991Dec 18, 2022

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.

ezio-melotti removed the CLA signed label

Jul 13, 2022

github-actionsbot removed the staleStale PR or inactive for long period of time. label

Aug 7, 2022

This was referencedNov 3, 2022

[C API] Convert a few stdlib extensions to the limited C API (PEP 384)#85283

Closed

/std:c++20 instead of /std:c++17 used for _wmimodule.cpp, but seems unnecessary#99191

Closed

shihai1991 changed the title~~bpo-41111: Extending Argument Clinic to support to use the limited C API.~~gh-85283: Extending Argument Clinic to support to use the limited C API.

Dec 18, 2022

Copy link

MemberAuthor

shihai1991 commentedDec 18, 2022

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
Thanks Mike, Serhiy for your review.

shihai1991 closed this

Dec 18, 2022

Copy link

Member

CAM-Gerlach commentedDec 20, 2022

@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.