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
vstinner merged 1 commit into from
Mar 17, 2025
Merged

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

vstinner merged 1 commit into from
Mar 17, 2025

Conversation

@vstinner
Copy link
Member

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

@bedevere-appbedevere-appbot mentioned this pull request Mar 14, 2025
Closed
@vstinnervstinner 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

@colesburycolesburyMar 14, 2025

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

@colesburycolesburyMar 14, 2025

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
MemberAuthor

@vstinnervstinnerMar 15, 2025

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

@gpsheadgpshead 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
MemberAuthor

vstinner commented Mar 17, 2025

PR rewritten on top of commit a1aeec6:

commit a1aeec61c4321ba9a6966109343bd88dcf9cb26a Author: Mark Shannon <mark@hotpy.org> 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

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

vstinner commented Mar 17, 2025

I wrote this PR to prepare #131356.

@vstinnervstinner 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
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@colesburycolesburycolesbury left review comments

@gpsheadgpsheadgpshead approved these changes

@JelleZijlstraJelleZijlstraJelleZijlstra approved these changes

@isidenticalisidenticalAwaiting requested review from isidenticalisidentical is a code owner

@Eclips4Eclips4Awaiting requested review from Eclips4Eclips4 is a code owner

@pablogsalpablogsalAwaiting requested review from pablogsalpablogsal is a code owner

@lysnikolaoulysnikolaouAwaiting requested review from lysnikolaoulysnikolaou is a code owner

@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner

@carljmcarljmAwaiting requested review from carljmcarljm is a code owner

@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner

@iritkatrieliritkatrielAwaiting requested review from iritkatrieliritkatriel is a code owner

@brandtbucherbrandtbucherAwaiting requested review from brandtbucher

@savannahostrowskisavannahostrowskiAwaiting requested review from savannahostrowski

@brettcannonbrettcannonAwaiting requested review from brettcannonbrettcannon is a code owner

@ncoghlanncoghlanAwaiting requested review from ncoghlanncoghlan is a code owner

@warsawwarsawAwaiting requested review from warsawwarsaw 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