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 — bpo-47115: Document which parts of structs are in limited API/stable ABI by encukou · Pull Request #32196 · python/cpython

Conversation

@encukou

@encukou encukou commented Mar 30, 2022 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Some screenshots of the result:

Opaque struct:
image
Fully visible structs:
image
image
Partial struct (see the issue for details):
image
Included member of the above:
image

Non-struct items don't change:
image

https://bugs.python.org/issue47115

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
encukou added 3 commits March 30, 2022 15:46
@encukou
stable_abi.txt: Fix several function types that were listed as structs
655bfe5
@encukou
Document which parts of structs are in limited API/stable ABI
9f4c764
@encukou
Add a blurb
8c8a03d
@encukou
encukou requested review from erlend-aasland and vstinner March 30, 2022 15:19
@bedevere-bot bedevere-bot added the awaiting core review label Mar 30, 2022
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Mar 30, 2022
@encukou encukou changed the title Document which parts of structs are in limited API/stable ABI bpo-47115: Document which parts of structs are in limited API/stable ABI Mar 30, 2022
@arhadthedev

arhadthedev commented Mar 30, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

A side question since stable_abi.dat is rewritten anyway: can it be renamed into stable_abi.csv (in a separate PR, for sure) so (1) GitHub will format it as a pretty table, (2) spreadsheet editors like OpenOffice Calc and Microsoft Excel could be used? Or there are third party tools that use it so the renaming will break them?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner reviewed Mar 30, 2022
View reviewed changes
Comment thread Misc/stable_abi.txt
opaque
struct PyFrameObject
added 3.2
opaque

@vstinner vstinner Mar 30, 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.

By the way, I recently added "The structure is not part of the C API." to:
https://docs.python.org/dev/c-api/frame.html

Sorry, something went wrong.

Uh oh!

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

@encukou encukou Mar 30, 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.

That's wrong. If it's not part of the API at all, you can't use functions like PyFrame_GetBack that take it as an argument.

Sorry, something went wrong.

Uh oh!

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

@vstinner vstinner Mar 31, 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.

It seems like there is a misunderstanding on what "a structure" means here. Python C API is misleading because it uses short PyFrameObject type name rather than struct PyFrameObject.

In practice in C, "PyFrameObject" is a type name, not a structure. I recently added Include/pytypedefs.h to clarify which type names are always available in the C API (and fix a compiler error), even if their related structures are opaque. I understand that the type (type "name") is part of the C API, but the structure is not.

I'm not sure how https://docs.python.org/dev/c-api/frame.html should be rephrased. The follow this PR, maybe "The structure is opaque" which may become redundant once this PR is merged.

Sorry, something went wrong.

Uh oh!

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

@encukou encukou Apr 1, 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.

Ah, I see.
This should be explained/avoided: if you don't know the term "opaque structure" you are likely to search for it, but if you don't realize the distinction between "type" and "structure", you'll probably just be confused.

I'll work on the rephrasing next week. Please let me know if you know of other places with similar wording.

Sorry, something went wrong.

Uh oh!

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

@encukou encukou Apr 1, 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 started going through all the structs to make sure the docs make sense.
And I quite like the note for PyInterpreterState: “There are no public members in this structure.” Short and clear! I'll use that for PyFrameObject too.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@vstinner

vstinner commented Mar 30, 2022

Copy link
Copy Markdown
Member

Is there a definition of an "opaque structure" in the Python doc? Is there a link from "opaque struct" in then generated doc?
All reactions

Sorry, something went wrong.

Uh oh!

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

encukou commented Mar 30, 2022

Copy link
Copy Markdown
Member Author

Is there a definition of an "opaque structure" in the Python doc? Is there a link from "opaque struct" in then generated doc?

IMO it's a generally known term, it even has a Wikipedia entry. I don't think the docs need to define it.
erlend-aasland 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.
@encukou

encukou commented Mar 30, 2022

Copy link
Copy Markdown
Member Author

@arhadthedev, I'll open an issue for it.
All reactions

Sorry, something went wrong.

Uh oh!

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

encukou commented Mar 30, 2022

Copy link
Copy Markdown
Member Author

Issue for the renaming (and more): https://bugs.python.org/issue47168
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland
erlend-aasland reviewed Mar 30, 2022
View reviewed changes

@erlend-aasland erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

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.

Nice; I think this doc enhancement will be helpful for both core devs and others.

From what I could see, the Py changes look sensible, but I'm not at all familiar with these doc tools. I'll have another look at this tomorrow.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Misc/stable_abi.txt Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/stable_abi.txt Resolved

Uh oh!

There was an error while loading. Please reload this page.
encukou and others added 3 commits April 1, 2022 17:23
@encukou Erlend Egeberg Aasland
Apply suggestions from code review
efb7cd6 
Co-authored-by: Erlend Egeberg Aasland <[email protected]>
@encukou
Use “members” as the term for struct fields
89826aa
@encukou
Adjust wording of the PyFrameObject docs
4b188ae
@encukou

encukou commented Apr 1, 2022

Copy link
Copy Markdown
Member Author

So, how about this?
image
(The “structure of the objects used to describe frame objects” is weird, but I'm leaving that out of scope of this PR.)
erlend-aasland 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 commented Apr 1, 2022

Copy link
Copy Markdown
Member

“There are no public members in this structure.”

That's explicit. It prevents the confusion about "the structure is not part of the C API" and confusion between "structure" and "type". In practice, an opaque structure ... is an empty structure, right ;-) The only subtle difference is that sizeof() doesn't work on an opaque structure, but I don't think that it's worth it to do into the details, unless we add an entry to the glossary for "opaque structure".
encukou 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 approved these changes Apr 1, 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 Apr 1, 2022
erlend-aasland
erlend-aasland approved these changes Apr 1, 2022
View reviewed changes

@erlend-aasland erlend-aasland left a comment •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

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.

Looks good to me also.

It looks to me that this comment is now outdated:

cpython/Doc/tools/extensions/c_annotations.py

Lines 101 to 103 in 4b188ae

# Stable ABI annotation. These have two forms:
# Part of the [Stable ABI](link).
# Part of the [Stable ABI](link) since version X.Y.

Either update it or delete it :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@encukou
Reword the "Only some members" message and adjust comment
c1ef70a
@encukou

encukou commented Apr 6, 2022

Copy link
Copy Markdown
Member Author

Comment adjusted, and one more tweak to the wording:
image

(It should be possible to make these -- PyObject & PyVarObject -- fully opaque in the 3.12 limited API.)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@encukou
encukou merged commit d79f118 into python:main Apr 6, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Apr 6, 2022
@encukou
encukou deleted the abi3-struct-docs branch April 6, 2022 14:50
@thatbirdguythatuknownot thatbirdguythatuknownot mannequin mentioned this pull request Apr 10, 2022
Documentation inconsistency with the stable ABI #91271
Closed
encukou added a commit to encukou/cpython that referenced this pull request Aug 5, 2022
@encukou Erlend Egeberg Aasland
bpo-47115: Document which parts of structs are in limited API/stable …
e832130 
…ABI (pythonGH-32196)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>
encukou added a commit that referenced this pull request Aug 5, 2022
@encukou Erlend Egeberg Aasland
gh-91271: Document which parts of structs are in limited API/stable A…
b66b6e1 
…BI (GH-32196) (GH-95711)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>
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
@erlend-aasland erlend-aasland erlend-aasland approved these changes

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.

Uh oh!

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

6 participants

@encukou @arhadthedev @vstinner @erlend-aasland @the-knights-who-say-ni @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.