Skip to content

gh-99202: Fix extension type from documentation for compiling in C++20 mode #102518

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 6, 2023

Conversation

jpn--
Copy link
Contributor

@jpn-- jpn-- commented Mar 7, 2023

As noted in #99202, C++20 added support for designated initializers and fails to compile if you mix named and unnamed initializers.

The documentation currently includes a section "Defining Extension Types: Tutorial" that mixed named and unnamed initializers, so it will not compile as shown.

This PR adds names on the header macros, so they do now compile in C++20 mode. The change is only made in the demo documentation, not any "real" code.

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@ghost
Copy link

ghost commented Mar 7, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@arhadthedev
Copy link
Member

@vstinner @encukou @erlend-aasland @bruno-at-bareos as participants of the parent issue.

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. IMO it's fine to use this syntax in the documentation for new extensions.

@encukou: What do you think?

Python 3.11 itself now targets C11: https://peps.python.org/pep-0007/#c-dialect Python still supports building extensions with older C standards and various C++ standards.

@encukou
Copy link
Member

encukou commented Apr 6, 2023

This actually looks great! I'm surprised that there aren't any obvious downsides!

IMO it would be nice to introduce PyVarObject_HEAD_BASE macros, named to match the field name, which wouldn't include the comma. But that's a separate issue: it would still make sense to use that with .ob_base =

Victor, do you want to merge? (I can't commit to watching/fixing buildbots this week.)

@vstinner vstinner merged commit 23cf1e2 into python:main Apr 6, 2023
@vstinner
Copy link
Member

vstinner commented Apr 6, 2023

Merged, thanks!

warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants