NotificationsYou must be signed in to change notification settings
Fork33.3k
Star69.7k

[3.12] gh-130740: Move some`stdbool.h` includes after`Python.h` (#130738)#130757

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.

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

Already on GitHub?Sign in to your account

Jump to bottom

Merged

picnixz merged 3 commits intopython:3.12frompicnixz:bp/312/214562ed4ddc248b007f718ed92ebcc0c3669611-130740

Mar 4, 2025

Merged

[3.12] gh-130740: Move some`stdbool.h` includes after`Python.h` (#130738)#130757

picnixz merged 3 commits intopython:3.12frompicnixz:bp/312/214562ed4ddc248b007f718ed92ebcc0c3669611-130740

Mar 4, 2025

Conversation

Copy link

Member

picnixz commentedMar 2, 2025•
edited by bedevere-appbot
Loading

Issue:Ensurestdbool.h is included afterPython.h #130740

chouquetteand others added2 commits

March 2, 2025 11:05

pythongh-130740: Move somestdbool.hincludes afterPython.h(pyt…

6486103

…hon#130738)Move some `#include <stdbool.h>` after `#include "Python.h"` when `pyconfig.h` is notincluded first and when we are in a platform-agnostic context. This is to avoid havingfeatures defined by `stdbool.h` before those decided by `Python.h`.

backport

6a3dfb7

This was referencedMar 2, 2025

gh-130740: Move somestdbool.h includes afterPython.h#130738

Merged

Ensurestdbool.h is included afterPython.h#130740

Closed

include stdbool.h in opcode_metadata.h

efd75da

picnixz marked this pull request as ready for review

March 2, 2025 10:38

picnixz requested review fromiritkatriel,lysnikolaou,markshannon andpablogsal ascode owners

March 2, 2025 10:38

bedevere-appbot added the awaiting core review label

Mar 2, 2025

Copy link

MemberAuthor

Choose a reason for hiding this comment

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

Why do you add <stdbool.h> include?

Copy link

MemberAuthor

picnixzMar 3, 2025•
edited
Loading

Choose a reason for hiding this comment

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

Because there are somebool inside thegenerate_metadata.h file which, without this include, doesn't compile. Previously, it was because we included stdbool.h before including this file so it was impliticitly available (seehttps://github.com/python/cpython/actions/runs/13614386994/job/38055588576?pr=130757)

Copy link

Member

markshannon commentedMar 3, 2025

The change toTools/cases_generator/generate_cases.py seems fine. Harmless at worst.

Copy link

Member

markshannon commentedMar 3, 2025

Don't the generated files need to be regenerated?

Copy link

MemberAuthor

picnixz commentedMar 3, 2025

Don't the generated files need to be regenerated?

I should have regenerated them?efd75da#diff-f862bc60824529a25ba378be850ecc6762f03c6c944266d30ac7172d10132498. Or are there more rules that I needed to run?

Copy link

Member

vstinner commentedMar 3, 2025

I downloaded the PR and ranmake regen-all: there is no change.

vstinner approved these changes

Mar 3, 2025

View reviewed changes

Copy link

Member

vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

bedevere-appbot added awaiting merge and removed awaiting core review labels

Mar 3, 2025

Copy link

MemberAuthor

picnixz commentedMar 3, 2025•
edited
Loading

make regen-all

I think some of the targets need to be regenerated separately (this may include the regen-cases but I don't remember well) The other files that could have been regenerated aregenerated_cases.c.h but I'm not changing their output as they don't have a "bool" (or at least my compiler didn't complain about it). What really mattered wasopcode_metadata.h.

EDIT: regen-all was sufficient, my bad!