From 3906de66cabd7430a3546b1ac90164a8e0505726 Mon Sep 17 00:00:00 2001 From: b-pass Date: Fri, 23 May 2025 16:33:22 -0400 Subject: [PATCH 01/13] These objects outlive the interpreter, they must be immortal. --- include/pybind11/detail/common.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 2e81150f16..2cdcde64ef 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -309,6 +309,12 @@ } while (0) #endif +#ifdef _Py_IMMORTAL_REFCNT +# define PYBIND11_IMMORTALIZE(o) Py_SET_REFCNT(o.ptr(), _Py_IMMORTAL_REFCNT) +#else +# define PYBIND11_IMMORTALIZE(o) +#endif + #define PYBIND11_CHECK_PYTHON_VERSION \ { \ const char *compiled_ver \ @@ -379,12 +385,14 @@ PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") slots[0] = {Py_mod_exec, \ reinterpret_cast(&PYBIND11_CONCAT(pybind11_exec_, name))}; \ slots[1] = {0, nullptr}; \ - return ::pybind11::module_::initialize_multiphase_module_def( \ + auto o = ::pybind11::module_::initialize_multiphase_module_def( \ PYBIND11_TOSTRING(name), \ nullptr, \ &PYBIND11_CONCAT(pybind11_module_def_, name), \ slots, \ ##__VA_ARGS__); \ + PYBIND11_IMMORTALIZE(o); \ + return o; \ }(); \ return result.ptr(); \ } From ac8bc821f5725ffd32b6e6f688d110b58f01e51d Mon Sep 17 00:00:00 2001 From: b-pass Date: Fri, 23 May 2025 16:54:53 -0400 Subject: [PATCH 02/13] Add some alternatives for immortalize on older versions --- include/pybind11/detail/common.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 2cdcde64ef..86103028e9 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -311,8 +311,10 @@ #ifdef _Py_IMMORTAL_REFCNT # define PYBIND11_IMMORTALIZE(o) Py_SET_REFCNT(o.ptr(), _Py_IMMORTAL_REFCNT) +#elif defined(Py_SET_REFCNT) +# define PYBIND11_IMMORTALIZE(o) Py_SET_REFCNT(o.ptr(), (UINT_MAX>>2)) #else -# define PYBIND11_IMMORTALIZE(o) +# define PYBIND11_IMMORTALIZE(o) Py_INCREF(o.ptr()) #endif #define PYBIND11_CHECK_PYTHON_VERSION \ From 476fd4b77243ae5bfcba90c0c8dddc72923678a7 Mon Sep 17 00:00:00 2001 From: b-pass Date: Fri, 23 May 2025 17:05:33 -0400 Subject: [PATCH 03/13] Format --- include/pybind11/detail/common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 86103028e9..81579cd527 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -312,7 +312,7 @@ #ifdef _Py_IMMORTAL_REFCNT # define PYBIND11_IMMORTALIZE(o) Py_SET_REFCNT(o.ptr(), _Py_IMMORTAL_REFCNT) #elif defined(Py_SET_REFCNT) -# define PYBIND11_IMMORTALIZE(o) Py_SET_REFCNT(o.ptr(), (UINT_MAX>>2)) +# define PYBIND11_IMMORTALIZE(o) Py_SET_REFCNT(o.ptr(), (UINT_MAX >> 2)) #else # define PYBIND11_IMMORTALIZE(o) Py_INCREF(o.ptr()) #endif From 441db31a762fd626888073317b8d7f90dcd88979 Mon Sep 17 00:00:00 2001 From: b-pass Date: Fri, 23 May 2025 17:17:58 -0400 Subject: [PATCH 04/13] Parenthesize macro parameter to make clang-tidy happy --- include/pybind11/detail/common.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 81579cd527..c6cef39804 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -310,11 +310,11 @@ #endif #ifdef _Py_IMMORTAL_REFCNT -# define PYBIND11_IMMORTALIZE(o) Py_SET_REFCNT(o.ptr(), _Py_IMMORTAL_REFCNT) +# define PYBIND11_IMMORTALIZE(o) Py_SET_REFCNT((o).ptr(), _Py_IMMORTAL_REFCNT) #elif defined(Py_SET_REFCNT) -# define PYBIND11_IMMORTALIZE(o) Py_SET_REFCNT(o.ptr(), (UINT_MAX >> 2)) +# define PYBIND11_IMMORTALIZE(o) Py_SET_REFCNT((o).ptr(), (UINT_MAX >> 2)) #else -# define PYBIND11_IMMORTALIZE(o) Py_INCREF(o.ptr()) +# define PYBIND11_IMMORTALIZE(o) Py_INCREF((o).ptr()) #endif #define PYBIND11_CHECK_PYTHON_VERSION \ From c4346e3f253bffbf870c7d5de89d84669c1ed778 Mon Sep 17 00:00:00 2001 From: b-pass Date: Sat, 24 May 2025 08:45:18 -0400 Subject: [PATCH 05/13] Return the PyModuleDef after it is initialized, call PyModuleDef_Init for every interpreter. --- include/pybind11/detail/common.h | 16 +++----------- include/pybind11/pybind11.h | 37 +++++++++++++------------------- 2 files changed, 18 insertions(+), 35 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index c6cef39804..905a6b8bce 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -309,14 +309,6 @@ } while (0) #endif -#ifdef _Py_IMMORTAL_REFCNT -# define PYBIND11_IMMORTALIZE(o) Py_SET_REFCNT((o).ptr(), _Py_IMMORTAL_REFCNT) -#elif defined(Py_SET_REFCNT) -# define PYBIND11_IMMORTALIZE(o) Py_SET_REFCNT((o).ptr(), (UINT_MAX >> 2)) -#else -# define PYBIND11_IMMORTALIZE(o) Py_INCREF((o).ptr()) -#endif - #define PYBIND11_CHECK_PYTHON_VERSION \ { \ const char *compiled_ver \ @@ -382,21 +374,19 @@ PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") PYBIND11_CHECK_PYTHON_VERSION \ pre_init; \ PYBIND11_ENSURE_INTERNALS_READY \ - static auto result = []() { \ + static auto def = []() { \ auto &slots = PYBIND11_CONCAT(pybind11_module_slots_, name); \ slots[0] = {Py_mod_exec, \ reinterpret_cast(&PYBIND11_CONCAT(pybind11_exec_, name))}; \ slots[1] = {0, nullptr}; \ - auto o = ::pybind11::module_::initialize_multiphase_module_def( \ + return ::pybind11::module_::initialize_multiphase_module_def( \ PYBIND11_TOSTRING(name), \ nullptr, \ &PYBIND11_CONCAT(pybind11_module_def_, name), \ slots, \ ##__VA_ARGS__); \ - PYBIND11_IMMORTALIZE(o); \ - return o; \ }(); \ - return result.ptr(); \ + return PyModuleDef_Init(def); \ } PYBIND11_WARNING_POP diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 9650be596f..1d2100a0cd 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1491,11 +1491,11 @@ class module_ : public object { additional slots from the supplied options (and the empty sentinel slot). \endrst */ template - static object initialize_multiphase_module_def(const char *name, - const char *doc, - module_def *def, - slots_array &slots, - Options &&...options) { + static module_def *initialize_multiphase_module_def(const char *name, + const char *doc, + module_def *def, + slots_array &slots, + Options &&...options) { size_t next_slot = 0; size_t term_slot = slots.size() - 1; @@ -1528,23 +1528,16 @@ class module_ : public object { // module_def is PyModuleDef // Placement new (not an allocation). - new (def) PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT, - /* m_name */ name, - /* m_doc */ options::show_user_defined_docstrings() ? doc : nullptr, - /* m_size */ 0, - /* m_methods */ nullptr, - /* m_slots */ &slots[0], - /* m_traverse */ nullptr, - /* m_clear */ nullptr, - /* m_free */ nullptr}; - auto *m = PyModuleDef_Init(def); - if (m == nullptr) { - if (PyErr_Occurred()) { - throw error_already_set(); - } - pybind11_fail("Internal error in module_::initialize_multiphase_module_def()"); - } - return reinterpret_borrow(m); + return new (def) + PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT, + /* m_name */ name, + /* m_doc */ options::show_user_defined_docstrings() ? doc : nullptr, + /* m_size */ 0, + /* m_methods */ nullptr, + /* m_slots */ &slots[0], + /* m_traverse */ nullptr, + /* m_clear */ nullptr, + /* m_free */ nullptr}; } }; From 8736e1ace0b4faca0b14dd68a3689b3649f9eea3 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 24 May 2025 12:17:54 -0700 Subject: [PATCH 06/13] Resolve long-standing `using module_def = PyModuleDef; // TODO: Can this be removed` The TODO was introduced with: https://github.com/pybind/pybind11/pull/3719/commits/7800e7faeddd132994057e99f04fbdf7274329ce --- include/pybind11/detail/common.h | 4 ++-- include/pybind11/pybind11.h | 14 ++++++-------- tests/test_modules.cpp | 3 +-- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 905a6b8bce..16407a6dee 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -366,7 +366,7 @@ PYBIND11_WARNING_PUSH PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") #define PYBIND11_MODULE_PYINIT(name, pre_init, ...) \ - static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name); \ + static PyModuleDef PYBIND11_CONCAT(pybind11_module_def_, name); \ static ::pybind11::module_::slots_array PYBIND11_CONCAT(pybind11_module_slots_, name); \ static int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject *); \ static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \ @@ -374,7 +374,7 @@ PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") PYBIND11_CHECK_PYTHON_VERSION \ pre_init; \ PYBIND11_ENSURE_INTERNALS_READY \ - static auto def = []() { \ + static PyModuleDef *def = []() { \ auto &slots = PYBIND11_CONCAT(pybind11_module_slots_, name); \ slots[0] = {Py_mod_exec, \ reinterpret_cast(&PYBIND11_CONCAT(pybind11_exec_, name))}; \ diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 1d2100a0cd..88ba77f3c9 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1438,8 +1438,6 @@ class module_ : public object { PyModule_AddObject(ptr(), name, obj.inc_ref().ptr() /* steals a reference */); } - using module_def = PyModuleDef; // TODO: Can this be removed (it was needed only for Python 2)? - /** \rst Create a new top-level module that can be used as the main module of a C extension. @@ -1447,7 +1445,7 @@ class module_ : public object { \endrst */ static module_ create_extension_module(const char *name, const char *doc, - module_def *def, + PyModuleDef *def, mod_gil_not_used gil_not_used = mod_gil_not_used(false)) { // module_def is PyModuleDef @@ -1491,11 +1489,11 @@ class module_ : public object { additional slots from the supplied options (and the empty sentinel slot). \endrst */ template - static module_def *initialize_multiphase_module_def(const char *name, - const char *doc, - module_def *def, - slots_array &slots, - Options &&...options) { + static PyModuleDef *initialize_multiphase_module_def(const char *name, + const char *doc, + PyModuleDef *def, + slots_array &slots, + Options &&...options) { size_t next_slot = 0; size_t term_slot = slots.size() - 1; diff --git a/tests/test_modules.cpp b/tests/test_modules.cpp index 7f01687c77..842a3bc4b8 100644 --- a/tests/test_modules.cpp +++ b/tests/test_modules.cpp @@ -78,8 +78,7 @@ TEST_SUBMODULE(modules, m) { class DupeException {}; // Go ahead and leak, until we have a non-leaking py::module_ constructor - auto dm - = py::module_::create_extension_module("dummy", nullptr, new py::module_::module_def); + auto dm = py::module_::create_extension_module("dummy", nullptr, new PyModuleDef); auto failures = py::list(); py::class_(dm, "Dupe1"); From 3cd05c64dcad9032e0fd25f038743ffe47dabdeb Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 24 May 2025 12:44:28 -0700 Subject: [PATCH 07/13] [skip ci] Clean up mentions of `module_def` in comments. --- include/pybind11/pybind11.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 88ba77f3c9..baac42d80d 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1441,14 +1441,13 @@ class module_ : public object { /** \rst Create a new top-level module that can be used as the main module of a C extension. - ``def`` should point to a statically allocated module_def. + ``def`` should point to a statically allocated PyModuleDef. \endrst */ static module_ create_extension_module(const char *name, const char *doc, PyModuleDef *def, mod_gil_not_used gil_not_used = mod_gil_not_used(false)) { - // module_def is PyModuleDef // Placement new (not an allocation). new (def) PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT, /* m_name */ name, @@ -1484,7 +1483,7 @@ class module_ : public object { /** \rst Initialize a module def for use with multi-phase module initialization. - ``def`` should point to a statically allocated module_def. + ``def`` should point to a statically allocated PyModuleDef. ``slots`` must already contain a Py_mod_exec or Py_mod_create slot and will be filled with additional slots from the supplied options (and the empty sentinel slot). \endrst */ @@ -1524,7 +1523,6 @@ class module_ : public object { } slots[next_slot++] = {0, nullptr}; - // module_def is PyModuleDef // Placement new (not an allocation). return new (def) PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT, From e17afcab308f0f30a1d432efc6794b39c688b550 Mon Sep 17 00:00:00 2001 From: b-pass Date: Sat, 24 May 2025 17:48:54 -0400 Subject: [PATCH 08/13] Clean up PYBIND11_MODULE_PYINIT even more - Get rid of initialize_multiphase_module_def and just initialize it here in the function. - Make a simple slots init - Mkae these function statics instead of globals - Get rid of the lambda and just use the PyModuleDef member initializer. --- include/pybind11/detail/common.h | 29 +++++----- include/pybind11/pybind11.h | 99 +++++++++++++------------------- 2 files changed, 52 insertions(+), 76 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 16407a6dee..63c2e6bffc 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -366,32 +366,29 @@ PYBIND11_WARNING_PUSH PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") #define PYBIND11_MODULE_PYINIT(name, pre_init, ...) \ - static PyModuleDef PYBIND11_CONCAT(pybind11_module_def_, name); \ - static ::pybind11::module_::slots_array PYBIND11_CONCAT(pybind11_module_slots_, name); \ static int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject *); \ - static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \ PYBIND11_PLUGIN_IMPL(name) { \ PYBIND11_CHECK_PYTHON_VERSION \ pre_init; \ PYBIND11_ENSURE_INTERNALS_READY \ - static PyModuleDef *def = []() { \ - auto &slots = PYBIND11_CONCAT(pybind11_module_slots_, name); \ - slots[0] = {Py_mod_exec, \ - reinterpret_cast(&PYBIND11_CONCAT(pybind11_exec_, name))}; \ - slots[1] = {0, nullptr}; \ - return ::pybind11::module_::initialize_multiphase_module_def( \ - PYBIND11_TOSTRING(name), \ - nullptr, \ - &PYBIND11_CONCAT(pybind11_module_def_, name), \ - slots, \ - ##__VA_ARGS__); \ - }(); \ - return PyModuleDef_Init(def); \ + static ::pybind11::detail::slots_array slots = ::pybind11::detail::init_slots( \ + &PYBIND11_CONCAT(pybind11_exec_, name), ##__VA_ARGS__); \ + static PyModuleDef def{/* m_base */ PyModuleDef_HEAD_INIT, \ + /* m_name */ PYBIND11_TOSTRING(name), \ + /* m_doc */ nullptr, \ + /* m_size */ 0, \ + /* m_methods */ nullptr, \ + /* m_slots */ slots.data(), \ + /* m_traverse */ nullptr, \ + /* m_clear */ nullptr, \ + /* m_free */ nullptr}; \ + return PyModuleDef_Init(&def); \ } PYBIND11_WARNING_POP #define PYBIND11_MODULE_EXEC(name, variable) \ + static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \ int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject * pm) { \ try { \ auto m = pybind11::reinterpret_borrow<::pybind11::module_>(pm); \ diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index baac42d80d..9a872f8ee5 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1325,6 +1325,45 @@ inline void *multi_interp_slot(F &&, O &&...o) { } #endif +/// Must be a POD type, and must hold enough entries for all of the possible slots PLUS ONE for +/// the sentinel (0) end slot. +using slots_array = std::array; + +/// Initialize an array of slots based on the supplied exec slot and options. +template +static slots_array init_slots(int (*exec_fn)(PyObject *), Options &&...options) { + size_t next_slot = 0; + slots_array slots; + constexpr size_t term_slot = slots.size() - 1; + + if (exec_fn != nullptr) { + slots[next_slot++] = {Py_mod_exec, reinterpret_cast(exec_fn)}; + } + +#ifdef Py_mod_multiple_interpreters + if (next_slot >= term_slot) { + pybind11_fail("initialize_multiphase_module_def: not enough space in slots"); + } + slots[next_slot++] = {Py_mod_multiple_interpreters, multi_interp_slot(options...)}; +#endif + + if (gil_not_used_option(options...)) { +#if defined(Py_mod_gil) && defined(Py_GIL_DISABLED) + if (next_slot >= term_slot) { + pybind11_fail("initialize_multiphase_module_def: not enough space in slots"); + } + slots[next_slot++] = {Py_mod_gil, Py_MOD_GIL_NOT_USED}; +#endif + } + + // slots must have a zero end sentinel + if (next_slot > term_slot) { + pybind11_fail("initialize_multiphase_module_def: not enough space in slots"); + } + slots[next_slot++] = {0, nullptr}; + return slots; +} + PYBIND11_NAMESPACE_END(detail) /// Wrapper for Python extension modules @@ -1475,66 +1514,6 @@ class module_ : public object { // For Python 2, reinterpret_borrow was correct. return reinterpret_borrow(m); } - - /// Must be a POD type, and must hold enough entries for all of the possible slots PLUS ONE for - /// the sentinel (0) end slot. - using slots_array = std::array; - - /** \rst - Initialize a module def for use with multi-phase module initialization. - - ``def`` should point to a statically allocated PyModuleDef. - ``slots`` must already contain a Py_mod_exec or Py_mod_create slot and will be filled with - additional slots from the supplied options (and the empty sentinel slot). - \endrst */ - template - static PyModuleDef *initialize_multiphase_module_def(const char *name, - const char *doc, - PyModuleDef *def, - slots_array &slots, - Options &&...options) { - size_t next_slot = 0; - size_t term_slot = slots.size() - 1; - - // find the end of the supplied slots - while (next_slot < term_slot && slots[next_slot].slot != 0) { - ++next_slot; - } - -#ifdef Py_mod_multiple_interpreters - if (next_slot >= term_slot) { - pybind11_fail("initialize_multiphase_module_def: not enough space in slots"); - } - slots[next_slot++] = {Py_mod_multiple_interpreters, detail::multi_interp_slot(options...)}; -#endif - - if (detail::gil_not_used_option(options...)) { -#if defined(Py_mod_gil) && defined(Py_GIL_DISABLED) - if (next_slot >= term_slot) { - pybind11_fail("initialize_multiphase_module_def: not enough space in slots"); - } - slots[next_slot++] = {Py_mod_gil, Py_MOD_GIL_NOT_USED}; -#endif - } - - // slots must have a zero end sentinel - if (next_slot > term_slot) { - pybind11_fail("initialize_multiphase_module_def: not enough space in slots"); - } - slots[next_slot++] = {0, nullptr}; - - // Placement new (not an allocation). - return new (def) - PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT, - /* m_name */ name, - /* m_doc */ options::show_user_defined_docstrings() ? doc : nullptr, - /* m_size */ 0, - /* m_methods */ nullptr, - /* m_slots */ &slots[0], - /* m_traverse */ nullptr, - /* m_clear */ nullptr, - /* m_free */ nullptr}; - } }; PYBIND11_NAMESPACE_BEGIN(detail) From dffb2d526cec24e7e41143b7d677e6d768a54ebc Mon Sep 17 00:00:00 2001 From: b-pass Date: Sat, 24 May 2025 17:56:44 -0400 Subject: [PATCH 09/13] Add a comment --- include/pybind11/detail/common.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 63c2e6bffc..9f08a60f80 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -365,6 +365,13 @@ PYBIND11_WARNING_PUSH PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") +/** +Create a PyInit_ function for this module. + +Note that this is run once for each (sub-)interpreter the module is imported into, including +possibly concurrently. The PyModuleDef is allowed to be static, but the PyObject* resulting from +PyModuleDef_Init should be treated like any other PyObject (so not shared across interpreters). + */ #define PYBIND11_MODULE_PYINIT(name, pre_init, ...) \ static int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject *); \ PYBIND11_PLUGIN_IMPL(name) { \ From 131d7d56fb3b819fd6f47469a1f30b460676ccea Mon Sep 17 00:00:00 2001 From: b-pass Date: Sun, 25 May 2025 08:24:33 -0400 Subject: [PATCH 10/13] Clean up error handling in init_slots, add a comment --- include/pybind11/detail/common.h | 8 ++++---- include/pybind11/pybind11.h | 17 +++++------------ 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 9f08a60f80..8535611b33 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -363,8 +363,10 @@ } \ PyObject *pybind11_init() +// this push is for the next several macros PYBIND11_WARNING_PUSH PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") + /** Create a PyInit_ function for this module. @@ -392,8 +394,6 @@ PyModuleDef_Init should be treated like any other PyObject (so not shared across return PyModuleDef_Init(&def); \ } -PYBIND11_WARNING_POP - #define PYBIND11_MODULE_EXEC(name, variable) \ static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \ int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject * pm) { \ @@ -446,12 +446,12 @@ PYBIND11_WARNING_POP } \endrst */ -PYBIND11_WARNING_PUSH -PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments") #define PYBIND11_MODULE(name, variable, ...) \ PYBIND11_MODULE_PYINIT( \ name, (pybind11::detail::get_num_interpreters_seen() += 1), ##__VA_ARGS__) \ PYBIND11_MODULE_EXEC(name, variable) + +// pop gnu-zero-variadic-macro-arguments PYBIND11_WARNING_POP PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 9a872f8ee5..1dfd41a207 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1331,36 +1331,29 @@ using slots_array = std::array; /// Initialize an array of slots based on the supplied exec slot and options. template -static slots_array init_slots(int (*exec_fn)(PyObject *), Options &&...options) { - size_t next_slot = 0; +static slots_array init_slots(int (*exec_fn)(PyObject *), Options &&...options) noexcept { + /* NOTE: slots_array MUST be large enough to hold all possibile options. If you add an option + here, you MUST also increase the size of slots_array in the typedef! */ slots_array slots; - constexpr size_t term_slot = slots.size() - 1; + size_t next_slot = 0; if (exec_fn != nullptr) { slots[next_slot++] = {Py_mod_exec, reinterpret_cast(exec_fn)}; } #ifdef Py_mod_multiple_interpreters - if (next_slot >= term_slot) { - pybind11_fail("initialize_multiphase_module_def: not enough space in slots"); - } slots[next_slot++] = {Py_mod_multiple_interpreters, multi_interp_slot(options...)}; #endif if (gil_not_used_option(options...)) { #if defined(Py_mod_gil) && defined(Py_GIL_DISABLED) - if (next_slot >= term_slot) { - pybind11_fail("initialize_multiphase_module_def: not enough space in slots"); - } slots[next_slot++] = {Py_mod_gil, Py_MOD_GIL_NOT_USED}; #endif } // slots must have a zero end sentinel - if (next_slot > term_slot) { - pybind11_fail("initialize_multiphase_module_def: not enough space in slots"); - } slots[next_slot++] = {0, nullptr}; + return slots; } From 69a649875b586217e401bf5f9cb62aef4e404a3c Mon Sep 17 00:00:00 2001 From: b-pass Date: Sun, 25 May 2025 08:42:25 -0400 Subject: [PATCH 11/13] [skip ci] spelling --- include/pybind11/pybind11.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 1dfd41a207..13b9ed8a60 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1332,7 +1332,7 @@ using slots_array = std::array; /// Initialize an array of slots based on the supplied exec slot and options. template static slots_array init_slots(int (*exec_fn)(PyObject *), Options &&...options) noexcept { - /* NOTE: slots_array MUST be large enough to hold all possibile options. If you add an option + /* NOTE: slots_array MUST be large enough to hold all possible options. If you add an option here, you MUST also increase the size of slots_array in the typedef! */ slots_array slots; size_t next_slot = 0; From 93d093cb8b64173bd79b331972cd119ae12d534b Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 25 May 2025 10:20:12 -0700 Subject: [PATCH 12/13] Add back `using module_def = PyModuleDef;` --- include/pybind11/pybind11.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 26cfb3f9f3..740d2e070c 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1470,6 +1470,9 @@ class module_ : public object { PyModule_AddObject(ptr(), name, obj.inc_ref().ptr() /* steals a reference */); } + // DEPRECATED (since PR #5688): Use PyModuleDef directly instead. + using module_def = PyModuleDef; + /** \rst Create a new top-level module that can be used as the main module of a C extension. From 580480f06b6040184826f3af2a8cc007d83ca721 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 25 May 2025 10:30:41 -0700 Subject: [PATCH 13/13] Replace `typedef` with `type alias` in comment. --- include/pybind11/pybind11.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 740d2e070c..e747e274d1 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1333,7 +1333,7 @@ using slots_array = std::array; template static slots_array init_slots(int (*exec_fn)(PyObject *), Options &&...options) noexcept { /* NOTE: slots_array MUST be large enough to hold all possible options. If you add an option - here, you MUST also increase the size of slots_array in the typedef! */ + here, you MUST also increase the size of slots_array in the type alias above! */ slots_array slots; size_t next_slot = 0;