GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-149044: Fix PySlot_END macro for C++ by vstinner · Pull Request #158866 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Oct 5, 2026 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Initialize all fields in the macro. Otherwise, g++ -Wall -Wextra complains that some fields are not initialized
[-Werror=missing-field-initializers]:

error: missing initializer for member ‘PySlot::sl_flags’
error: missing initializer for member ‘PySlot::’
error: missing initializer for member ‘PySlot::’

  • Issue: Implement PEP 820 – PySlot: Unified slot system for the C API #149044

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
pythongh-149044: Fix PySlot_END macro for C++
47abf28 
Initialize all fields in the macro. Otherwise, g++ -Wall -Wextra
complains that some fields are not initialized
[-Werror=missing-field-initializers]:

  error: missing initializer for member ‘PySlot::sl_flags’
  error: missing initializer for member ‘PySlot::<anonymous>’
  error: missing initializer for member ‘PySlot::<anonymous>’
@vstinner vstinner added topic-C-API needs backport to 3.15 bugs and security fixes labels Oct 5, 2026
@bedevere-app bedevere-app Bot added the type-feature A feature request or enhancement label Oct 5, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Oct 5, 2026
Implement PEP 820 – PySlot: Unified slot system for the C API #149044
Closed
@bedevere-app bedevere-app Bot added the awaiting core review label Oct 5, 2026
@vstinner
Add Changelog
c3671d7
@vstinner

vstinner commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

Note: I found this issue while working on adding support for the limited C API to pythoncapi-compat which builds its C/C++ extension with -Wall -Wextra. See python/pythoncapi-compat#184.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner vstinner removed the type-feature A feature request or enhancement label Oct 5, 2026
@vstinner

vstinner commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

I created this PR to just fix PySlot_END so it can be easily backported.

I also prepared draft PR #158867 which enables -Wall -Wextra in test_cext for the main branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner commented Oct 5, 2026
View reviewed changes
Comment thread Include/slots.h Outdated
{.sl_id=(NAME), .sl_flags=PySlot_STATIC, .sl_ptr=(VALUE)}

#define PySlot_END {0}
#define PySlot_END {0, 0, {0}, {0}}

@vstinner vstinner Oct 5, 2026

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'm not 100% sure that this syntax works on all C/C++ compilers. In C, it's common to use {0}. A more ugly alternative is to have a separated implementation for C++:

#ifdef __cplusplus
#  define PySlot_END {0, 0, {0}, {0}}
#else
#  define PySlot_END {0}
#endif

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@encukou encukou Oct 6, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Maybe use NULL?

Suggested change
#define PySlot_END {0, 0, {0}, {0}}
#define PySlot_END {0, 0, {0}, {NULL}}

Not sure if compilers warn about this, but I heard that C++ is stricter about nullptr↔int casts.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@da-woods da-woods Oct 6, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

You might get away with

#  define PySlot_END {}

for c++ (at least c++11 upwards I think). That's the idiomatic thing for "initialize everything to its default (likely 0) value". But maybe the explicit version with all members is better given it's a fairly short list of members.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
Merge branch 'main' into pyslot_end
905c80b
@bedevere-app bedevere-app Bot added the type-feature A feature request or enhancement label Oct 5, 2026
@vstinner

vstinner commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

I updated the branch to retrieve the MSan fix.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

cc @encukou
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
Initialize PySlot.sl_ptr with _Py_NULL
62b19d8
@vstinner

vstinner commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

@encukou @da-woods: I updated the PR to use _Py_NULL. It should C++ compiler warnings in more cases. Would you mind to review the updated PR?

@da-woods: Using {} is tempting, but I also like the idea of having a single implementation to make it easier to read/maintain. Sadly, I don't think that {} is correct in C (is it?).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@da-woods

da-woods commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Yeah I think a single implementation is probably cleaner. I don't believe {} would be valid C.
vstinner reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
encukou
encukou approved these changes Oct 7, 2026
View reviewed changes

@encukou encukou left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thank you!

({} is valid C! But not our C: it's new in C23...)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner reacted with hooray emoji
All reactions
  • 🎉 1 reaction
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Oct 7, 2026
@encukou
encukou merged commit ba9d96f into python:main Oct 7, 2026
58 of 59 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label Oct 7, 2026
@miss-islington-app

miss-islington-app Bot commented Oct 7, 2026

Copy link
Copy Markdown

Thanks @vstinner for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app

bedevere-app Bot commented Oct 7, 2026

Copy link
Copy Markdown

GH-158965 is a backport of this pull request to the 3.15 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label Oct 7, 2026
@vstinner

vstinner commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

({} is valid C! But not our C: it's new in C23...)

Oh, C23 seems to be far in the future, we "just" moved to C11 for the internal C code :-) But it's great that the C standard is still evolving! Thanks for the review and the merge. Using _Py_NULL was a good idea.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
vstinner deleted the pyslot_end branch October 7, 2026 12:28
vstinner added a commit that referenced this pull request Oct 9, 2026
@miss-islington @vstinner
[3.15] gh-149044: Avoid C++ warnings in PySlot_END macro (GH-158866) (#…
c78b5a0 
…158965)

gh-149044: Avoid C++ warnings in PySlot_END macro (GH-158866)

Initialize all fields in the macro. Otherwise, g++ -Wall -Wextra
complains that some fields are not initialized
[-Werror=missing-field-initializers]:

  error: missing initializer for member ‘PySlot::sl_flags’
  error: missing initializer for member ‘PySlot::<anonymous>’
  error: missing initializer for member ‘PySlot::<anonymous>’
(cherry picked from commit ba9d96f)

Co-authored-by: Victor Stinner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukou encukou encukou approved these changes
+1 more reviewer
@da-woods da-woods da-woods left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

topic-C-API type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@vstinner @da-woods @encukou
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.