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-45459: Add pytypedefs.h header file by vstinner · Pull Request #31527 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Feb 23, 2022 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Move forward declarations of Python C API types to a new pytypedefs.h
header file to solve interdependency issues between header files.

pytypedefs.h contains forward declarations of the following types:

  • PyCodeObject
  • PyFrameObject
  • PyGetSetDef
  • PyInterpreterState
  • PyLongObject
  • PyMemberDef
  • PyMethodDef
  • PyModuleDef
  • PyObject
  • PyThreadState
  • PyTypeObject

https://bugs.python.org/issue45459

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 Feb 23, 2022
@vstinner
vstinner requested a review from a team as a code owner February 23, 2022 14:12
@bedevere-bot bedevere-bot added the awaiting core review label Feb 23, 2022
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 23, 2022
@vstinner
bpo-45459: Add pytypedefs.h header file
9ff68e1
Move forward declarations of Python C API types to a new pytypedefs.h
header file to solve interdependency issues between header files.

pytypedefs.h contains forward declarations of the following types:

* PyCodeObject
* PyFrameObject
* PyGetSetDef
* PyInterpreterState
* PyLongObject
* PyMemberDef
* PyMethodDef
* PyModuleDef
* PyObject
* PyThreadState
* PyTypeObject
@vstinner

vstinner commented Feb 23, 2022

Copy link
Copy Markdown
Member Author

Last years, more and more interdependencies have been added. Examples:

  • pybuffer.h uses PyObject before it's defined by object.h (included later).
  • object.h must forward declare PyModuleDef since it's used before it's defined by moduleobject.h (included after object.h).
  • object.h uses struct PyMethodDef which is defined later by methodobject.h (included after object.h). object.h has to use struct PyMethodDef instead of PyMethodDef to solve the interdpendency issue.

This PR cleans up header files by moving forward declarations at the top of Python.h.

With this PR, it becomes possible to use type names like PyObject or PyModuleDef rather than using structure names like struct _object or struct PyModuleDef. I didn't make this change in this PR to keep it easier to review. I created GH-31528 draft PR (see the second commit of this PR) to use type names.

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented Feb 23, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

cc @benjaminp @erlend-aasland @corona10

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented Feb 23, 2022

Copy link
Copy Markdown
Member Author

If you don't want to go this way, I wrote the bare minimum fix for the clang compiler warning: GH-31539.

But as I wrote in my previous comment, IMO it's now time to cleanup these forward declarations to have a sane way to fix this issuse in a single place. There are too many scattered forward declarations in the Python C API.

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented Feb 23, 2022

Copy link
Copy Markdown
Member Author

If you don't want to go this way, I wrote the bare minimum fix for the clang compiler warning: #31539.

Defining types in multiple header files is tricky. At my first attempt, I defined PyObject inside #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030b0000 and it didn't work when building xxlimited35 which targets the limited C API version 3.5...

All reactions

Sorry, something went wrong.

Uh oh!

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

corona10
corona10 approved these changes Feb 24, 2022
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Feb 24, 2022
@vstinner
vstinner merged commit ec091bd into python:main Feb 24, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Feb 24, 2022
@vstinner
vstinner deleted the pytypedefs branch February 24, 2022 16:07
@vstinner vstinner mentioned this pull request Feb 24, 2022
bpo-45459: Fix object.h: don't define PyObject type twice #31539
Closed
@erlend-aasland

erlend-aasland commented Feb 24, 2022

Copy link
Copy Markdown
Contributor

This looks nice and clean. AFAICS, there is no downside; approved post-merge :)

All reactions

Sorry, something went wrong.

Uh oh!

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

asvetlov pushed a commit that referenced this pull request Feb 26, 2022
@vstinner @asvetlov
bpo-45459: Add pytypedefs.h header file (GH-31527)
d285118
Move forward declarations of Python C API types to a new pytypedefs.h
header file to solve interdependency issues between header files.

pytypedefs.h contains forward declarations of the following types:

* PyCodeObject
* PyFrameObject
* PyGetSetDef
* PyInterpreterState
* PyLongObject
* PyMemberDef
* PyMethodDef
* PyModuleDef
* PyObject
* PyThreadState
* PyTypeObject
itamaro added a commit to itamaro/cpython that referenced this pull request May 24, 2022
@itamaro
Comment nit: fix comment reference in pycore_interp.h
249a207
pythonGH-31527 moved this typedef to `Include/pytypedefs.h`, so this comment should point at the correct location
@itamaro itamaro mentioned this pull request May 24, 2022
Comment nit: fix comment reference in pycore_interp.h #93188
Merged
vstinner pushed a commit that referenced this pull request Jul 9, 2022
@itamaro
pycore_interp.h: Fix comment about pytypedefs.h (#93188)
0fc8ac0
GH-31527 moved this typedef to `Include/pytypedefs.h`, so this comment
should point at the correct location
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@corona10 corona10 corona10 approved these changes

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.

5 participants

@vstinner @erlend-aasland @corona10 @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.