Skip to content

Consolidate common code in PYBIND11_MODULE and PYBIND11_EMBEDDED_MODULE #5670

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 4 commits into from
May 20, 2025

Conversation

b-pass
Copy link
Contributor

@b-pass b-pass commented May 19, 2025

Description

As requested in #5665 by @rwgk.

The difference between the two macros is really small, almost all of the code is the same. So we can consolidate it into a common macro called by both. Both macros still exist and take the same number of arguments.

b-pass added 2 commits May 18, 2025 20:23
…to one macro

The difference between the two is really small, almost all of the code was the same.
@henryiii
Copy link
Collaborator

error: extra ';' [-Werror=pedantic]

@rwgk
Copy link
Collaborator

rwgk commented May 19, 2025

The 3.14t • macos-14 failure looks like a flake (GIL deadlock) that's definitely unrelated to this PR,

the AppVeyor failure looks like a temporary infrastructure failure,

but this one seems real, almost certainly a consequence of #5523:

/__w/pybind11/pybind11/include/pybind11/detail/common.h:287:64: error: redundant redeclaration of 'PyObject* PyInit_widget_module()' in same scope [-Werror=redundant-decls]
  287 |     extern "C" PYBIND11_MAYBE_UNUSED PYBIND11_EXPORT PyObject *PyInit_##name();
      | 

Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

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

Awesome, thanks a lot!

Leaving it to @henryiii to merge (to not interfere with his RC work).

@henryiii henryiii merged commit 9e6fe46 into pybind:master May 20, 2025
73 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs changelog Possibly needs a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants