Skip to content

gh-131238: Add explicit include to pycore headers #131257

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 17, 2025
Merged

gh-131238: Add explicit include to pycore headers #131257

merged 1 commit into from
Mar 17, 2025

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Mar 14, 2025 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request Mar 14, 2025
Closed
@vstinner vstinner marked this pull request as draft March 14, 2025 19:39
colesbury
colesbury reviewed Mar 14, 2025
View reviewed changes
Parser/asdl_c.py
#include "pycore_lock.h" // _PyOnceFlag
#include "pycore_modsupport.h" // _PyArg_NoPositional()
#include "pycore_pystate.h" // _PyInterpreterState_GET()
#include "pycore_runtime.h" // _Py_ID()
Copy link
Contributor

Choose a reason for hiding this comment

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

_Py_ID is defined in pycore_global_strings.h

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see your comment on the issue... it needs access to _PyRuntime's internals.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, in practice, pycore_runtime.h should be used.

gpshead
gpshead approved these changes Mar 15, 2025
View reviewed changes
Copy link
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

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

approving as a general concept without trying to understand each one of these directly under the assumption you know what you're doing to try and move towards a less tangled circular mess and anything actually wrong here would fail to build.

@vstinner
Copy link
Member Author

PR rewritten on top of commit a1aeec6:

commit a1aeec61c4321ba9a6966109343bd88dcf9cb26a
Author: Mark Shannon <[email protected]>
Date:   Mon Mar 17 09:19:04 2025 +0000

    GH-131238: Core header refactor (GH-131250)
    
    * Moves most structs in pycore_ header files into pycore_structs.h and pycore_runtime_structs.h
    
    * Removes many cross-header dependencies

@vstinner vstinner marked this pull request as ready for review March 17, 2025 11:32
@vstinner vstinner merged commit 978e37b into python:main Mar 17, 2025
45 checks passed
@vstinner vstinner deleted the pycore_deps branch March 17, 2025 11:32
@vstinner
Copy link
Member Author

I wrote this PR to prepare #131356.

@vstinner vstinner mentioned this pull request Mar 17, 2025
Merged
plashchynski pushed a commit to plashchynski/cpython that referenced this pull request Mar 17, 2025
@vstinner @plashchynski
pythongh-131238: Add explicit includes to pycore headers (python#131257)
3ffc4ac
seehwan pushed a commit to seehwan/cpython that referenced this pull request Apr 16, 2025
@vstinner @seehwan80
pythongh-131238: Add explicit includes to pycore headers (python#131257)
8c6b2df
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@colesbury colesbury colesbury left review comments

@gpshead gpshead gpshead approved these changes

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes

@isidentical isidentical Awaiting requested review from isidentical isidentical is a code owner

@Eclips4 Eclips4 Awaiting requested review from Eclips4 Eclips4 is a code owner

@pablogsal pablogsal Awaiting requested review from pablogsal pablogsal is a code owner

@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

@carljm carljm Awaiting requested review from carljm carljm is a code owner

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

@iritkatriel iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner

@brandtbucher brandtbucher Awaiting requested review from brandtbucher

@savannahostrowski savannahostrowski Awaiting requested review from savannahostrowski

@brettcannon brettcannon Awaiting requested review from brettcannon brettcannon is a code owner

@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner

@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vstinner @gpshead @colesbury @JelleZijlstra