GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-108494: AC supports pos-only args in limited C API by vstinner · Pull Request #108498 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Aug 25, 2023 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

AC now checks for "#define Py_LIMITED_API" pattern to use the limited C API.

  • Issue: Add support for the limited C API to Argument Clinic #108494

Sorry, something went wrong.

Uh oh!

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

All reactions
@vstinner vstinner added the skip news label Aug 25, 2023
@vstinner
vstinner requested review from AlexWaygood and erlend-aasland as code owners August 25, 2023 22:00
@bedevere-bot bedevere-bot mentioned this pull request Aug 25, 2023
Add support for the limited C API to Argument Clinic #108494
Closed
@bedevere-bot bedevere-bot added the awaiting core review label Aug 25, 2023
@vstinner
pythongh-108494: AC supports pos-only args in limited C API
c38deb5
AC now checks for "#define Py_LIMITED_API" pattern to use the limited
C API.
@vstinner
vstinner force-pushed the ac_pos_limited branch from 8da84e6 to c38deb5 Compare August 25, 2023 22:01
@vstinner
vstinner merged commit 86bc9e3 into python:main Aug 25, 2023
@vstinner
vstinner deleted the ac_pos_limited branch August 25, 2023 22:39
@bedevere-bot bedevere-bot removed the awaiting core review label Aug 25, 2023
@erlend-aasland
erlend-aasland requested a review from serhiy-storchaka August 26, 2023 06:24
serhiy-storchaka
serhiy-storchaka reviewed Aug 26, 2023
View reviewed changes
Comment thread Tools/clinic/clinic.py
{parse_arguments}))
goto exit;
""", indent=4)]
argname_fmt = 'args[%d]'

@serhiy-storchaka serhiy-storchaka Aug 26, 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.

It is not correct. In best case it is unused.

Sorry, something went wrong.

Uh oh!

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

All reactions

@vstinner vstinner Aug 26, 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.

Since AC and parsing arguments are complex, I propose to implement incrementally the limited C API. I'm not sure that I get your comment, when is it unused? Do you want to propose a fix?

Sorry, something went wrong.

Uh oh!

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

All reactions
AlexWaygood
AlexWaygood reviewed Aug 28, 2023
View reviewed changes

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

@vstinner, would you be able to add some documentation for this change to https://docs.python.org/3/howto/clinic.html? It would be useful information for users of Argument Clinic to know that they can generate code that only uses the limited C API by adding #define Py_LIMITED_API to a file

Sorry, something went wrong.

Uh oh!

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

All reactions
@vstinner

vstinner commented Aug 28, 2023

Copy link
Copy Markdown
Member Author

@vstinner, would you be able to add some documentation for this change to https://docs.python.org/3/howto/clinic.html? It would be useful information for users of Argument Clinic to know that they can generate code that only uses the limited C API by adding #define Py_LIMITED_API to a file

I wrote PR #108584 to document the Limited C API feature.

All reactions

Sorry, something went wrong.

Uh oh!

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@AlexWaygood AlexWaygood AlexWaygood left review comments
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland 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.

4 participants

@vstinner @serhiy-storchaka @AlexWaygood @bedevere-bot
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 01402503 — 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.