FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-131238: Add explicit include to pycore headers by vstinner · Pull Request #131257 · python/cpython · GitHub

/ cpython Public

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

Merged
vstinner merged 1 commit into
python:mainfrom
vstinner:pycore_deps
Mar 17, 2025
ConversationCommits (1)ChecksFiles changed
Merged

gh-131238: Add explicit include to pycore headers#131257
vstinner merged 1 commit into
python:mainfrom
vstinner:pycore_deps

Conversation

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

Copy link
Copy Markdown
Member

bedevere-app Bot mentioned this pull request Mar 14, 2025
Closed
vstinner marked this pull request as draft March 14, 2025 19:39
colesbury reviewed Mar 14, 2025
View reviewed changes
Comment thread 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()

colesbury Mar 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

_Py_ID is defined in pycore_global_strings.h

colesbury Mar 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

vstinner Mar 15, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

gpshead approved these changes Mar 15, 2025
View reviewed changes

gpshead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 commented Mar 17, 2025

Copy link
Copy Markdown
Member Author

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

vstinner marked this pull request as ready for review March 17, 2025 11:32
vstinner merged commit 978e37b into python:main Mar 17, 2025
vstinner deleted the pycore_deps branch March 17, 2025 11:32

vstinner commented Mar 17, 2025

Copy link
Copy Markdown
Member Author

I wrote this PR to prepare #131356.

vstinner mentioned this pull request Mar 17, 2025
Merged
plashchynski pushed a commit to plashchynski/cpython that referenced this pull request Mar 17, 2025
pythongh-131238: Add explicit includes to pycore headers (python#131257)
3ffc4ac
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

colesbury colesbury left review comments
gpshead gpshead approved these changes
JelleZijlstra JelleZijlstra approved these changes
isidentical Awaiting requested review from isidentical isidentical is a code owner
Eclips4 Awaiting requested review from Eclips4 Eclips4 is a code owner
pablogsal Awaiting requested review from pablogsal pablogsal is a code owner
lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
carljm Awaiting requested review from carljm carljm is a code owner
markshannon Awaiting requested review from markshannon markshannon is a code owner
iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner
brandtbucher Awaiting requested review from brandtbucher
savannahostrowski Awaiting requested review from savannahostrowski
brettcannon Awaiting requested review from brettcannon brettcannon is a code owner
ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner
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

Back | FazBrowse Home | New Git URL