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-92154: Expose PyCode_GetCode in the C API by Fidget-Spinner · Pull Request #92168 · python/cpython

Conversation

@Fidget-Spinner

@Fidget-Spinner Fidget-Spinner commented May 2, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Fixes #92154.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Fidget-Spinner added 2 commits May 3, 2022 01:48
@Fidget-Spinner
Expose PyCode_GetCode
ba149a9
@Fidget-Spinner
Add news
d24ae35
@Fidget-Spinner
Fidget-Spinner requested a review from markshannon as a code owner May 2, 2022 17:50
@bedevere-bot bedevere-bot added the awaiting core review label May 2, 2022
@Fidget-Spinner
Fidget-Spinner requested a review from vstinner May 2, 2022 17:50
Fidget-Spinner
Fidget-Spinner commented May 2, 2022
View reviewed changes
Comment thread Objects/codeobject.c Outdated
PyCode_GetCode(PyObject *co)
{
if (!PyCode_Check(co)) {
PyErr_BadInternalCall();

@Fidget-Spinner Fidget-Spinner May 2, 2022

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.

I forgot if this or a TypeError is preferred for the C API (or no error setting at all?)

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon May 2, 2022

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.

Use PyCode_GetCode(PyCodeObject *co) and then you won't need the check.

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner Fidget-Spinner May 3, 2022

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.

I would like to, but co_code was previously PyObject*, and I want this to be a drop-in replacement as far as possible.

Sorry, something went wrong.

Uh oh!

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

@vstinner vstinner May 3, 2022

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.

PyErr_BadInternalCall() is fine, but I would even suggest replacing a runtime check with an assertion. The caller is responsible to pass the right type.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Fidget-Spinner
Fidget-Spinner requested a review from brandtbucher May 2, 2022 17:54
@Fidget-Spinner

Fidget-Spinner commented May 2, 2022

Copy link
Copy Markdown
Member Author

Also I frankly have no clue if we need to update stable_abi.toml. I'm not sure when it counts as stable ABI and when it doesn't.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Fidget-Spinner added 2 commits May 3, 2022 14:29
@Fidget-Spinner
Add DECREFs and fix compiler warnings
6bc5940
@Fidget-Spinner
fix docs formatting
8fdf0e8
vstinner
vstinner reviewed May 3, 2022
View reviewed changes
Comment thread Doc/c-api/code.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/whatsnew/3.11.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/whatsnew/3.11.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/whatsnew/3.11.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Objects/codeobject.c Outdated
PyCode_GetCode(PyObject *co)
{
if (!PyCode_Check(co)) {
PyErr_BadInternalCall();

@vstinner vstinner May 3, 2022

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.

PyErr_BadInternalCall() is fine, but I would even suggest replacing a runtime check with an assertion. The caller is responsible to pass the right type.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner

vstinner commented May 3, 2022

Copy link
Copy Markdown
Member

Also I frankly have no clue if we need to update stable_abi.toml. I'm not sure when it counts as stable ABI and when it doesn't.

Include/cpython/ is the API excluded from the limited C API and so excluded from the stable ABI: https://devguide.python.org/c-api/

Please don't add this function to the stable ABI yet. Let's wait for one Python release, and then see if it's stable or not.
Fidget-Spinner reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner reviewed May 3, 2022
View reviewed changes
Comment thread Doc/c-api/code.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@vstinner vstinner mentioned this pull request May 3, 2022
Add PyCode_GetCode() function python/pythoncapi-compat#34
Merged
@vstinner

vstinner commented May 3, 2022

Copy link
Copy Markdown
Member

I prepared PR python/pythoncapi-compat#34 to add the function to pythoncapi-compat.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner
Address code review by Victor
87c2bbe
vstinner
vstinner approved these changes May 3, 2022
View reviewed changes

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

LGTM.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels May 3, 2022
@Fidget-Spinner
Fidget-Spinner merged commit 6c7249f into python:main May 3, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label May 3, 2022
@Fidget-Spinner
Fidget-Spinner deleted the pycode_getcode branch May 3, 2022 13:13
@sweeneyde sweeneyde mentioned this pull request Jun 6, 2022
Severe performance degradation for tracing under 3.11 #93516
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner
@brandtbucher brandtbucher Awaiting requested review from brandtbucher

Uh oh!

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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[C-API] Add a function to access PyCodeObject.co_code in C

Uh oh!

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

4 participants

@Fidget-Spinner @vstinner @markshannon @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.