X Tutup
Skip to content

postpone <stdbool.h> inclusion after Python.h#130641

Merged
vstinner merged 1 commit intopython:mainfrom
chouquette:delay_stdbool_inclusion
Feb 28, 2025
Merged

postpone <stdbool.h> inclusion after Python.h#130641
vstinner merged 1 commit intopython:mainfrom
chouquette:delay_stdbool_inclusion

Conversation

@chouquette
Copy link
Contributor

@chouquette chouquette commented Feb 27, 2025

stdbool.h will cause <features.h> to be included before Python.h can define some macros to enable some additional features, causing multiple types not to be defined down the line

In my specific case, this was causing build failures when building python with zig cc

@chouquette
remove inclusions prior to Python.h
7169699 
stdbool.h will cause <features.h> to be included before Python.h can
define some macros to enable some additional features, causing multiple
types not to be defined down the line
@ghost
Copy link

ghost commented Feb 27, 2025 •
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Feb 27, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

vstinner
vstinner approved these changes Feb 28, 2025
View reviewed changes
Copy link
Member

@vstinner 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

@vstinner vstinner merged commit 830f04b into python:main Feb 28, 2025
51 checks passed
@vstinner vstinner added the needs backport to 3.13 bugs and security fixes label Feb 28, 2025
@miss-islington-app
Copy link

miss-islington-app bot commented Feb 28, 2025

Thanks @chouquette for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

miss-islington-app bot commented Feb 28, 2025

Sorry, @chouquette and @vstinner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 830f04b5056db92ba96387db0a778dcd19a39522 3.13

@bedevere-app
Copy link

bedevere-app bot commented Feb 28, 2025

GH-130675 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Feb 28, 2025
vstinner pushed a commit to vstinner/cpython that referenced this pull request Feb 28, 2025
@chouquette @vstinner
Postpone <stdbool.h> inclusion after Python.h (python#130641)
83bfd34 
Remove inclusions prior to Python.h.

<stdbool.h> will cause <features.h> to be included before Python.h can
define some macros to enable some additional features, causing multiple
types not to be defined down the line.

(cherry picked from commit 830f04b)
vstinner added a commit that referenced this pull request Feb 28, 2025
@vstinner @chouquette
[3.13] Postpone <stdbool.h> inclusion after Python.h (#130641) (#130675)
9184b1b 
Postpone <stdbool.h> inclusion after Python.h (#130641)

Remove inclusions prior to Python.h.

<stdbool.h> will cause <features.h> to be included before Python.h can
define some macros to enable some additional features, causing multiple
types not to be defined down the line.

(cherry picked from commit 830f04b)

Co-authored-by: Hugo Beauzée-Luyssen <hugo@beauzee.fr>
@colesbury
Copy link
Contributor

colesbury commented Mar 1, 2025

I noticed we still have a bunch of places where we #include <stdbool.h> first. When is this a problem and when is it not?

cpython/Python/flowgraph.c

Lines 1 to 4 in ddc27f9

#include <stdbool.h>
#include "Python.h"

cpython/Modules/_hashopenssl.c

Lines 25 to 26 in ddc27f9

#include <stdbool.h>
#include "Python.h"

cpython/Parser/string_parser.c

Lines 1 to 3 in ddc27f9

#include <stdbool.h>
#include <Python.h>

cpython/Python/codegen.c

Lines 15 to 17 in ddc27f9

#include <stdbool.h>
#include "Python.h"

cpython/Python/index_pool.c

Lines 1 to 3 in ddc27f9

#include <stdbool.h>
#include "Python.h"

cpython/Python/instruction_sequence.c

Lines 8 to 10 in ddc27f9

#include <stdbool.h>
#include "Python.h"

@chouquette
Copy link
Contributor Author

chouquette commented Mar 1, 2025

Disclaimer: I don't know the python code base at all

My take on this is that it seems Python.h is meant to be included as the first header for any source, similar to how config.h usually is for most autoconf based projects.
If that's the case, not doing so is likely to break in various "fun" ways depending on the libc implementation, so I'd say there never a good reason not to include Python.h first.
I only patched the errors I encountered but I can open a follow up PR

@chouquette chouquette deleted the delay_stdbool_inclusion branch March 1, 2025 17:03
@chouquette chouquette mentioned this pull request Mar 1, 2025
Merged
@picnixz picnixz mentioned this pull request Mar 1, 2025
Closed
@vstinner
Copy link
Member

vstinner commented Mar 3, 2025

Follow-up: PR #130738.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently 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

Assignees

@vstinner vstinner

Labels

skip issue skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chouquette @colesbury @vstinner
X Tutup