From 5efc7439d43f3dbe8de2526df3436b8d99b10dab Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 9 Sep 2024 14:31:38 -0400 Subject: [PATCH 01/15] chore(deps): bump the actions group with 2 updates (#5361) Bumps the actions group with 2 updates: [deadsnakes/action](https://github.com/deadsnakes/action) and [actions/attest-build-provenance](https://github.com/actions/attest-build-provenance). Updates `deadsnakes/action` from 3.1.0 to 3.2.0 - [Release notes](https://github.com/deadsnakes/action/releases) - [Commits](https://github.com/deadsnakes/action/compare/v3.1.0...v3.2.0) Updates `actions/attest-build-provenance` from 1.4.2 to 1.4.3 - [Release notes](https://github.com/actions/attest-build-provenance/releases) - [Changelog](https://github.com/actions/attest-build-provenance/blob/main/RELEASE.md) - [Commits](https://github.com/actions/attest-build-provenance/compare/6149ea5740be74af77f260b9db67e633f6b0a9a1...1c608d11d69870c2092266b3f9a6f3abbf17002c) --- updated-dependencies: - dependency-name: deadsnakes/action dependency-type: direct:production update-type: version-update:semver-minor dependency-group: actions - dependency-name: actions/attest-build-provenance dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 2 +- .github/workflows/pip.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3a687232bb..b4acbbf5e4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -243,7 +243,7 @@ jobs: - uses: actions/checkout@v4 - name: Setup Python ${{ matrix.python-version }} (deadsnakes) - uses: deadsnakes/action@v3.1.0 + uses: deadsnakes/action@v3.2.0 with: python-version: ${{ matrix.python-version }} debug: ${{ matrix.python-debug }} diff --git a/.github/workflows/pip.yml b/.github/workflows/pip.yml index af39716638..a2c4dba6bc 100644 --- a/.github/workflows/pip.yml +++ b/.github/workflows/pip.yml @@ -102,7 +102,7 @@ jobs: - uses: actions/download-artifact@v4 - name: Generate artifact attestation for sdist and wheel - uses: actions/attest-build-provenance@6149ea5740be74af77f260b9db67e633f6b0a9a1 # v1.4.2 + uses: actions/attest-build-provenance@1c608d11d69870c2092266b3f9a6f3abbf17002c # v1.4.3 with: subject-path: "*/pybind11*" From ef5a9560bb03efcac588888a825b7c7a0b8266cc Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 12 Sep 2024 21:18:29 -0700 Subject: [PATCH 02/15] Enable type-safe interoperability between different independent Python/C++ bindings systems. (#5296) * `self.__cpp_transporter__()` proof of concept: Enable passing C++ pointers across extensions even if the `PYBIND11_INTERNALS_VERSION`s do not match. * Include cleanup (mainly to resolve PyPy build failures). * Fix clang-tidy errors. * Resolve `error: extra * factor out platform_abi_id.h from internals.h (no functional changes) * factor out internals_version.h from internals.h (no functional changes) * Update CMakeLists.txt, tests/extra_python_package/test_files.py * Revert "factor out internals_version.h from internals.h (no functional changes)" This reverts commit 3ccea8cd4302cfaf2b6842f77ee3a2eefd61182a. * Remove internals_version.h from CMakeLists.txt, tests/extra_python_package/test_files.py * `.__cpp_transporter__()` implementation: compare `pybind11_platform_abi_id`, `cpp_typeid_name` * Add PremiumTraveler * Rename test_cpp_transporter_traveler_type.h -> test_cpp_transporter_traveler_types.h * Expand tests: `PremiumTraveler`, `get_points()` * Shuffle order of tests (no real changes). * Move `__cpp_transporter__` lambda to `py::cpp_transporter()` regular function. * Use `type_caster_generic::load(self)` instead of `cast(self)` * Pass `const std::type_info *` via `py::capsule` (instead of `cpp_typeid_name`). * Make platform_abi_id.h completely stand-alone. * rename exo_planet.cpp -> exo_planet_pybind11.cpp * Add exo_planet_c_api.cpp (incomplete). * Fix silly oversight (wrong filename in `#include`). * Resolve clang-tidy errors: ``` /__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:10:18: error: 'wrapGetLuggage' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors] 10 | static PyObject *wrapGetLuggage(PyObject *, PyObject *) { return PyUnicode_FromString("TODO"); } | ~~~~~~ ^ /__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:14:20: error: 'ThisMethodDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors] 14 | static PyMethodDef ThisMethodDef[] | ~~~~~~ ^ /__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:17:27: error: 'ThisModuleDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors] 17 | static struct PyModuleDef ThisModuleDef = { | ~~~~~~ ^ ``` * Implement exo_planet_c_api GetLuggage(), GetPoints() * Move new code from test_cpp_transporter_traveler_bindings.h to pybind11/detail/type_caster_base.h, under the name `class_dunder_cpp_transporter()` * Fix oversight. * Unconditionally add `__cpp_transporter__` method to all `py::class_` objects, but do not include that magic method in docstring signatures. * Back out pybind11/detail/platform_abi_id.h for now. Maximizing reusability can be handled separately, later. * Small cleanup. * Restore and add to `test_call_cpp_transporter_*()` * Ensure https://github.com/pybind/pybind11/issues/3788 does not bite again. * `class_dunder_cpp_transporter()`: replace `obj.cast()` with `std::string(obj)` * Add (simple) copyright notices in all newly added files. * Globally replace cpp_transporter with cpp_conduit * style: pre-commit fixes * IWYU fixes * Rename `class_dunder_cpp_conduit()` -> `cpp_conduit_method()` * Change `pybind11_platform_abi_id`, `pointer_kind` argument types from `str` to `bytes`. This avoids the unicode decode/encode roundtrips: * More robust (no decode/encode errors). * Minor runtime optimization. * Systematically rename `cap_cpp_type_info` -> `cpp_type_info_capsule` (no functional changes). * Systematically replace `cpp_type_info_capsule` `name`: `"const std::type_info *"` -> `typeid(std::type_info).name()` (this IS a functional change). This provides an extra layer of protection against C++ ABI mismatches: * The first and most important layer is that the `PYBIND11_PLATFORM_ABI_ID`s must match between extensions. * The second layer is that the `typeid(std::type_info).name()`s must match between extensions. * Fix sort order accident in tests/CMakeLists.txt * Apply suggestions from code review Co-authored-by: Aaron Gokaslan * style: pre-commit fixes * refactor: rename to _pybind_conduit_v1_ Signed-off-by: Henry Schreiner * Add test_home_planet_wrap_very_lonely_traveler(), test_exo_planet_pybind11_wrap_very_lonely_traveler() * Resolve clang-tidy errors: ``` /__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:39:32: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors] 10 | py::class_(m, "LonelyTraveler"); | ^ | std::move( ) /__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:43:52: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors] 43 | py::class_(m, "VeryLonelyTraveler"); | ^ | std::move( ) ``` --------- Signed-off-by: Henry Schreiner Co-authored-by: Ralf W. Grosse-Kunstleve Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Henry Schreiner Co-authored-by: Aaron Gokaslan --- CMakeLists.txt | 1 + include/pybind11/detail/cpp_conduit.h | 77 ++++++++++ include/pybind11/detail/internals.h | 10 +- include/pybind11/detail/type_caster_base.h | 40 +++++ include/pybind11/pybind11.h | 7 +- tests/CMakeLists.txt | 3 + tests/exo_planet_c_api.cpp | 103 +++++++++++++ tests/exo_planet_pybind11.cpp | 19 +++ tests/extra_python_package/test_files.py | 1 + tests/home_planet_very_lonely_traveler.cpp | 13 ++ tests/test_cpp_conduit.cpp | 22 +++ tests/test_cpp_conduit.py | 162 +++++++++++++++++++++ tests/test_cpp_conduit_traveler_bindings.h | 47 ++++++ tests/test_cpp_conduit_traveler_types.h | 25 ++++ 14 files changed, 524 insertions(+), 6 deletions(-) create mode 100644 include/pybind11/detail/cpp_conduit.h create mode 100644 tests/exo_planet_c_api.cpp create mode 100644 tests/exo_planet_pybind11.cpp create mode 100644 tests/home_planet_very_lonely_traveler.cpp create mode 100644 tests/test_cpp_conduit.cpp create mode 100644 tests/test_cpp_conduit.py create mode 100644 tests/test_cpp_conduit_traveler_bindings.h create mode 100644 tests/test_cpp_conduit_traveler_types.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 2b6b564e52..b088eb55f5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -129,6 +129,7 @@ endif() set(PYBIND11_HEADERS include/pybind11/detail/class.h include/pybind11/detail/common.h + include/pybind11/detail/cpp_conduit.h include/pybind11/detail/descr.h include/pybind11/detail/init.h include/pybind11/detail/internals.h diff --git a/include/pybind11/detail/cpp_conduit.h b/include/pybind11/detail/cpp_conduit.h new file mode 100644 index 0000000000..b66c2d39c0 --- /dev/null +++ b/include/pybind11/detail/cpp_conduit.h @@ -0,0 +1,77 @@ +// Copyright (c) 2024 The pybind Community. + +#pragma once + +#include + +#include "common.h" +#include "internals.h" + +#include + +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) +PYBIND11_NAMESPACE_BEGIN(detail) + +// Forward declaration needed here: Refactoring opportunity. +extern "C" inline PyObject *pybind11_object_new(PyTypeObject *type, PyObject *, PyObject *); + +inline bool type_is_managed_by_our_internals(PyTypeObject *type_obj) { +#if defined(PYPY_VERSION) + auto &internals = get_internals(); + return bool(internals.registered_types_py.find(type_obj) + != internals.registered_types_py.end()); +#else + return bool(type_obj->tp_new == pybind11_object_new); +#endif +} + +inline bool is_instance_method_of_type(PyTypeObject *type_obj, PyObject *attr_name) { + PyObject *descr = _PyType_Lookup(type_obj, attr_name); + return bool((descr != nullptr) && PyInstanceMethod_Check(descr)); +} + +inline object try_get_cpp_conduit_method(PyObject *obj) { + if (PyType_Check(obj)) { + return object(); + } + PyTypeObject *type_obj = Py_TYPE(obj); + str attr_name("_pybind11_conduit_v1_"); + bool assumed_to_be_callable = false; + if (type_is_managed_by_our_internals(type_obj)) { + if (!is_instance_method_of_type(type_obj, attr_name.ptr())) { + return object(); + } + assumed_to_be_callable = true; + } + PyObject *method = PyObject_GetAttr(obj, attr_name.ptr()); + if (method == nullptr) { + PyErr_Clear(); + return object(); + } + if (!assumed_to_be_callable && PyCallable_Check(method) == 0) { + Py_DECREF(method); + return object(); + } + return reinterpret_steal(method); +} + +inline void *try_raw_pointer_ephemeral_from_cpp_conduit(handle src, + const std::type_info *cpp_type_info) { + object method = try_get_cpp_conduit_method(src.ptr()); + if (method) { + capsule cpp_type_info_capsule(const_cast(static_cast(cpp_type_info)), + typeid(std::type_info).name()); + object cpp_conduit = method(bytes(PYBIND11_PLATFORM_ABI_ID), + cpp_type_info_capsule, + bytes("raw_pointer_ephemeral")); + if (isinstance(cpp_conduit)) { + return reinterpret_borrow(cpp_conduit).get_pointer(); + } + } + return nullptr; +} + +#define PYBIND11_HAS_CPP_CONDUIT 1 + +PYBIND11_NAMESPACE_END(detail) +PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 17e6df8244..7cc6f53adf 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -321,15 +321,17 @@ struct type_info { # define PYBIND11_INTERNALS_KIND "" #endif +#define PYBIND11_PLATFORM_ABI_ID \ + PYBIND11_INTERNALS_KIND PYBIND11_COMPILER_TYPE PYBIND11_STDLIB PYBIND11_BUILD_ABI \ + PYBIND11_BUILD_TYPE + #define PYBIND11_INTERNALS_ID \ "__pybind11_internals_v" PYBIND11_TOSTRING(PYBIND11_INTERNALS_VERSION) \ - PYBIND11_INTERNALS_KIND PYBIND11_COMPILER_TYPE PYBIND11_STDLIB \ - PYBIND11_BUILD_ABI PYBIND11_BUILD_TYPE "__" + PYBIND11_PLATFORM_ABI_ID "__" #define PYBIND11_MODULE_LOCAL_ID \ "__pybind11_module_local_v" PYBIND11_TOSTRING(PYBIND11_INTERNALS_VERSION) \ - PYBIND11_INTERNALS_KIND PYBIND11_COMPILER_TYPE PYBIND11_STDLIB \ - PYBIND11_BUILD_ABI PYBIND11_BUILD_TYPE "__" + PYBIND11_PLATFORM_ABI_ID "__" /// Each module locally stores a pointer to the `internals` data. The data /// itself is shared among modules with the same `PYBIND11_INTERNALS_ID`. diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index bc42252f9d..e40e44ba6c 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -12,14 +12,17 @@ #include #include "common.h" +#include "cpp_conduit.h" #include "descr.h" #include "internals.h" #include "typeid.h" #include "value_and_holder.h" #include +#include #include #include +#include #include #include #include @@ -611,6 +614,13 @@ class type_caster_generic { } return false; } + bool try_cpp_conduit(handle src) { + value = try_raw_pointer_ephemeral_from_cpp_conduit(src, cpptype); + if (value != nullptr) { + return true; + } + return false; + } void check_holder_compat() {} PYBIND11_NOINLINE static void *local_load(PyObject *src, const type_info *ti) { @@ -742,6 +752,10 @@ class type_caster_generic { return true; } + if (convert && cpptype && this_.try_cpp_conduit(src)) { + return true; + } + return false; } @@ -769,6 +783,32 @@ class type_caster_generic { void *value = nullptr; }; +inline object cpp_conduit_method(handle self, + const bytes &pybind11_platform_abi_id, + const capsule &cpp_type_info_capsule, + const bytes &pointer_kind) { +#ifdef PYBIND11_HAS_STRING_VIEW + using cpp_str = std::string_view; +#else + using cpp_str = std::string; +#endif + if (cpp_str(pybind11_platform_abi_id) != PYBIND11_PLATFORM_ABI_ID) { + return none(); + } + if (std::strcmp(cpp_type_info_capsule.name(), typeid(std::type_info).name()) != 0) { + return none(); + } + if (cpp_str(pointer_kind) != "raw_pointer_ephemeral") { + throw std::runtime_error("Invalid pointer_kind: \"" + std::string(pointer_kind) + "\""); + } + const auto *cpp_type_info = cpp_type_info_capsule.get_pointer(); + type_caster_generic caster(*cpp_type_info); + if (!caster.load(self, false)) { + return none(); + } + return capsule(caster.value, cpp_type_info->name()); +} + /** * Determine suitable casting operator for pointer-or-lvalue-casting type casters. The type caster * needs to provide `operator T*()` and `operator T&()` operators. diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 1806583e6b..5219c0ff85 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -592,7 +592,8 @@ class cpp_function : public function { int index = 0; /* Create a nice pydoc rec including all signatures and docstrings of the functions in the overload chain */ - if (chain && options::show_function_signatures()) { + if (chain && options::show_function_signatures() + && std::strcmp(rec->name, "_pybind11_conduit_v1_") != 0) { // First a generic signature signatures += rec->name; signatures += "(*args, **kwargs)\n"; @@ -601,7 +602,8 @@ class cpp_function : public function { // Then specific overload signatures bool first_user_def = true; for (auto *it = chain_start; it != nullptr; it = it->next) { - if (options::show_function_signatures()) { + if (options::show_function_signatures() + && std::strcmp(rec->name, "_pybind11_conduit_v1_") != 0) { if (index > 0) { signatures += '\n'; } @@ -1601,6 +1603,7 @@ class class_ : public detail::generic_type { = instances[std::type_index(typeid(type))]; }); } + def("_pybind11_conduit_v1_", cpp_conduit_method); } template ::value, int> = 0> diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index d2156f46d3..03ff39138d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -118,6 +118,7 @@ set(PYBIND11_TEST_FILES test_const_name test_constants_and_functions test_copy_move + test_cpp_conduit test_custom_type_casters test_custom_type_setup test_docstring_options @@ -218,6 +219,8 @@ tests_extra_targets("test_exceptions.py;test_local_bindings.py;test_stl.py;test_ # And add additional targets for other tests. tests_extra_targets("test_exceptions.py" "cross_module_interleaved_error_already_set") tests_extra_targets("test_gil_scoped.py" "cross_module_gil_utils") +tests_extra_targets("test_cpp_conduit.py" + "exo_planet_pybind11;exo_planet_c_api;home_planet_very_lonely_traveler") set(PYBIND11_EIGEN_REPO "https://gitlab.com/libeigen/eigen.git" diff --git a/tests/exo_planet_c_api.cpp b/tests/exo_planet_c_api.cpp new file mode 100644 index 0000000000..3bde0b27b5 --- /dev/null +++ b/tests/exo_planet_c_api.cpp @@ -0,0 +1,103 @@ +// Copyright (c) 2024 The pybind Community. + +// THIS MUST STAY AT THE TOP! +#include // EXCLUSIVELY for PYBIND11_PLATFORM_ABI_ID +// Potential future direction to maximize reusability: +// (e.g. for use from SWIG, Cython, PyCLIF, nanobind): +// #include +// This would only depend on: +// 1. A C++ compiler, WITHOUT requiring -fexceptions. +// 2. Python.h + +#include "test_cpp_conduit_traveler_types.h" + +#include +#include + +namespace { + +void *get_cpp_conduit_void_ptr(PyObject *py_obj, const std::type_info *cpp_type_info) { + PyObject *cpp_type_info_capsule + = PyCapsule_New(const_cast(static_cast(cpp_type_info)), + typeid(std::type_info).name(), + nullptr); + if (cpp_type_info_capsule == nullptr) { + return nullptr; + } + PyObject *cpp_conduit = PyObject_CallMethod(py_obj, + "_pybind11_conduit_v1_", + "yOy", + PYBIND11_PLATFORM_ABI_ID, + cpp_type_info_capsule, + "raw_pointer_ephemeral"); + Py_DECREF(cpp_type_info_capsule); + if (cpp_conduit == nullptr) { + return nullptr; + } + void *void_ptr = PyCapsule_GetPointer(cpp_conduit, cpp_type_info->name()); + Py_DECREF(cpp_conduit); + if (PyErr_Occurred()) { + return nullptr; + } + return void_ptr; +} + +template +T *get_cpp_conduit_type_ptr(PyObject *py_obj) { + void *void_ptr = get_cpp_conduit_void_ptr(py_obj, &typeid(T)); + if (void_ptr == nullptr) { + return nullptr; + } + return static_cast(void_ptr); +} + +extern "C" PyObject *wrapGetLuggage(PyObject * /*self*/, PyObject *traveler) { + const auto *cpp_traveler + = get_cpp_conduit_type_ptr(traveler); + if (cpp_traveler == nullptr) { + return nullptr; + } + return PyUnicode_FromString(cpp_traveler->luggage.c_str()); +} + +extern "C" PyObject *wrapGetPoints(PyObject * /*self*/, PyObject *premium_traveler) { + const auto *cpp_premium_traveler + = get_cpp_conduit_type_ptr( + premium_traveler); + if (cpp_premium_traveler == nullptr) { + return nullptr; + } + return PyLong_FromLong(static_cast(cpp_premium_traveler->points)); +} + +PyMethodDef ThisMethodDef[] = {{"GetLuggage", wrapGetLuggage, METH_O, nullptr}, + {"GetPoints", wrapGetPoints, METH_O, nullptr}, + {nullptr, nullptr, 0, nullptr}}; + +struct PyModuleDef ThisModuleDef = { + PyModuleDef_HEAD_INIT, // m_base + "exo_planet_c_api", // m_name + nullptr, // m_doc + -1, // m_size + ThisMethodDef, // m_methods + nullptr, // m_slots + nullptr, // m_traverse + nullptr, // m_clear + nullptr // m_free +}; + +} // namespace + +#if defined(WIN32) || defined(_WIN32) +# define EXO_PLANET_C_API_EXPORT __declspec(dllexport) +#else +# define EXO_PLANET_C_API_EXPORT __attribute__((visibility("default"))) +#endif + +extern "C" EXO_PLANET_C_API_EXPORT PyObject *PyInit_exo_planet_c_api() { + PyObject *m = PyModule_Create(&ThisModuleDef); + if (m == nullptr) { + return nullptr; + } + return m; +} diff --git a/tests/exo_planet_pybind11.cpp b/tests/exo_planet_pybind11.cpp new file mode 100644 index 0000000000..9d1a2b84b6 --- /dev/null +++ b/tests/exo_planet_pybind11.cpp @@ -0,0 +1,19 @@ +// Copyright (c) 2024 The pybind Community. + +#if defined(PYBIND11_INTERNALS_VERSION) +# undef PYBIND11_INTERNALS_VERSION +#endif +#define PYBIND11_INTERNALS_VERSION 900000001 + +#include "test_cpp_conduit_traveler_bindings.h" + +namespace pybind11_tests { +namespace test_cpp_conduit { + +PYBIND11_MODULE(exo_planet_pybind11, m) { + wrap_traveler(m); + m.def("wrap_very_lonely_traveler", [m]() { wrap_very_lonely_traveler(m); }); +} + +} // namespace test_cpp_conduit +} // namespace pybind11_tests diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py index 18299bf5a4..cb11933a70 100644 --- a/tests/extra_python_package/test_files.py +++ b/tests/extra_python_package/test_files.py @@ -53,6 +53,7 @@ detail_headers = { "include/pybind11/detail/class.h", "include/pybind11/detail/common.h", + "include/pybind11/detail/cpp_conduit.h", "include/pybind11/detail/descr.h", "include/pybind11/detail/init.h", "include/pybind11/detail/internals.h", diff --git a/tests/home_planet_very_lonely_traveler.cpp b/tests/home_planet_very_lonely_traveler.cpp new file mode 100644 index 0000000000..78d50cff5d --- /dev/null +++ b/tests/home_planet_very_lonely_traveler.cpp @@ -0,0 +1,13 @@ +// Copyright (c) 2024 The pybind Community. + +#include "test_cpp_conduit_traveler_bindings.h" + +namespace pybind11_tests { +namespace test_cpp_conduit { + +PYBIND11_MODULE(home_planet_very_lonely_traveler, m) { + m.def("wrap_very_lonely_traveler", [m]() { wrap_very_lonely_traveler(m); }); +} + +} // namespace test_cpp_conduit +} // namespace pybind11_tests diff --git a/tests/test_cpp_conduit.cpp b/tests/test_cpp_conduit.cpp new file mode 100644 index 0000000000..4ee4f06905 --- /dev/null +++ b/tests/test_cpp_conduit.cpp @@ -0,0 +1,22 @@ +// Copyright (c) 2024 The pybind Community. + +#include "pybind11_tests.h" +#include "test_cpp_conduit_traveler_bindings.h" + +#include + +namespace pybind11_tests { +namespace test_cpp_conduit { + +TEST_SUBMODULE(cpp_conduit, m) { + m.attr("PYBIND11_PLATFORM_ABI_ID") = py::bytes(PYBIND11_PLATFORM_ABI_ID); + m.attr("cpp_type_info_capsule_Traveler") + = py::capsule(&typeid(Traveler), typeid(std::type_info).name()); + m.attr("cpp_type_info_capsule_int") = py::capsule(&typeid(int), typeid(std::type_info).name()); + + wrap_traveler(m); + wrap_lonely_traveler(m); +} + +} // namespace test_cpp_conduit +} // namespace pybind11_tests diff --git a/tests/test_cpp_conduit.py b/tests/test_cpp_conduit.py new file mode 100644 index 0000000000..51fcf69368 --- /dev/null +++ b/tests/test_cpp_conduit.py @@ -0,0 +1,162 @@ +# Copyright (c) 2024 The pybind Community. + +from __future__ import annotations + +import exo_planet_c_api +import exo_planet_pybind11 +import home_planet_very_lonely_traveler +import pytest + +from pybind11_tests import cpp_conduit as home_planet + + +def test_traveler_getattr_actually_exists(): + t_h = home_planet.Traveler("home") + assert t_h.any_name == "Traveler GetAttr: any_name luggage: home" + + +def test_premium_traveler_getattr_actually_exists(): + t_h = home_planet.PremiumTraveler("home", 7) + assert t_h.secret_name == "PremiumTraveler GetAttr: secret_name points: 7" + + +def test_call_cpp_conduit_success(): + t_h = home_planet.Traveler("home") + cap = t_h._pybind11_conduit_v1_( + home_planet.PYBIND11_PLATFORM_ABI_ID, + home_planet.cpp_type_info_capsule_Traveler, + b"raw_pointer_ephemeral", + ) + assert cap.__class__.__name__ == "PyCapsule" + + +def test_call_cpp_conduit_platform_abi_id_mismatch(): + t_h = home_planet.Traveler("home") + cap = t_h._pybind11_conduit_v1_( + home_planet.PYBIND11_PLATFORM_ABI_ID + b"MISMATCH", + home_planet.cpp_type_info_capsule_Traveler, + b"raw_pointer_ephemeral", + ) + assert cap is None + + +def test_call_cpp_conduit_cpp_type_info_capsule_mismatch(): + t_h = home_planet.Traveler("home") + cap = t_h._pybind11_conduit_v1_( + home_planet.PYBIND11_PLATFORM_ABI_ID, + home_planet.cpp_type_info_capsule_int, + b"raw_pointer_ephemeral", + ) + assert cap is None + + +def test_call_cpp_conduit_pointer_kind_invalid(): + t_h = home_planet.Traveler("home") + with pytest.raises( + RuntimeError, match='^Invalid pointer_kind: "raw_pointer_ephemreal"$' + ): + t_h._pybind11_conduit_v1_( + home_planet.PYBIND11_PLATFORM_ABI_ID, + home_planet.cpp_type_info_capsule_Traveler, + b"raw_pointer_ephemreal", + ) + + +def test_home_only_basic(): + t_h = home_planet.Traveler("home") + assert t_h.luggage == "home" + assert home_planet.get_luggage(t_h) == "home" + + +def test_home_only_premium(): + p_h = home_planet.PremiumTraveler("home", 2) + assert p_h.luggage == "home" + assert home_planet.get_luggage(p_h) == "home" + assert home_planet.get_points(p_h) == 2 + + +def test_exo_only_basic(): + t_e = exo_planet_pybind11.Traveler("exo") + assert t_e.luggage == "exo" + assert exo_planet_pybind11.get_luggage(t_e) == "exo" + + +def test_exo_only_premium(): + p_e = exo_planet_pybind11.PremiumTraveler("exo", 3) + assert p_e.luggage == "exo" + assert exo_planet_pybind11.get_luggage(p_e) == "exo" + assert exo_planet_pybind11.get_points(p_e) == 3 + + +def test_home_passed_to_exo_basic(): + t_h = home_planet.Traveler("home") + assert exo_planet_pybind11.get_luggage(t_h) == "home" + + +def test_exo_passed_to_home_basic(): + t_e = exo_planet_pybind11.Traveler("exo") + assert home_planet.get_luggage(t_e) == "exo" + + +def test_home_passed_to_exo_premium(): + p_h = home_planet.PremiumTraveler("home", 2) + assert exo_planet_pybind11.get_luggage(p_h) == "home" + assert exo_planet_pybind11.get_points(p_h) == 2 + + +def test_exo_passed_to_home_premium(): + p_e = exo_planet_pybind11.PremiumTraveler("exo", 3) + assert home_planet.get_luggage(p_e) == "exo" + assert home_planet.get_points(p_e) == 3 + + +@pytest.mark.parametrize( + "traveler_type", [home_planet.Traveler, exo_planet_pybind11.Traveler] +) +def test_exo_planet_c_api_traveler(traveler_type): + t = traveler_type("socks") + assert exo_planet_c_api.GetLuggage(t) == "socks" + + +@pytest.mark.parametrize( + "premium_traveler_type", + [home_planet.PremiumTraveler, exo_planet_pybind11.PremiumTraveler], +) +def test_exo_planet_c_api_premium_traveler(premium_traveler_type): + pt = premium_traveler_type("gucci", 5) + assert exo_planet_c_api.GetLuggage(pt) == "gucci" + assert exo_planet_c_api.GetPoints(pt) == 5 + + +def test_home_planet_wrap_very_lonely_traveler(): + # This does not exercise the cpp_conduit feature, but is here to + # demonstrate that the cpp_conduit feature does not solve all + # cross-extension interoperability issues. + # Here is the proof that the following works for extensions with + # matching `PYBIND11_INTERNALS_ID`s: + # test_cpp_conduit.cpp: + # py::class_ + # home_planet_very_lonely_traveler.cpp: + # py::class_ + # See test_exo_planet_pybind11_wrap_very_lonely_traveler() for the negative + # test. + assert home_planet.LonelyTraveler is not None # Verify that the base class exists. + home_planet_very_lonely_traveler.wrap_very_lonely_traveler() + # Ensure that the derived class exists. + assert home_planet_very_lonely_traveler.VeryLonelyTraveler is not None + + +def test_exo_planet_pybind11_wrap_very_lonely_traveler(): + # See comment under test_home_planet_wrap_very_lonely_traveler() first. + # Here the `PYBIND11_INTERNALS_ID`s don't match between: + # test_cpp_conduit.cpp: + # py::class_ + # exo_planet_pybind11.cpp: + # py::class_ + assert home_planet.LonelyTraveler is not None # Verify that the base class exists. + with pytest.raises( + RuntimeError, + match='^generic_type: type "VeryLonelyTraveler" referenced unknown base type ' + '"pybind11_tests::test_cpp_conduit::LonelyTraveler"$', + ): + exo_planet_pybind11.wrap_very_lonely_traveler() diff --git a/tests/test_cpp_conduit_traveler_bindings.h b/tests/test_cpp_conduit_traveler_bindings.h new file mode 100644 index 0000000000..4e52c90c15 --- /dev/null +++ b/tests/test_cpp_conduit_traveler_bindings.h @@ -0,0 +1,47 @@ +// Copyright (c) 2024 The pybind Community. + +#pragma once + +#include + +#include "test_cpp_conduit_traveler_types.h" + +#include + +namespace pybind11_tests { +namespace test_cpp_conduit { + +namespace py = pybind11; + +inline void wrap_traveler(py::module_ m) { + py::class_(m, "Traveler") + .def(py::init()) + .def_readwrite("luggage", &Traveler::luggage) + // See issue #3788: + .def("__getattr__", [](const Traveler &self, const std::string &key) { + return "Traveler GetAttr: " + key + " luggage: " + self.luggage; + }); + + m.def("get_luggage", [](const Traveler &person) { return person.luggage; }); + + py::class_(m, "PremiumTraveler") + .def(py::init()) + .def_readwrite("points", &PremiumTraveler::points) + // See issue #3788: + .def("__getattr__", [](const PremiumTraveler &self, const std::string &key) { + return "PremiumTraveler GetAttr: " + key + " points: " + std::to_string(self.points); + }); + + m.def("get_points", [](const PremiumTraveler &person) { return person.points; }); +} + +inline void wrap_lonely_traveler(py::module_ m) { + py::class_(std::move(m), "LonelyTraveler"); +} + +inline void wrap_very_lonely_traveler(py::module_ m) { + py::class_(std::move(m), "VeryLonelyTraveler"); +} + +} // namespace test_cpp_conduit +} // namespace pybind11_tests diff --git a/tests/test_cpp_conduit_traveler_types.h b/tests/test_cpp_conduit_traveler_types.h new file mode 100644 index 0000000000..b8e6a5a771 --- /dev/null +++ b/tests/test_cpp_conduit_traveler_types.h @@ -0,0 +1,25 @@ +// Copyright (c) 2024 The pybind Community. + +#pragma once + +#include + +namespace pybind11_tests { +namespace test_cpp_conduit { + +struct Traveler { + explicit Traveler(const std::string &luggage) : luggage(luggage) {} + std::string luggage; +}; + +struct PremiumTraveler : Traveler { + explicit PremiumTraveler(const std::string &luggage, int points) + : Traveler(luggage), points(points) {} + int points; +}; + +struct LonelyTraveler {}; +struct VeryLonelyTraveler : LonelyTraveler {}; + +} // namespace test_cpp_conduit +} // namespace pybind11_tests From 5b7c0b04b9f9993912808da444de3b2ebdc65e80 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 13 Sep 2024 16:59:50 -0400 Subject: [PATCH 03/15] docs: update changelog for 2.13.6 (#5372) * docs: update changelog for 2.13.6 Signed-off-by: Henry Schreiner * docs: mention supported versions --------- Signed-off-by: Henry Schreiner --- docs/changelog.rst | 55 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0145317743..a91082113f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -31,6 +31,37 @@ New Features: * The ``array_caster`` in pybind11/stl.h was enhanced to support value types that are not default-constructible. `#5305 `_ +* Added ``py::warnings`` namespace with ``py::warnings::warn`` and ``py::warnings::new_warning_type`` that provides the interface for Python warnings. + `#5291 `_ + +Version 2.13.6 (September 13, 2024) +----------------------------------- + +New Features: + +* A new ``self._pybind11_conduit_v1_()`` method is automatically added to all + ``py::class_``-wrapped types, to enable type-safe interoperability between + different independent Python/C++ bindings systems, including pybind11 + versions with different ``PYBIND11_INTERNALS_VERSION``'s. Supported on + pybind11 2.11.2, 2.12.1, and 2.13.6+. + `#5296 `_ + + +Bug fixes: + +* Using ``__cpp_nontype_template_args`` instead of ``__cpp_nontype_template_parameter_class``. + `#5330 `_ + +* Properly translate C++ exception to Python exception when creating Python buffer from wrapped object. + `#5324 `_ + + +Documentation: + +* Adds an answer (FAQ) for "What is a highly conclusive and simple way to find memory leaks?". + `#5340 `_ + + Version 2.13.5 (August 22, 2024) -------------------------------- @@ -238,6 +269,18 @@ Other: * Update docs and noxfile. `#5071 `_ +Version 2.12.1 (September 13, 2024) +----------------------------------- + +New Features: + +* A new ``self._pybind11_conduit_v1_()`` method is automatically added to all + ``py::class_``-wrapped types, to enable type-safe interoperability between + different independent Python/C++ bindings systems, including pybind11 + versions with different ``PYBIND11_INTERNALS_VERSION``'s. Supported on + pybind11 2.11.2, 2.12.1, and 2.13.6+. + `#5296 `_ + Version 2.12.0 (March 27, 2024) ------------------------------- @@ -413,6 +456,18 @@ Other: * An ``assert()`` was added to help Coverty avoid generating a false positive. `#4817 `_ +Version 2.11.2 (September 13, 2024) +----------------------------------- + +New Features: + +* A new ``self._pybind11_conduit_v1_()`` method is automatically added to all + ``py::class_``-wrapped types, to enable type-safe interoperability between + different independent Python/C++ bindings systems, including pybind11 + versions with different ``PYBIND11_INTERNALS_VERSION``'s. Supported on + pybind11 2.11.2, 2.12.1, and 2.13.6+. + `#5296 `_ + Version 2.11.1 (July 17, 2023) ------------------------------ From 0cf3a0f7b514b3a3fb541ad3197964fa5c4da554 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 13 Sep 2024 20:21:43 -0400 Subject: [PATCH 04/15] ci: PyPI attestations (#5374) --- .github/workflows/pip.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pip.yml b/.github/workflows/pip.yml index a2c4dba6bc..3713537379 100644 --- a/.github/workflows/pip.yml +++ b/.github/workflows/pip.yml @@ -91,11 +91,12 @@ jobs: runs-on: ubuntu-latest if: github.event_name == 'release' && github.event.action == 'published' needs: [packaging] - environment: pypi + environment: + name: pypi + url: https://pypi.org/p/pybind11 permissions: id-token: write attestations: write - contents: read steps: # Downloads all to directories matching the artifact names @@ -110,8 +111,10 @@ jobs: uses: pypa/gh-action-pypi-publish@release/v1 with: packages-dir: standard/ + attestations: true - name: Publish global package uses: pypa/gh-action-pypi-publish@release/v1 with: packages-dir: global/ + attestations: true From a7910be6307e442297e55c84901a96dd0d046e93 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Sat, 14 Sep 2024 23:51:50 -0700 Subject: [PATCH 05/15] Add warn disable for GGC 12 bound checking error (#5355) Issue: #5224 The `internals.registered_types_py...` line in pybind11.h triggers a false-positive bounds checking warning in GCC 12. This is discussed in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115824. The workaround implemented is taken from suggestions then refactored to use the `PYBIND11_WARNING_PUSH` and `PYBIND11_WARNING_POP` MACROS. --- include/pybind11/pybind11.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 5219c0ff85..c52df969e6 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1382,7 +1382,17 @@ class generic_type : public object { } else { internals.registered_types_cpp[tindex] = tinfo; } + + PYBIND11_WARNING_PUSH +#if defined(__GNUC__) && __GNUC__ == 12 + // When using GCC 12 these warnings are disabled as they trigger + // false positive warnings. Discussed here: + // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115824. + PYBIND11_WARNING_DISABLE_GCC("-Warray-bounds") + PYBIND11_WARNING_DISABLE_GCC("-Wstringop-overread") +#endif internals.registered_types_py[(PyTypeObject *) m_ptr] = {tinfo}; + PYBIND11_WARNING_POP }); if (rec.bases.size() > 1 || rec.multiple_inheritance) { From 1d9483ff735cc0f7fad391f03e9db2716cf5bbfb Mon Sep 17 00:00:00 2001 From: vfdev Date: Tue, 17 Sep 2024 18:47:20 +0200 Subject: [PATCH 06/15] Added exception translator specific mutex used with try_translate_exceptions (#5362) * Added exception translator specific mutex used with try_translate_exceptions Fixes #5346 * - Replaced with_internals_for_exception_translator by with_exception_translators - Incremented PYBIND11_INTERNALS_VERSION - Added a test * style: pre-commit fixes * Fixed formatting and added explicit to ctors * Addressed PR review comments --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .../pybind11/detail/exception_translation.h | 22 +++++------ include/pybind11/detail/internals.h | 20 +++++++++- include/pybind11/pybind11.h | 21 +++++----- tests/custom_exceptions.py | 10 +++++ tests/test_exceptions.cpp | 39 +++++++++++++++++++ tests/test_exceptions.py | 5 +++ 6 files changed, 96 insertions(+), 21 deletions(-) create mode 100644 tests/custom_exceptions.py diff --git a/include/pybind11/detail/exception_translation.h b/include/pybind11/detail/exception_translation.h index 2764180bb0..22ae8a1c94 100644 --- a/include/pybind11/detail/exception_translation.h +++ b/include/pybind11/detail/exception_translation.h @@ -50,17 +50,17 @@ inline void try_translate_exceptions() { - delegate translation to the next translator by throwing a new type of exception. */ - bool handled = with_internals([&](internals &internals) { - auto &local_exception_translators = get_local_internals().registered_exception_translators; - if (detail::apply_exception_translators(local_exception_translators)) { - return true; - } - auto &exception_translators = internals.registered_exception_translators; - if (detail::apply_exception_translators(exception_translators)) { - return true; - } - return false; - }); + bool handled = with_exception_translators( + [&](std::forward_list &exception_translators, + std::forward_list &local_exception_translators) { + if (detail::apply_exception_translators(local_exception_translators)) { + return true; + } + if (detail::apply_exception_translators(exception_translators)) { + return true; + } + return false; + }); if (!handled) { set_error(PyExc_SystemError, "Exception escaped from default exception translator!"); diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 7cc6f53adf..fa224e0326 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -39,7 +39,11 @@ # if PY_VERSION_HEX >= 0x030C0000 || defined(_MSC_VER) // Version bump for Python 3.12+, before first 3.12 beta release. // Version bump for MSVC piggy-backed on PR #4779. See comments there. -# define PYBIND11_INTERNALS_VERSION 5 +# ifdef Py_GIL_DISABLED +# define PYBIND11_INTERNALS_VERSION 6 +# else +# define PYBIND11_INTERNALS_VERSION 5 +# endif # else # define PYBIND11_INTERNALS_VERSION 4 # endif @@ -177,6 +181,7 @@ static_assert(sizeof(instance_map_shard) % 64 == 0, struct internals { #ifdef Py_GIL_DISABLED pymutex mutex; + pymutex exception_translator_mutex; #endif // std::type_index -> pybind11's type information type_map registered_types_cpp; @@ -643,6 +648,19 @@ inline auto with_internals(const F &cb) -> decltype(cb(get_internals())) { return cb(internals); } +template +inline auto with_exception_translators(const F &cb) + -> decltype(cb(get_internals().registered_exception_translators, + get_local_internals().registered_exception_translators)) { + auto &internals = get_internals(); +#ifdef Py_GIL_DISABLED + std::unique_lock lock((internals).exception_translator_mutex); +#endif + auto &local_internals = get_local_internals(); + return cb(internals.registered_exception_translators, + local_internals.registered_exception_translators); +} + inline std::uint64_t mix64(std::uint64_t z) { // David Stafford's variant 13 of the MurmurHash3 finalizer popularized // by the SplitMix PRNG. diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index c52df969e6..bcdf641f43 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2586,10 +2586,12 @@ void implicitly_convertible() { } inline void register_exception_translator(ExceptionTranslator &&translator) { - detail::with_internals([&](detail::internals &internals) { - internals.registered_exception_translators.push_front( - std::forward(translator)); - }); + detail::with_exception_translators( + [&](std::forward_list &exception_translators, + std::forward_list &local_exception_translators) { + (void) local_exception_translators; + exception_translators.push_front(std::forward(translator)); + }); } /** @@ -2599,11 +2601,12 @@ inline void register_exception_translator(ExceptionTranslator &&translator) { * the exception. */ inline void register_local_exception_translator(ExceptionTranslator &&translator) { - detail::with_internals([&](detail::internals &internals) { - (void) internals; - detail::get_local_internals().registered_exception_translators.push_front( - std::forward(translator)); - }); + detail::with_exception_translators( + [&](std::forward_list &exception_translators, + std::forward_list &local_exception_translators) { + (void) exception_translators; + local_exception_translators.push_front(std::forward(translator)); + }); } /** diff --git a/tests/custom_exceptions.py b/tests/custom_exceptions.py new file mode 100644 index 0000000000..b1a092d761 --- /dev/null +++ b/tests/custom_exceptions.py @@ -0,0 +1,10 @@ +from __future__ import annotations + + +class PythonMyException7(Exception): + def __init__(self, message): + self.message = message + super().__init__(message) + + def __str__(self): + return "[PythonMyException7]: " + self.message.a diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index c1d05bb241..0a970065b4 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -111,6 +111,16 @@ struct PythonAlreadySetInDestructor { py::str s; }; +struct CustomData { + explicit CustomData(const std::string &a) : a(a) {} + std::string a; +}; + +struct MyException7 { + explicit MyException7(const CustomData &message) : message(message) {} + CustomData message; +}; + TEST_SUBMODULE(exceptions, m) { m.def("throw_std_exception", []() { throw std::runtime_error("This exception was intentionally thrown."); }); @@ -385,4 +395,33 @@ TEST_SUBMODULE(exceptions, m) { // m.def("pass_exception_void", [](const py::exception&) {}); // Does not compile. m.def("return_exception_void", []() { return py::exception(); }); + + m.def("throws7", []() { + auto data = CustomData("abc"); + throw MyException7(data); + }); + + py::class_(m, "CustomData", py::module_local()) + .def(py::init()) + .def_readwrite("a", &CustomData::a); + + PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store + PythonMyException7_storage; + PythonMyException7_storage.call_once_and_store_result([&]() { + auto mod = py::module_::import("custom_exceptions"); + py::object obj = mod.attr("PythonMyException7"); + return obj; + }); + + py::register_local_exception_translator([](std::exception_ptr p) { + try { + if (p) { + std::rethrow_exception(p); + } + } catch (const MyException7 &e) { + auto exc_type = PythonMyException7_storage.get_stored(); + py::object exc_inst = exc_type(e.message); + PyErr_SetObject(PyExc_Exception, exc_inst.ptr()); + } + }); } diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index ba5063a749..a8fd105ea0 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -3,6 +3,7 @@ import sys import pytest +from custom_exceptions import PythonMyException7 import env import pybind11_cross_module_tests as cm @@ -195,6 +196,10 @@ def test_custom(msg): raise RuntimeError("Exception error: caught child from parent") from err assert msg(excinfo.value) == "this is a helper-defined translated exception" + with pytest.raises(PythonMyException7) as excinfo: + m.throws7() + assert msg(excinfo.value) == "[PythonMyException7]: abc" + def test_nested_throws(capture): """Tests nested (e.g. C++ -> Python -> C++) exception handling""" From ad9fd39e143c8296a49a1b5b258cb6aa24e23889 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 17 Sep 2024 10:19:17 -0700 Subject: [PATCH 07/15] chore(deps): bump pypa/cibuildwheel in the actions group (#5376) Bumps the actions group with 1 update: [pypa/cibuildwheel](https://github.com/pypa/cibuildwheel). Updates `pypa/cibuildwheel` from 2.20 to 2.21 - [Release notes](https://github.com/pypa/cibuildwheel/releases) - [Changelog](https://github.com/pypa/cibuildwheel/blob/main/docs/changelog.md) - [Commits](https://github.com/pypa/cibuildwheel/compare/v2.20...v2.21) --- updated-dependencies: - dependency-name: pypa/cibuildwheel dependency-type: direct:production update-type: version-update:semver-minor dependency-group: actions ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/emscripten.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/emscripten.yaml b/.github/workflows/emscripten.yaml index 14b2b9dc7d..79c783e7fe 100644 --- a/.github/workflows/emscripten.yaml +++ b/.github/workflows/emscripten.yaml @@ -22,7 +22,7 @@ jobs: submodules: true fetch-depth: 0 - - uses: pypa/cibuildwheel@v2.20 + - uses: pypa/cibuildwheel@v2.21 env: PYODIDE_BUILD_EXPORTS: whole_archive with: From 1f8b4a7f1a1c5cc9bd6e0d63fe15540e6c458b24 Mon Sep 17 00:00:00 2001 From: Hintay Date: Fri, 20 Sep 2024 00:24:35 +0900 Subject: [PATCH 08/15] fix(cmake): `NO_EXTRAS` in `pybind11_add_module` function partially working (#5378) --- tools/pybind11NewTools.cmake | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tools/pybind11NewTools.cmake b/tools/pybind11NewTools.cmake index d3e938fab0..8bac211e14 100644 --- a/tools/pybind11NewTools.cmake +++ b/tools/pybind11NewTools.cmake @@ -274,10 +274,6 @@ function(pybind11_add_module target_name) target_link_libraries(${target_name} PRIVATE pybind11::embed) endif() - if(MSVC) - target_link_libraries(${target_name} PRIVATE pybind11::windows_extras) - endif() - # -fvisibility=hidden is required to allow multiple modules compiled against # different pybind versions to work properly, and for some features (e.g. # py::module_local). We force it on everything inside the `pybind11` From 7e418f49243bb7d13fa92cf2634af1eeac386465 Mon Sep 17 00:00:00 2001 From: gentlegiantJGC Date: Tue, 24 Sep 2024 18:28:22 +0100 Subject: [PATCH 09/15] Allow subclasses of py::args and py::kwargs (#5381) * Allow subclasses of py::args and py::kwargs The current implementation does not allow subclasses of args or kwargs. This change allows subclasses to be used. * Added test case * style: pre-commit fixes * Added missing semi-colons * style: pre-commit fixes * Added handle_type_name * Moved classes outside of function * Added namespaces * style: pre-commit fixes * Refactored tests Added more tests and moved tests to more appropriate locations. * style: pre-commit fixes --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- include/pybind11/cast.h | 4 ++-- tests/test_kwargs_and_defaults.cpp | 26 ++++++++++++++++++++++++++ tests/test_kwargs_and_defaults.py | 11 +++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 0c862e4bec..e3c8b9f659 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1570,9 +1570,9 @@ class argument_loader { using indices = make_index_sequence; template - using argument_is_args = std::is_same, args>; + using argument_is_args = std::is_base_of>; template - using argument_is_kwargs = std::is_same, kwargs>; + using argument_is_kwargs = std::is_base_of>; // Get kwargs argument position, or -1 if not present: static constexpr auto kwargs_pos = constexpr_last(); diff --git a/tests/test_kwargs_and_defaults.cpp b/tests/test_kwargs_and_defaults.cpp index bc76ec7c2d..09036ccd51 100644 --- a/tests/test_kwargs_and_defaults.cpp +++ b/tests/test_kwargs_and_defaults.cpp @@ -14,6 +14,26 @@ #include +// Classes needed for subclass test. +class ArgsSubclass : public py::args { + using py::args::args; +}; +class KWArgsSubclass : public py::kwargs { + using py::kwargs::kwargs; +}; +namespace pybind11 { +namespace detail { +template <> +struct handle_type_name { + static constexpr auto name = const_name("*Args"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("**KWArgs"); +}; +} // namespace detail +} // namespace pybind11 + TEST_SUBMODULE(kwargs_and_defaults, m) { auto kw_func = [](int x, int y) { return "x=" + std::to_string(x) + ", y=" + std::to_string(y); }; @@ -322,4 +342,10 @@ TEST_SUBMODULE(kwargs_and_defaults, m) { py::pos_only{}, py::arg("i"), py::arg("j")); + + // Test support for args and kwargs subclasses + m.def("args_kwargs_subclass_function", + [](const ArgsSubclass &args, const KWArgsSubclass &kwargs) { + return py::make_tuple(args, kwargs); + }); } diff --git a/tests/test_kwargs_and_defaults.py b/tests/test_kwargs_and_defaults.py index b9b1a7ea87..e3f758165c 100644 --- a/tests/test_kwargs_and_defaults.py +++ b/tests/test_kwargs_and_defaults.py @@ -17,6 +17,10 @@ def test_function_signatures(doc): assert ( doc(m.args_kwargs_function) == "args_kwargs_function(*args, **kwargs) -> tuple" ) + assert ( + doc(m.args_kwargs_subclass_function) + == "args_kwargs_subclass_function(*Args, **KWArgs) -> tuple" + ) assert ( doc(m.KWClass.foo0) == "foo0(self: m.kwargs_and_defaults.KWClass, arg0: int, arg1: float) -> None" @@ -98,6 +102,7 @@ def test_arg_and_kwargs(): args = "a1", "a2" kwargs = {"arg3": "a3", "arg4": 4} assert m.args_kwargs_function(*args, **kwargs) == (args, kwargs) + assert m.args_kwargs_subclass_function(*args, **kwargs) == (args, kwargs) def test_mixed_args_and_kwargs(msg): @@ -413,6 +418,12 @@ def test_args_refcount(): ) assert refcount(myval) == expected + assert m.args_kwargs_subclass_function(7, 8, myval, a=1, b=myval) == ( + (7, 8, myval), + {"a": 1, "b": myval}, + ) + assert refcount(myval) == expected + exp3 = refcount(myval, myval, myval) assert m.args_refcount(myval, myval, myval) == (exp3, exp3, exp3) assert refcount(myval) == expected From c4a05f934478f4e177665eee8e095af16e6d1af3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20=C5=A0im=C3=A1=C4=8Dek?= Date: Mon, 7 Oct 2024 23:12:04 +0200 Subject: [PATCH 10/15] Add support for GraalPy (#5380) * Initial support for GraalPy * Mark tests that currently fail on GraalPy with xfail * Add graalpy to CI * Limit test deps on graalpy to available binary wheels * Skip cmake test installed_function on GraalPy CMake won't find libpython on GraalPy, it either fails or silently picks CPython's libpython. * Factor out setting function docstrings into a macro * Try to narrow down skipped tests --- .github/workflows/ci.yml | 5 +++++ include/pybind11/cast.h | 2 +- include/pybind11/detail/common.h | 16 ++++++++++++++- include/pybind11/detail/internals.h | 5 +++-- include/pybind11/detail/type_caster_base.h | 2 +- include/pybind11/eval.h | 8 ++++---- include/pybind11/pybind11.h | 14 ++++++------- include/pybind11/pytypes.h | 4 ++-- tests/conftest.py | 4 ++-- tests/env.py | 1 + tests/requirements.txt | 13 ++++++------ tests/test_buffers.py | 4 ++++ tests/test_call_policies.cpp | 4 ++-- tests/test_call_policies.py | 5 +++++ tests/test_callbacks.cpp | 2 +- tests/test_callbacks.py | 2 ++ tests/test_class.py | 2 +- tests/test_cmake_build/CMakeLists.txt | 16 ++++++++++----- tests/test_cpp_conduit.py | 6 +++++- tests/test_custom_type_setup.py | 4 ++-- tests/test_eigen_matrix.py | 2 ++ tests/test_eigen_tensor.py | 3 +++ tests/test_embed/CMakeLists.txt | 8 +++++--- tests/test_enum.py | 3 +++ tests/test_eval.py | 2 +- tests/test_exceptions.py | 3 +++ tests/test_factory_constructors.py | 13 ++++++++++++ tests/test_gil_scoped.py | 16 +++++++++++++++ tests/test_iostream.py | 6 ++++++ tests/test_kwargs_and_defaults.py | 2 ++ tests/test_methods_and_attributes.py | 8 +++++++- tests/test_modules.py | 7 +++++-- tests/test_multiple_inheritance.py | 24 ++++++++++++++-------- tests/test_numpy_array.py | 3 ++- tests/test_operator_overloading.py | 3 +++ tests/test_pickling.py | 2 +- tests/test_pytypes.py | 8 +++++++- tests/test_sequences_and_iterators.py | 19 ++++++++++++----- tests/test_smart_ptr.py | 12 +++++++++++ tests/test_stl.py | 2 ++ tests/test_type_caster_pyobject_ptr.cpp | 3 ++- tests/test_virtual_functions.py | 9 ++++++-- 42 files changed, 211 insertions(+), 66 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b4acbbf5e4..4bd01cb7fa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,6 +39,7 @@ jobs: - 'pypy-3.8' - 'pypy-3.9' - 'pypy-3.10' + - 'graalpy-24.1' # Items in here will either be added to the build matrix (if not # present), or add new keys to an existing matrix element if all the @@ -67,6 +68,10 @@ jobs: # Extra ubuntu latest job - runs-on: ubuntu-latest python: '3.11' + exclude: + # The setup-python action currently doesn't have graalpy for windows + - python: 'graalpy-24.1' + runs-on: 'windows-2022' name: "🐍 ${{ matrix.python }} • ${{ matrix.runs-on }} • x64 ${{ matrix.args }}" diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index e3c8b9f659..c44ff29d3d 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -343,7 +343,7 @@ class type_caster { #else // Alternate approach for CPython: this does the same as the above, but optimized // using the CPython API so as to avoid an unneeded attribute lookup. - else if (auto *tp_as_number = src.ptr()->ob_type->tp_as_number) { + else if (auto *tp_as_number = Py_TYPE(src.ptr())->tp_as_number) { if (PYBIND11_NB_BOOL(tp_as_number)) { res = (*PYBIND11_NB_BOOL(tp_as_number))(src.ptr()); } diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 2a39e88f37..fa47199221 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -299,7 +299,7 @@ PYBIND11_WARNING_DISABLE_MSVC(4505) # define PYBIND11_INTERNAL_NUMPY_1_ONLY_DETECTED #endif -#if defined(PYPY_VERSION) && !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT) +#if (defined(PYPY_VERSION) || defined(GRAALVM_PYTHON)) && !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT) # define PYBIND11_SIMPLE_GIL_MANAGEMENT #endif @@ -387,6 +387,20 @@ PYBIND11_WARNING_POP #define PYBIND11_CONCAT(first, second) first##second #define PYBIND11_ENSURE_INTERNALS_READY pybind11::detail::get_internals(); +#if !defined(GRAALVM_PYTHON) +# define PYBIND11_PYCFUNCTION_GET_DOC(func) ((func)->m_ml->ml_doc) +# define PYBIND11_PYCFUNCTION_SET_DOC(func, doc) \ + do { \ + (func)->m_ml->ml_doc = (doc); \ + } while (0) +#else +# define PYBIND11_PYCFUNCTION_GET_DOC(func) (GraalPyCFunction_GetDoc((PyObject *) (func))) +# define PYBIND11_PYCFUNCTION_SET_DOC(func, doc) \ + do { \ + GraalPyCFunction_SetDoc((PyObject *) (func), (doc)); \ + } while (0) +#endif + #define PYBIND11_CHECK_PYTHON_VERSION \ { \ const char *compiled_ver \ diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index fa224e0326..330f5e1cce 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -454,7 +454,7 @@ inline void translate_local_exception(std::exception_ptr p) { inline object get_python_state_dict() { object state_dict; -#if PYBIND11_INTERNALS_VERSION <= 4 || defined(PYPY_VERSION) +#if PYBIND11_INTERNALS_VERSION <= 4 || defined(PYPY_VERSION) || defined(GRAALVM_PYTHON) state_dict = reinterpret_borrow(PyEval_GetBuiltins()); #else # if PY_VERSION_HEX < 0x03090000 @@ -727,7 +727,8 @@ const char *c_str(Args &&...args) { } inline const char *get_function_record_capsule_name() { -#if PYBIND11_INTERNALS_VERSION > 4 + // On GraalPy, pointer equality of the names is currently not guaranteed +#if PYBIND11_INTERNALS_VERSION > 4 && !defined(GRAALVM_PYTHON) return get_internals().function_record_capsule_name.c_str(); #else return nullptr; diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index e40e44ba6c..e7b94aff2a 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -459,7 +459,7 @@ PYBIND11_NOINLINE handle get_object_handle(const void *ptr, const detail::type_i } inline PyThreadState *get_thread_state_unchecked() { -#if defined(PYPY_VERSION) +#if defined(PYPY_VERSION) || defined(GRAALVM_PYTHON) return PyThreadState_GET(); #elif PY_VERSION_HEX < 0x030D0000 return _PyThreadState_UncheckedGet(); diff --git a/include/pybind11/eval.h b/include/pybind11/eval.h index 74d9b96b86..3ed1b5a4a9 100644 --- a/include/pybind11/eval.h +++ b/include/pybind11/eval.h @@ -94,18 +94,18 @@ void exec(const char (&s)[N], object global = globals(), object local = object() eval(s, std::move(global), std::move(local)); } -#if defined(PYPY_VERSION) +#if defined(PYPY_VERSION) || defined(GRAALVM_PYTHON) template object eval_file(str, object, object) { - pybind11_fail("eval_file not supported in PyPy3. Use eval"); + pybind11_fail("eval_file not supported in this interpreter. Use eval"); } template object eval_file(str, object) { - pybind11_fail("eval_file not supported in PyPy3. Use eval"); + pybind11_fail("eval_file not supported in this interpreter. Use eval"); } template object eval_file(str) { - pybind11_fail("eval_file not supported in PyPy3. Use eval"); + pybind11_fail("eval_file not supported in this interpreter. Use eval"); } #else template diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index bcdf641f43..2527d25faf 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -573,8 +573,7 @@ class cpp_function : public function { // chain. chain_start = rec; rec->next = chain; - auto rec_capsule - = reinterpret_borrow(((PyCFunctionObject *) m_ptr)->m_self); + auto rec_capsule = reinterpret_borrow(PyCFunction_GET_SELF(m_ptr)); rec_capsule.set_pointer(unique_rec.release()); guarded_strdup.release(); } else { @@ -634,12 +633,11 @@ class cpp_function : public function { } } - /* Install docstring */ auto *func = (PyCFunctionObject *) m_ptr; - std::free(const_cast(func->m_ml->ml_doc)); // Install docstring if it's non-empty (when at least one option is enabled) - func->m_ml->ml_doc - = signatures.empty() ? nullptr : PYBIND11_COMPAT_STRDUP(signatures.c_str()); + auto *doc = signatures.empty() ? nullptr : PYBIND11_COMPAT_STRDUP(signatures.c_str()); + std::free(const_cast(PYBIND11_PYCFUNCTION_GET_DOC(func))); + PYBIND11_PYCFUNCTION_SET_DOC(func, doc); if (rec->is_method) { m_ptr = PYBIND11_INSTANCE_METHOD_NEW(m_ptr, rec->scope.ptr()); @@ -2780,8 +2778,8 @@ get_type_override(const void *this_ptr, const type_info *this_type, const char * } /* Don't call dispatch code if invoked from overridden function. - Unfortunately this doesn't work on PyPy. */ -#if !defined(PYPY_VERSION) + Unfortunately this doesn't work on PyPy and GraalPy. */ +#if !defined(PYPY_VERSION) && !defined(GRAALVM_PYTHON) # if PY_VERSION_HEX >= 0x03090000 PyFrameObject *frame = PyThreadState_GetFrame(PyThreadState_Get()); if (frame != nullptr) { diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 1e76d7bc13..7aafab6dcc 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -643,7 +643,7 @@ struct error_fetch_and_normalize { bool have_trace = false; if (m_trace) { -#if !defined(PYPY_VERSION) +#if !defined(PYPY_VERSION) && !defined(GRAALVM_PYTHON) auto *tb = reinterpret_cast(m_trace.ptr()); // Get the deepest trace possible. @@ -1356,7 +1356,7 @@ inline bool PyUnicode_Check_Permissive(PyObject *o) { # define PYBIND11_STR_CHECK_FUN PyUnicode_Check #endif -inline bool PyStaticMethod_Check(PyObject *o) { return o->ob_type == &PyStaticMethod_Type; } +inline bool PyStaticMethod_Check(PyObject *o) { return Py_TYPE(o) == &PyStaticMethod_Type; } class kwargs_proxy : public handle { public: diff --git a/tests/conftest.py b/tests/conftest.py index c40b11221f..9e7ca88120 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -28,8 +28,8 @@ @pytest.fixture(scope="session", autouse=True) def use_multiprocessing_forkserver_on_linux(): - if sys.platform != "linux": - # The default on Windows and macOS is "spawn": If it's not broken, don't fix it. + if sys.platform != "linux" or sys.implementation.name == "graalpy": + # The default on Windows, macOS and GraalPy is "spawn": If it's not broken, don't fix it. return # Full background: https://github.com/pybind/pybind11/issues/4105#issuecomment-1301004592 diff --git a/tests/env.py b/tests/env.py index 9f5347f2e5..b513e455d1 100644 --- a/tests/env.py +++ b/tests/env.py @@ -12,6 +12,7 @@ CPYTHON = platform.python_implementation() == "CPython" PYPY = platform.python_implementation() == "PyPy" +GRAALPY = sys.implementation.name == "graalpy" PY_GIL_DISABLED = bool(sysconfig.get_config_var("Py_GIL_DISABLED")) diff --git a/tests/requirements.txt b/tests/requirements.txt index f4f838ec0a..688ff6fe89 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -2,11 +2,12 @@ build~=1.0; python_version>="3.8" numpy~=1.23.0; python_version=="3.8" and platform_python_implementation=="PyPy" numpy~=1.25.0; python_version=="3.9" and platform_python_implementation=='PyPy' -numpy~=1.21.5; platform_python_implementation!="PyPy" and python_version>="3.8" and python_version<"3.10" -numpy~=1.22.2; platform_python_implementation!="PyPy" and python_version=="3.10" -numpy~=1.26.0; platform_python_implementation!="PyPy" and python_version>="3.11" and python_version<"3.13" +numpy~=1.26.0; platform_python_implementation=="GraalVM" and sys_platform=="linux" +numpy~=1.21.5; platform_python_implementation!="PyPy" and platform_python_implementation!="GraalVM" and python_version>="3.8" and python_version<"3.10" +numpy~=1.22.2; platform_python_implementation!="PyPy" and platform_python_implementation!="GraalVM" and python_version=="3.10" +numpy~=1.26.0; platform_python_implementation!="PyPy" and platform_python_implementation!="GraalVM" and python_version>="3.11" and python_version<"3.13" pytest~=7.0 pytest-timeout -scipy~=1.5.4; platform_python_implementation!="PyPy" and python_version<"3.10" -scipy~=1.8.0; platform_python_implementation!="PyPy" and python_version=="3.10" and sys_platform!='win32' -scipy~=1.11.1; platform_python_implementation!="PyPy" and python_version>="3.11" and python_version<"3.13" and sys_platform!='win32' +scipy~=1.5.4; platform_python_implementation!="PyPy" and platform_python_implementation!="GraalVM" and python_version<"3.10" +scipy~=1.8.0; platform_python_implementation!="PyPy" and platform_python_implementation!="GraalVM" and python_version=="3.10" and sys_platform!='win32' +scipy~=1.11.1; platform_python_implementation!="PyPy" and platform_python_implementation!="GraalVM" and python_version>="3.11" and python_version<"3.13" and sys_platform!='win32' diff --git a/tests/test_buffers.py b/tests/test_buffers.py index 535f33c2de..1aff38ea76 100644 --- a/tests/test_buffers.py +++ b/tests/test_buffers.py @@ -82,6 +82,8 @@ def test_from_python(): for j in range(m4.cols()): assert m3[i, j] == m4[i, j] + if env.GRAALPY: + pytest.skip("ConstructorStats is incompatible with GraalPy.") cstats = ConstructorStats.get(m.Matrix) assert cstats.alive() == 1 del m3, m4 @@ -118,6 +120,8 @@ def test_to_python(): mat2[2, 3] = 5 assert mat2[2, 3] == 5 + if env.GRAALPY: + pytest.skip("ConstructorStats is incompatible with GraalPy.") cstats = ConstructorStats.get(m.Matrix) assert cstats.alive() == 1 del mat diff --git a/tests/test_call_policies.cpp b/tests/test_call_policies.cpp index 92924cb452..9140f7e9f2 100644 --- a/tests/test_call_policies.cpp +++ b/tests/test_call_policies.cpp @@ -95,8 +95,8 @@ TEST_SUBMODULE(call_policies, m) { }, py::call_guard()); -#if !defined(PYPY_VERSION) - // `py::call_guard()` should work in PyPy as well, +#if !defined(PYPY_VERSION) && !defined(GRAALVM_PYTHON) + // `py::call_guard()` should work in PyPy/GraalPy as well, // but it's unclear how to test it without `PyGILState_GetThisThreadState`. auto report_gil_status = []() { auto is_gil_held = false; diff --git a/tests/test_call_policies.py b/tests/test_call_policies.py index 91670deb37..3b614df45e 100644 --- a/tests/test_call_policies.py +++ b/tests/test_call_policies.py @@ -8,6 +8,7 @@ @pytest.mark.xfail("env.PYPY", reason="sometimes comes out 1 off on PyPy", strict=False) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_keep_alive_argument(capture): n_inst = ConstructorStats.detail_reg_inst() with capture: @@ -60,6 +61,7 @@ def test_keep_alive_argument(capture): assert str(excinfo.value) == "Could not activate keep_alive!" +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_keep_alive_return_value(capture): n_inst = ConstructorStats.detail_reg_inst() with capture: @@ -118,6 +120,7 @@ def test_keep_alive_return_value(capture): # https://foss.heptapod.net/pypy/pypy/-/issues/2447 @pytest.mark.xfail("env.PYPY", reason="_PyObject_GetDictPtr is unimplemented") +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_alive_gc(capture): n_inst = ConstructorStats.detail_reg_inst() p = m.ParentGC() @@ -137,6 +140,7 @@ def test_alive_gc(capture): ) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_alive_gc_derived(capture): class Derived(m.Parent): pass @@ -159,6 +163,7 @@ class Derived(m.Parent): ) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_alive_gc_multi_derived(capture): class Derived(m.Parent, m.Child): def __init__(self): diff --git a/tests/test_callbacks.cpp b/tests/test_callbacks.cpp index e303e76567..f28f0a9e28 100644 --- a/tests/test_callbacks.cpp +++ b/tests/test_callbacks.cpp @@ -270,7 +270,7 @@ TEST_SUBMODULE(callbacks, m) { m.add_object("custom_function", PyCFunction_New(custom_def, rec_capsule.ptr())); // This test requires a new ABI version to pass -#if PYBIND11_INTERNALS_VERSION > 4 +#if PYBIND11_INTERNALS_VERSION > 4 && !defined(GRAALVM_PYTHON) // rec_capsule with nullptr name py::capsule rec_capsule2(std::malloc(1), [](void *data) { std::free(data); }); m.add_object("custom_function2", PyCFunction_New(custom_def, rec_capsule2.ptr())); diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py index db6d8dece0..00b9015d0a 100644 --- a/tests/test_callbacks.py +++ b/tests/test_callbacks.py @@ -90,6 +90,7 @@ def f(*args, **kwargs): ) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_lambda_closure_cleanup(): m.test_lambda_closure_cleanup() cstats = m.payload_cstats() @@ -98,6 +99,7 @@ def test_lambda_closure_cleanup(): assert cstats.move_constructions >= 1 +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_cpp_callable_cleanup(): alive_counts = m.test_cpp_callable_cleanup() assert alive_counts == [0, 1, 2, 1, 2, 1, 0] diff --git a/tests/test_class.py b/tests/test_class.py index 9b2b1d8346..470d2a3269 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -361,7 +361,7 @@ def test_brace_initialization(): assert b.vec == [123, 456] -@pytest.mark.xfail("env.PYPY") +@pytest.mark.xfail("env.PYPY or env.GRAALPY") def test_class_refcount(): """Instances must correctly increase/decrease the reference count of their types (#1029)""" from sys import getrefcount diff --git a/tests/test_cmake_build/CMakeLists.txt b/tests/test_cmake_build/CMakeLists.txt index f28bde08e5..ec365c0f67 100644 --- a/tests/test_cmake_build/CMakeLists.txt +++ b/tests/test_cmake_build/CMakeLists.txt @@ -55,8 +55,10 @@ possibly_uninitialized(PYTHON_MODULE_EXTENSION Python_INTERPRETER_ID) pybind11_add_build_test(subdirectory_function) pybind11_add_build_test(subdirectory_target) -if("${PYTHON_MODULE_EXTENSION}" MATCHES "pypy" OR "${Python_INTERPRETER_ID}" STREQUAL "PyPy") - message(STATUS "Skipping embed test on PyPy") +if("${PYTHON_MODULE_EXTENSION}" MATCHES "pypy" + OR "${Python_INTERPRETER_ID}" STREQUAL "PyPy" + OR "${PYTHON_MODULE_EXTENSION}" MATCHES "graalpy") + message(STATUS "Skipping embed test on PyPy or GraalPy") else() pybind11_add_build_test(subdirectory_embed) endif() @@ -66,10 +68,14 @@ if(PYBIND11_INSTALL) mock_install ${CMAKE_COMMAND} "-DCMAKE_INSTALL_PREFIX=${pybind11_BINARY_DIR}/mock_install" -P "${pybind11_BINARY_DIR}/cmake_install.cmake") - pybind11_add_build_test(installed_function INSTALL) + if(NOT "${PYTHON_MODULE_EXTENSION}" MATCHES "graalpy") + pybind11_add_build_test(installed_function INSTALL) + endif() pybind11_add_build_test(installed_target INSTALL) - if(NOT ("${PYTHON_MODULE_EXTENSION}" MATCHES "pypy" OR "${Python_INTERPRETER_ID}" STREQUAL "PyPy" - )) + if(NOT + ("${PYTHON_MODULE_EXTENSION}" MATCHES "pypy" + OR "${Python_INTERPRETER_ID}" STREQUAL "PyPy" + OR "${PYTHON_MODULE_EXTENSION}" MATCHES "graalpy")) pybind11_add_build_test(installed_embed INSTALL) endif() endif() diff --git a/tests/test_cpp_conduit.py b/tests/test_cpp_conduit.py index 51fcf69368..eb300587fa 100644 --- a/tests/test_cpp_conduit.py +++ b/tests/test_cpp_conduit.py @@ -7,6 +7,7 @@ import home_planet_very_lonely_traveler import pytest +import env from pybind11_tests import cpp_conduit as home_planet @@ -27,7 +28,10 @@ def test_call_cpp_conduit_success(): home_planet.cpp_type_info_capsule_Traveler, b"raw_pointer_ephemeral", ) - assert cap.__class__.__name__ == "PyCapsule" + assert cap.__class__.__name__ == "PyCapsule" or ( + # Note: this will become unnecessary in the next GraalPy release + env.GRAALPY and cap.__class__.__name__ == "capsule" + ) def test_call_cpp_conduit_platform_abi_id_mismatch(): diff --git a/tests/test_custom_type_setup.py b/tests/test_custom_type_setup.py index ca3340bd53..bb2865cade 100644 --- a/tests/test_custom_type_setup.py +++ b/tests/test_custom_type_setup.py @@ -34,7 +34,7 @@ def add_ref(obj): # PyPy does not seem to reliably garbage collect. -@pytest.mark.skipif("env.PYPY") +@pytest.mark.skipif("env.PYPY or env.GRAALPY") def test_self_cycle(gc_tester): obj = m.OwnsPythonObjects() obj.value = obj @@ -42,7 +42,7 @@ def test_self_cycle(gc_tester): # PyPy does not seem to reliably garbage collect. -@pytest.mark.skipif("env.PYPY") +@pytest.mark.skipif("env.PYPY or env.GRAALPY") def test_indirect_cycle(gc_tester): obj = m.OwnsPythonObjects() obj_list = [obj] diff --git a/tests/test_eigen_matrix.py b/tests/test_eigen_matrix.py index e1d7433f15..7baf999536 100644 --- a/tests/test_eigen_matrix.py +++ b/tests/test_eigen_matrix.py @@ -2,6 +2,7 @@ import pytest +import env # noqa: F401 from pybind11_tests import ConstructorStats np = pytest.importorskip("numpy") @@ -409,6 +410,7 @@ def assert_keeps_alive(cl, method, *args): assert cstats.alive() == start_with +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_eigen_keepalive(): a = m.ReturnTester() cstats = ConstructorStats.get(m.ReturnTester) diff --git a/tests/test_eigen_tensor.py b/tests/test_eigen_tensor.py index a2b99d9d7d..0860c1dad7 100644 --- a/tests/test_eigen_tensor.py +++ b/tests/test_eigen_tensor.py @@ -4,6 +4,8 @@ import pytest +import env # noqa: F401 + np = pytest.importorskip("numpy") eigen_tensor = pytest.importorskip("pybind11_tests.eigen_tensor") submodules = [eigen_tensor.c_style, eigen_tensor.f_style] @@ -61,6 +63,7 @@ def assert_equal_tensor_ref(mat, writeable=True, modified=None): @pytest.mark.parametrize("m", submodules) @pytest.mark.parametrize("member_name", ["member", "member_view"]) +@pytest.mark.skipif("env.GRAALPY", reason="Different refcounting mechanism") def test_reference_internal(m, member_name): if not hasattr(sys, "getrefcount"): pytest.skip("No reference counting") diff --git a/tests/test_embed/CMakeLists.txt b/tests/test_embed/CMakeLists.txt index 9b539cd42d..f646458d1e 100644 --- a/tests/test_embed/CMakeLists.txt +++ b/tests/test_embed/CMakeLists.txt @@ -1,8 +1,10 @@ possibly_uninitialized(PYTHON_MODULE_EXTENSION Python_INTERPRETER_ID) -if("${PYTHON_MODULE_EXTENSION}" MATCHES "pypy" OR "${Python_INTERPRETER_ID}" STREQUAL "PyPy") - message(STATUS "Skipping embed test on PyPy") - add_custom_target(cpptest) # Dummy target on PyPy. Embedding is not supported. +if("${PYTHON_MODULE_EXTENSION}" MATCHES "pypy" + OR "${Python_INTERPRETER_ID}" STREQUAL "PyPy" + OR "${PYTHON_MODULE_EXTENSION}" MATCHES "graalpy") + message(STATUS "Skipping embed test on PyPy or GraalPy") + add_custom_target(cpptest) # Dummy target on PyPy or GraalPy. Embedding is not supported. set(_suppress_unused_variable_warning "${DOWNLOAD_CATCH}") return() endif() diff --git a/tests/test_enum.py b/tests/test_enum.py index 9914b90013..03cd1c1a64 100644 --- a/tests/test_enum.py +++ b/tests/test_enum.py @@ -3,9 +3,11 @@ import pytest +import env # noqa: F401 from pybind11_tests import enums as m +@pytest.mark.xfail("env.GRAALPY", reason="TODO should get fixed on GraalPy side") def test_unscoped_enum(): assert str(m.UnscopedEnum.EOne) == "UnscopedEnum.EOne" assert str(m.UnscopedEnum.ETwo) == "UnscopedEnum.ETwo" @@ -193,6 +195,7 @@ def test_implicit_conversion(): assert repr(x) == "{: 3, : 4}" +@pytest.mark.xfail("env.GRAALPY", reason="TODO should get fixed on GraalPy side") def test_binary_operators(): assert int(m.Flags.Read) == 4 assert int(m.Flags.Write) == 2 diff --git a/tests/test_eval.py b/tests/test_eval.py index 45b68ece72..8ac1907c7a 100644 --- a/tests/test_eval.py +++ b/tests/test_eval.py @@ -19,7 +19,7 @@ def test_evals(capture): assert m.test_eval_failure() -@pytest.mark.xfail("env.PYPY", raises=RuntimeError) +@pytest.mark.xfail("env.PYPY or env.GRAALPY", raises=RuntimeError) def test_eval_file(): filename = os.path.join(os.path.dirname(__file__), "test_eval_call.py") assert m.test_eval_file(filename) diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index a8fd105ea0..21449d58ce 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -201,6 +201,7 @@ def test_custom(msg): assert msg(excinfo.value) == "[PythonMyException7]: abc" +@pytest.mark.xfail("env.GRAALPY", reason="TODO should get fixed on GraalPy side") def test_nested_throws(capture): """Tests nested (e.g. C++ -> Python -> C++) exception handling""" @@ -369,6 +370,7 @@ def _test_flaky_exception_failure_point_init_py_3_12(): "env.PYPY and sys.version_info[:2] < (3, 12)", reason="PyErr_NormalizeException Segmentation fault", ) +@pytest.mark.xfail("env.GRAALPY", reason="TODO should be fixed on GraalPy side") def test_flaky_exception_failure_point_init(): if sys.version_info[:2] < (3, 12): _test_flaky_exception_failure_point_init_before_py_3_12() @@ -376,6 +378,7 @@ def test_flaky_exception_failure_point_init(): _test_flaky_exception_failure_point_init_py_3_12() +@pytest.mark.xfail("env.GRAALPY", reason="TODO should be fixed on GraalPy side") def test_flaky_exception_failure_point_str(): what, py_err_set_after_what = m.error_already_set_what( FlakyException, ("failure_point_str",) diff --git a/tests/test_factory_constructors.py b/tests/test_factory_constructors.py index 0ddad5e323..1d3a9bcddd 100644 --- a/tests/test_factory_constructors.py +++ b/tests/test_factory_constructors.py @@ -4,11 +4,13 @@ import pytest +import env # noqa: F401 from pybind11_tests import ConstructorStats from pybind11_tests import factory_constructors as m from pybind11_tests.factory_constructors import tag +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_init_factory_basic(): """Tests py::init_factory() wrapper around various ways of returning the object""" @@ -102,6 +104,7 @@ def test_init_factory_signature(msg): ) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_init_factory_casting(): """Tests py::init_factory() wrapper with various upcasting and downcasting returns""" @@ -150,6 +153,7 @@ def test_init_factory_casting(): ] +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_init_factory_alias(): """Tests py::init_factory() wrapper with value conversions and alias types""" @@ -220,6 +224,7 @@ def get(self): ] +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_init_factory_dual(): """Tests init factory functions with dual main/alias factory functions""" from pybind11_tests.factory_constructors import TestFactory7 @@ -302,6 +307,7 @@ def get(self): ] +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_no_placement_new(capture): """Prior to 2.2, `py::init<...>` relied on the type supporting placement new; this tests a class without placement new support.""" @@ -350,6 +356,7 @@ def strip_comments(s): return re.sub(r"\s+#.*", "", s) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_reallocation_a(capture, msg): """When the constructor is overloaded, previous overloads can require a preallocated value. This test makes sure that such preallocated values only happen when they might be necessary, @@ -372,6 +379,7 @@ def test_reallocation_a(capture, msg): ) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_reallocation_b(capture, msg): with capture: create_and_destroy(1.5) @@ -388,6 +396,7 @@ def test_reallocation_b(capture, msg): ) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_reallocation_c(capture, msg): with capture: create_and_destroy(2, 3) @@ -402,6 +411,7 @@ def test_reallocation_c(capture, msg): ) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_reallocation_d(capture, msg): with capture: create_and_destroy(2.5, 3) @@ -417,6 +427,7 @@ def test_reallocation_d(capture, msg): ) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_reallocation_e(capture, msg): with capture: create_and_destroy(3.5, 4.5) @@ -432,6 +443,7 @@ def test_reallocation_e(capture, msg): ) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_reallocation_f(capture, msg): with capture: create_and_destroy(4, 0.5) @@ -448,6 +460,7 @@ def test_reallocation_f(capture, msg): ) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_reallocation_g(capture, msg): with capture: create_and_destroy(5, "hi") diff --git a/tests/test_gil_scoped.py b/tests/test_gil_scoped.py index eab92093c8..257e66612f 100644 --- a/tests/test_gil_scoped.py +++ b/tests/test_gil_scoped.py @@ -211,6 +211,10 @@ def _run_in_threads(test_fn, num_threads, parallel): @pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") @pytest.mark.parametrize("test_fn", ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK) +@pytest.mark.skipif( + "env.GRAALPY", + reason="GraalPy transiently complains about unfinished threads at process exit", +) def test_run_in_process_one_thread(test_fn): """Makes sure there is no GIL deadlock when running in a thread. @@ -221,6 +225,10 @@ def test_run_in_process_one_thread(test_fn): @pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") @pytest.mark.parametrize("test_fn", ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK) +@pytest.mark.skipif( + "env.GRAALPY", + reason="GraalPy transiently complains about unfinished threads at process exit", +) def test_run_in_process_multiple_threads_parallel(test_fn): """Makes sure there is no GIL deadlock when running in a thread multiple times in parallel. @@ -231,6 +239,10 @@ def test_run_in_process_multiple_threads_parallel(test_fn): @pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") @pytest.mark.parametrize("test_fn", ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK) +@pytest.mark.skipif( + "env.GRAALPY", + reason="GraalPy transiently complains about unfinished threads at process exit", +) def test_run_in_process_multiple_threads_sequential(test_fn): """Makes sure there is no GIL deadlock when running in a thread multiple times sequentially. @@ -241,6 +253,10 @@ def test_run_in_process_multiple_threads_sequential(test_fn): @pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") @pytest.mark.parametrize("test_fn", ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK) +@pytest.mark.skipif( + "env.GRAALPY", + reason="GraalPy transiently complains about unfinished threads at process exit", +) def test_run_in_process_direct(test_fn): """Makes sure there is no GIL deadlock when using processes. diff --git a/tests/test_iostream.py b/tests/test_iostream.py index c3d987787a..606028d6fe 100644 --- a/tests/test_iostream.py +++ b/tests/test_iostream.py @@ -6,8 +6,14 @@ import pytest +import env # noqa: F401 from pybind11_tests import iostream as m +pytestmark = pytest.mark.skipif( + "env.GRAALPY", + reason="Delayed prints from finalizers from other tests can end up in the output", +) + def test_captured(capsys): msg = "I've been redirected to Python, I hope!" diff --git a/tests/test_kwargs_and_defaults.py b/tests/test_kwargs_and_defaults.py index e3f758165c..e3e9a0a0d1 100644 --- a/tests/test_kwargs_and_defaults.py +++ b/tests/test_kwargs_and_defaults.py @@ -2,6 +2,7 @@ import pytest +import env # noqa: F401 from pybind11_tests import kwargs_and_defaults as m @@ -383,6 +384,7 @@ def test_signatures(): ) +@pytest.mark.skipif("env.GRAALPY", reason="Different refcounting mechanism") def test_args_refcount(): """Issue/PR #1216 - py::args elements get double-inc_ref()ed when combined with regular arguments""" diff --git a/tests/test_methods_and_attributes.py b/tests/test_methods_and_attributes.py index dfa31f5465..91c7b7751e 100644 --- a/tests/test_methods_and_attributes.py +++ b/tests/test_methods_and_attributes.py @@ -4,7 +4,7 @@ import pytest -import env # noqa: F401 +import env from pybind11_tests import ConstructorStats from pybind11_tests import methods_and_attributes as m @@ -68,6 +68,9 @@ def test_methods_and_attributes(): instance1.value = 100 assert str(instance1) == "ExampleMandA[value=100]" + if env.GRAALPY: + pytest.skip("ConstructorStats is incompatible with GraalPy.") + cstats = ConstructorStats.get(m.ExampleMandA) assert cstats.alive() == 2 del instance1, instance2 @@ -316,6 +319,8 @@ def test_dynamic_attributes(): instance.__dict__ = [] assert str(excinfo.value) == "__dict__ must be set to a dictionary, not a 'list'" + if env.GRAALPY: + pytest.skip("ConstructorStats is incompatible with GraalPy.") cstats = ConstructorStats.get(m.DynamicClass) assert cstats.alive() == 1 del instance @@ -337,6 +342,7 @@ class PythonDerivedDynamicClass(m.DynamicClass): # https://foss.heptapod.net/pypy/pypy/-/issues/2447 @pytest.mark.xfail("env.PYPY") +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_cyclic_gc(): # One object references itself instance = m.DynamicClass() diff --git a/tests/test_modules.py b/tests/test_modules.py index 95835e14ea..436271a701 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -39,6 +39,9 @@ def test_reference_internal(): assert str(b.get_a2()) == "A[43]" assert str(b.a2) == "A[43]" + if env.GRAALPY: + pytest.skip("ConstructorStats is incompatible with GraalPy.") + astats, bstats = ConstructorStats.get(ms.A), ConstructorStats.get(ms.B) assert astats.alive() == 2 assert bstats.alive() == 1 @@ -97,9 +100,9 @@ def test_def_submodule_failures(): sm = m.def_submodule(m, b"ScratchSubModuleName") # Using bytes to show it works. assert sm.__name__ == m.__name__ + "." + "ScratchSubModuleName" malformed_utf8 = b"\x80" - if env.PYPY: + if env.PYPY or env.GRAALPY: # It is not worth the effort finding a trigger for a failure when running with PyPy. - pytest.skip("Sufficiently exercised on platforms other than PyPy.") + pytest.skip("Sufficiently exercised on platforms other than PyPy/GraalPy.") else: # Meant to trigger PyModule_GetName() failure: sm_name_orig = sm.__name__ diff --git a/tests/test_multiple_inheritance.py b/tests/test_multiple_inheritance.py index d445824b54..6f5a656f5d 100644 --- a/tests/test_multiple_inheritance.py +++ b/tests/test_multiple_inheritance.py @@ -2,7 +2,7 @@ import pytest -import env # noqa: F401 +import env from pybind11_tests import ConstructorStats from pybind11_tests import multiple_inheritance as m @@ -279,8 +279,9 @@ def test_mi_unaligned_base(): c = m.I801C() d = m.I801D() - # + 4 below because we have the two instances, and each instance has offset base I801B2 - assert ConstructorStats.detail_reg_inst() == n_inst + 4 + if not env.GRAALPY: + # + 4 below because we have the two instances, and each instance has offset base I801B2 + assert ConstructorStats.detail_reg_inst() == n_inst + 4 b1c = m.i801b1_c(c) assert b1c is c b2c = m.i801b2_c(c) @@ -290,6 +291,9 @@ def test_mi_unaligned_base(): b2d = m.i801b2_d(d) assert b2d is d + if env.GRAALPY: + pytest.skip("ConstructorStats is incompatible with GraalPy.") + assert ConstructorStats.detail_reg_inst() == n_inst + 4 # no extra instances del c, b1c, b2c assert ConstructorStats.detail_reg_inst() == n_inst + 2 @@ -312,7 +316,8 @@ def test_mi_base_return(): assert d1.a == 1 assert d1.b == 2 - assert ConstructorStats.detail_reg_inst() == n_inst + 4 + if not env.GRAALPY: + assert ConstructorStats.detail_reg_inst() == n_inst + 4 c2 = m.i801c_b2() assert type(c2) is m.I801C @@ -324,12 +329,13 @@ def test_mi_base_return(): assert d2.a == 1 assert d2.b == 2 - assert ConstructorStats.detail_reg_inst() == n_inst + 8 + if not env.GRAALPY: + assert ConstructorStats.detail_reg_inst() == n_inst + 8 - del c2 - assert ConstructorStats.detail_reg_inst() == n_inst + 6 - del c1, d1, d2 - assert ConstructorStats.detail_reg_inst() == n_inst + del c2 + assert ConstructorStats.detail_reg_inst() == n_inst + 6 + del c1, d1, d2 + assert ConstructorStats.detail_reg_inst() == n_inst # Returning an unregistered derived type with a registered base; we won't # pick up the derived type, obviously, but should still work (as an object diff --git a/tests/test_numpy_array.py b/tests/test_numpy_array.py index bc7b3d5557..6e8bde826f 100644 --- a/tests/test_numpy_array.py +++ b/tests/test_numpy_array.py @@ -242,6 +242,7 @@ def assert_references(a, b, base=None): assert_references(a1m, a2, a1) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_numpy_view(capture): with capture: ac = m.ArrayClass() @@ -465,7 +466,7 @@ def test_array_resize(): assert b.shape == (8, 8) -@pytest.mark.xfail("env.PYPY") +@pytest.mark.xfail("env.PYPY or env.GRAALPY") def test_array_create_and_resize(): a = m.create_and_resize(2) assert a.size == 4 diff --git a/tests/test_operator_overloading.py b/tests/test_operator_overloading.py index b6760902dc..22300eb0f4 100644 --- a/tests/test_operator_overloading.py +++ b/tests/test_operator_overloading.py @@ -2,10 +2,12 @@ import pytest +import env # noqa: F401 from pybind11_tests import ConstructorStats from pybind11_tests import operators as m +@pytest.mark.xfail("env.GRAALPY", reason="TODO should get fixed on GraalPy side") def test_operator_overloading(): v1 = m.Vector2(1, 2) v2 = m.Vector(3, -1) @@ -83,6 +85,7 @@ def test_operator_overloading(): assert cstats.move_assignments == 0 +@pytest.mark.xfail("env.GRAALPY", reason="TODO should get fixed on GraalPy side") def test_operators_notimplemented(): """#393: need to return NotSupported to ensure correct arithmetic operator behavior""" diff --git a/tests/test_pickling.py b/tests/test_pickling.py index ad67a1df98..3e2e453969 100644 --- a/tests/test_pickling.py +++ b/tests/test_pickling.py @@ -20,7 +20,7 @@ def test_pickle_simple_callable(): # all C Python versions. with pytest.raises(TypeError) as excinfo: pickle.dumps(m.simple_callable) - assert re.search("can.*t pickle .*PyCapsule.* object", str(excinfo.value)) + assert re.search("can.*t pickle .*[Cc]apsule.* object", str(excinfo.value)) @pytest.mark.parametrize("cls_name", ["Pickleable", "PickleableNew"]) diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 6f015eec83..39d0b619b8 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -262,6 +262,7 @@ def __repr__(self): m.str_from_std_string_input, ], ) +@pytest.mark.xfail("env.GRAALPY", reason="TODO should be fixed on GraalPy side") def test_surrogate_pairs_unicode_error(func): input_str = "\ud83d\ude4f".encode("utf-8", "surrogatepass") with pytest.raises(UnicodeDecodeError): @@ -420,6 +421,7 @@ def test_accessor_moves(): pytest.skip("Not defined: PYBIND11_HANDLE_REF_DEBUG") +@pytest.mark.xfail("env.GRAALPY", reason="TODO should be fixed on GraalPy side") def test_constructors(): """C++ default and converting constructors are equivalent to type calls in Python""" types = [bytes, bytearray, str, bool, int, float, tuple, list, dict, set] @@ -712,6 +714,7 @@ def test_pass_bytes_or_unicode_to_string_types(): m.pass_to_pybind11_str(malformed_utf8) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") @pytest.mark.parametrize( ("create_weakref", "create_weakref_with_callback"), [ @@ -765,7 +768,10 @@ def callback(_): ob = C() # Should raise TypeError on CPython - with pytest.raises(TypeError) if not env.PYPY else contextlib.nullcontext(): + cm = pytest.raises(TypeError) + if env.PYPY or env.GRAALPY: + cm = contextlib.nullcontext() + with cm: _ = create_weakref(ob, callback) if has_callback else create_weakref(ob) diff --git a/tests/test_sequences_and_iterators.py b/tests/test_sequences_and_iterators.py index f609f553de..20abd29d59 100644 --- a/tests/test_sequences_and_iterators.py +++ b/tests/test_sequences_and_iterators.py @@ -3,6 +3,7 @@ import pytest from pytest import approx # noqa: PT013 +import env from pybind11_tests import ConstructorStats from pybind11_tests import sequences_and_iterators as m @@ -110,7 +111,8 @@ def test_sequence(): cstats = ConstructorStats.get(m.Sequence) s = m.Sequence(5) - assert cstats.values() == ["of size", "5"] + if not env.GRAALPY: + assert cstats.values() == ["of size", "5"] assert "Sequence" in repr(s) assert len(s) == 5 @@ -123,16 +125,19 @@ def test_sequence(): assert s[3] == approx(56.78, rel=1e-05) rev = reversed(s) - assert cstats.values() == ["of size", "5"] + if not env.GRAALPY: + assert cstats.values() == ["of size", "5"] rev2 = s[::-1] - assert cstats.values() == ["of size", "5"] + if not env.GRAALPY: + assert cstats.values() == ["of size", "5"] it = iter(m.Sequence(0)) for _ in range(3): # __next__ must continue to raise StopIteration with pytest.raises(StopIteration): next(it) - assert cstats.values() == ["of size", "0"] + if not env.GRAALPY: + assert cstats.values() == ["of size", "0"] expected = [0, 56.78, 0, 0, 12.34] assert rev == approx(expected, rel=1e-05) @@ -140,10 +145,14 @@ def test_sequence(): assert rev == rev2 rev[0::2] = m.Sequence([2.0, 2.0, 2.0]) - assert cstats.values() == ["of size", "3", "from std::vector"] + if not env.GRAALPY: + assert cstats.values() == ["of size", "3", "from std::vector"] assert rev == approx([2, 56.78, 2, 0, 2], rel=1e-05) + if env.GRAALPY: + pytest.skip("ConstructorStats is incompatible with GraalPy.") + assert cstats.alive() == 4 del it assert cstats.alive() == 3 diff --git a/tests/test_smart_ptr.py b/tests/test_smart_ptr.py index bf0ae4aeb1..9729004ffc 100644 --- a/tests/test_smart_ptr.py +++ b/tests/test_smart_ptr.py @@ -2,10 +2,13 @@ import pytest +import env # noqa: F401 + m = pytest.importorskip("pybind11_tests.smart_ptr") from pybind11_tests import ConstructorStats # noqa: E402 +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_smart_ptr(capture): # Object1 for i, o in enumerate( @@ -118,6 +121,7 @@ def test_smart_ptr_refcounting(): assert m.test_object1_refcounting() +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_unique_nodelete(): o = m.MyObject4(23) assert o.value == 23 @@ -129,6 +133,7 @@ def test_unique_nodelete(): assert cstats.alive() == 0 +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_unique_nodelete4a(): o = m.MyObject4a(23) assert o.value == 23 @@ -140,6 +145,7 @@ def test_unique_nodelete4a(): assert cstats.alive() == 0 +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_unique_deleter(): m.MyObject4a(0) o = m.MyObject4b(23) @@ -156,6 +162,7 @@ def test_unique_deleter(): assert cstats4b.alive() == 0 +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_large_holder(): o = m.MyObject5(5) assert o.value == 5 @@ -165,6 +172,7 @@ def test_large_holder(): assert cstats.alive() == 0 +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_shared_ptr_and_references(): s = m.SharedPtrRef() stats = ConstructorStats.get(m.A) @@ -196,6 +204,7 @@ def test_shared_ptr_and_references(): assert stats.alive() == 0 +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_shared_ptr_from_this_and_references(): s = m.SharedFromThisRef() stats = ConstructorStats.get(m.B) @@ -242,6 +251,7 @@ def test_shared_ptr_from_this_and_references(): assert y is z +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_move_only_holder(): a = m.TypeWithMoveOnlyHolder.make() b = m.TypeWithMoveOnlyHolder.make_as_object() @@ -253,6 +263,7 @@ def test_move_only_holder(): assert stats.alive() == 0 +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_holder_with_addressof_operator(): # this test must not throw exception from c++ a = m.TypeForHolderWithAddressOf.make() @@ -283,6 +294,7 @@ def test_holder_with_addressof_operator(): assert stats.alive() == 0 +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_move_only_holder_with_addressof_operator(): a = m.TypeForMoveOnlyHolderWithAddressOf.make() a.print_object() diff --git a/tests/test_stl.py b/tests/test_stl.py index 6f548789ed..d1a9ff08b0 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -2,6 +2,7 @@ import pytest +import env # noqa: F401 from pybind11_tests import ConstructorStats, UserType from pybind11_tests import stl as m @@ -362,6 +363,7 @@ def test_function_with_string_and_vector_string_arg(): assert m.func_with_string_or_vector_string_arg_overload("A") == 3 +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_stl_ownership(): cstats = ConstructorStats.get(m.Placeholder) assert cstats.alive() == 0 diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index a45c08b648..758c4ee09c 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -37,7 +37,8 @@ struct WithPyObjectPtrReturnTrampoline : WithPyObjectPtrReturn { std::string call_return_pyobject_ptr(const WithPyObjectPtrReturn *base_class_ptr) { PyObject *returned_obj = base_class_ptr->return_pyobject_ptr(); -#if !defined(PYPY_VERSION) // It is not worth the trouble doing something special for PyPy. +// It is not worth the trouble doing something special for PyPy/GraalPy +#if !defined(PYPY_VERSION) && !defined(GRAALVM_PYTHON) if (Py_REFCNT(returned_obj) != 1) { py::pybind11_fail(__FILE__ ":" PYBIND11_TOSTRING(__LINE__)); } diff --git a/tests/test_virtual_functions.py b/tests/test_virtual_functions.py index bc0177787a..617c87b8e0 100644 --- a/tests/test_virtual_functions.py +++ b/tests/test_virtual_functions.py @@ -4,7 +4,7 @@ import pytest -import env # noqa: F401 +import env m = pytest.importorskip("pybind11_tests.virtual_functions") from pybind11_tests import ConstructorStats # noqa: E402 @@ -82,6 +82,9 @@ def get_string2(self): """ ) + if env.GRAALPY: + pytest.skip("ConstructorStats is incompatible with GraalPy.") + cstats = ConstructorStats.get(m.ExampleVirt) assert cstats.alive() == 3 del ex12, ex12p, ex12p2 @@ -91,6 +94,7 @@ def get_string2(self): assert cstats.move_constructions >= 0 +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_alias_delay_initialization1(capture): """`A` only initializes its trampoline class when we inherit from it @@ -130,6 +134,7 @@ def f(self): ) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_alias_delay_initialization2(capture): """`A2`, unlike the above, is configured to always initialize the alias @@ -188,7 +193,7 @@ def f(self): # PyPy: Reference count > 1 causes call with noncopyable instance # to fail in ncv1.print_nc() -@pytest.mark.xfail("env.PYPY") +@pytest.mark.xfail("env.PYPY or env.GRAALPY") @pytest.mark.skipif( not hasattr(m, "NCVirt"), reason="NCVirt does not work on Intel/PGI/NVCC compilers" ) From 56e69a20a5b4ab6d31a262dde5ef723a322033a5 Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin <15985472+sarlinpe@users.noreply.github.com> Date: Tue, 8 Oct 2024 19:49:35 +0200 Subject: [PATCH 11/15] Print key in KeyError in map.__getitem__/__delitem__ (#5397) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Print key in map.getitem/delitem KeyError * Add tests * Fix tests * Make robust * Make clang-tidy happy * Return a Python str * Show beginning and end of the message * Avoid implicit conversion * Split out `format_message_key_error_key_object()` to reduce amount of templated code. * Use `"✄✄✄"` instead of `"..."` Also rename variable to `cut_length`, to not get into even/odd issues with the meaning of "half". --------- Co-authored-by: Ralf W. Grosse-Kunstleve --- include/pybind11/stl_bind.h | 40 +++++++++++++++++++++++++++++++++++-- tests/test_stl_binders.py | 19 ++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h index fcb48dea33..af3a47f39c 100644 --- a/include/pybind11/stl_bind.h +++ b/include/pybind11/stl_bind.h @@ -694,6 +694,40 @@ struct ItemsViewImpl : public detail::items_view { Map ↦ }; +inline str format_message_key_error_key_object(handle py_key) { + str message = "pybind11::bind_map key"; + if (!py_key) { + return message; + } + try { + message = str(py_key); + } catch (const std::exception &) { + try { + message = repr(py_key); + } catch (const std::exception &) { + return message; + } + } + const ssize_t cut_length = 100; + if (len(message) > 2 * cut_length + 3) { + return str(message[slice(0, cut_length, 1)]) + str("✄✄✄") + + str(message[slice(-cut_length, static_cast(len(message)), 1)]); + } + return message; +} + +template +str format_message_key_error(const KeyType &key) { + object py_key; + try { + py_key = cast(key); + } catch (const std::exception &) { + do { // Trick to avoid "empty catch" warning/error. + } while (false); + } + return format_message_key_error_key_object(py_key); +} + PYBIND11_NAMESPACE_END(detail) template , typename... Args> @@ -785,7 +819,8 @@ class_ bind_map(handle scope, const std::string &name, Args && [](Map &m, const KeyType &k) -> MappedType & { auto it = m.find(k); if (it == m.end()) { - throw key_error(); + set_error(PyExc_KeyError, detail::format_message_key_error(k)); + throw error_already_set(); } return it->second; }, @@ -808,7 +843,8 @@ class_ bind_map(handle scope, const std::string &name, Args && cl.def("__delitem__", [](Map &m, const KeyType &k) { auto it = m.find(k); if (it == m.end()) { - throw key_error(); + set_error(PyExc_KeyError, detail::format_message_key_error(k)); + throw error_already_set(); } m.erase(it); }); diff --git a/tests/test_stl_binders.py b/tests/test_stl_binders.py index 09e784e2eb..9856ba462b 100644 --- a/tests/test_stl_binders.py +++ b/tests/test_stl_binders.py @@ -302,6 +302,25 @@ def test_map_delitem(): assert list(mm) == ["b"] assert list(mm.items()) == [("b", 2.5)] + with pytest.raises(KeyError) as excinfo: + mm["a_long_key"] + assert "a_long_key" in str(excinfo.value) + + with pytest.raises(KeyError) as excinfo: + del mm["a_long_key"] + assert "a_long_key" in str(excinfo.value) + + cut_length = 100 + k_very_long = "ab" * cut_length + "xyz" + with pytest.raises(KeyError) as excinfo: + mm[k_very_long] + assert k_very_long in str(excinfo.value) + k_very_long += "@" + with pytest.raises(KeyError) as excinfo: + mm[k_very_long] + k_repr = k_very_long[:cut_length] + "✄✄✄" + k_very_long[-cut_length:] + assert k_repr in str(excinfo.value) + um = m.UnorderedMapStringDouble() um["ua"] = 1.1 um["ub"] = 2.6 From af67e87393b0f867ccffc2702885eea12de063fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20K=C3=B6ppe?= Date: Tue, 8 Oct 2024 18:51:20 +0100 Subject: [PATCH 12/15] docs/advanced A document about deadlock potential with C++ statics (#5394) * [docs/advanced] A document about deadlock potential with C++ statics * [docs/advanced] Refer to deadlock.md from misc.rst * [docs/advanced] Fix tables in deadlock.md * Use :ref:`deadlock-reference-label` * Revert "Use :ref:`deadlock-reference-label`" This reverts commit e5734d275fa0d38ad4a58a595797353813e65df1. * Add simple references to docs/advanced/deadlock.md filename. (Maybe someone can work on clickable links later.) --------- Co-authored-by: Ralf W. Grosse-Kunstleve --- docs/advanced/deadlock.md | 391 ++++++++++++++++++++++++++ docs/advanced/misc.rst | 9 +- include/pybind11/gil_safe_call_once.h | 2 + 3 files changed, 401 insertions(+), 1 deletion(-) create mode 100644 docs/advanced/deadlock.md diff --git a/docs/advanced/deadlock.md b/docs/advanced/deadlock.md new file mode 100644 index 0000000000..f1bab5bdb0 --- /dev/null +++ b/docs/advanced/deadlock.md @@ -0,0 +1,391 @@ +# Double locking, deadlocking, GIL + +[TOC] + +## Introduction + +### Overview + +In concurrent programming with locks, *deadlocks* can arise when more than one +mutex is locked at the same time, and careful attention has to be paid to lock +ordering to avoid this. Here we will look at a common situation that occurs in +native extensions for CPython written in C++. + +### Deadlocks + +A deadlock can occur when more than one thread attempts to lock more than one +mutex, and two of the threads lock two of the mutexes in different orders. For +example, consider mutexes `mu1` and `mu2`, and threads T1 and T2, executing: + +| | T1 | T2 | +|--- | ------------------- | -------------------| +|1 | `mu1.lock()`{.good} | `mu2.lock()`{.good}| +|2 | `mu2.lock()`{.bad} | `mu1.lock()`{.bad} | +|3 | `/* work */` | `/* work */` | +|4 | `mu2.unlock()` | `mu1.unlock()` | +|5 | `mu1.unlock()` | `mu2.unlock()` | + +Now if T1 manages to lock `mu1` and T2 manages to lock `mu2` (as indicated in +green), then both threads will block while trying to lock the respective other +mutex (as indicated in red), but they are also unable to release the mutex that +they have locked (step 5). + +**The problem** is that it is possible for one thread to attempt to lock `mu1` +and then `mu2`, and for another thread to attempt to lock `mu2` and then `mu1`. +Note that it does not matter if either mutex is unlocked at any intermediate +point; what matters is only the order of any attempt to *lock* the mutexes. For +example, the following, more complex series of operations is just as prone to +deadlock: + +| | T1 | T2 | +|--- | ------------------- | -------------------| +|1 | `mu1.lock()`{.good} | `mu1.lock()`{.good}| +|2 | waiting for T2 | `mu2.lock()`{.good}| +|3 | waiting for T2 | `/* work */` | +|3 | waiting for T2 | `mu1.unlock()` | +|3 | `mu2.lock()`{.bad} | `/* work */` | +|3 | `/* work */` | `mu1.lock()`{.bad} | +|3 | `/* work */` | `/* work */` | +|4 | `mu2.unlock()` | `mu1.unlock()` | +|5 | `mu1.unlock()` | `mu2.unlock()` | + +When the mutexes involved in a locking sequence are known at compile-time, then +avoiding deadlocks is “merely” a matter of arranging the lock +operations carefully so as to only occur in one single, fixed order. However, it +is also possible for mutexes to only be determined at runtime. A typical example +of this is a database where each row has its own mutex. An operation that +modifies two rows in a single transaction (e.g. “transferring an amount +from one account to another”) must lock two row mutexes, but the locking +order cannot be established at compile time. In this case, a dynamic +“deadlock avoidance algorithm” is needed. (In C++, `std::lock` +provides such an algorithm. An algorithm might use a non-blocking `try_lock` +operation on a mutex, which can either succeed or fail to lock the mutex, but +returns without blocking.) + +Conceptually, one could also consider it a deadlock if _the same_ thread +attempts to lock a mutex that it has already locked (e.g. when some locked +operation accidentally recurses into itself): `mu.lock();`{.good} +`mu.lock();`{.bad} However, this is a slightly separate issue: Typical mutexes +are either of _recursive_ or _non-recursive_ kind. A recursive mutex allows +repeated locking and requires balanced unlocking. A non-recursive mutex can be +implemented more efficiently, and/but for efficiency reasons does not actually +guarantee a deadlock on second lock. Instead, the API simply forbids such use, +making it a precondition that the thread not already hold the mutex, with +undefined behaviour on violation. + +### “Once” initialization + +A common programming problem is to have an operation happen precisely once, even +if requested concurrently. While it is clear that we need to track in some +shared state somewhere whether the operation has already happened, it is worth +noting that this state only ever transitions, once, from `false` to `true`. This +is considerably simpler than a general shared state that can change values +arbitrarily. Next, we also need a mechanism for all but one thread to block +until the initialization has completed, which we can provide with a mutex. The +simplest solution just always locks the mutex: + +```c++ +// The "once" mechanism: +constinit absl::Mutex mu(absl::kConstInit); +constinit bool init_done = false; + +// The operation of interest: +void f(); + +void InitOnceNaive() { + absl::MutexLock lock(&mu); + if (!init_done) { + f(); + init_done = true; + } +} +``` + +This works, but the efficiency-minded reader will observe that once the +operation has completed, all future lock contention on the mutex is +unnecessary. This leads to the (in)famous “double-locking” +algorithm, which was historically hard to write correctly. The idea is to check +the boolean *before* locking the mutex, and avoid locking if the operation has +already completed. However, accessing shared state concurrently when at least +one access is a write is prone to causing a data race and needs to be done +according to an appropriate concurrent programming model. In C++ we use atomic +variables: + +```c++ +// The "once" mechanism: +constinit absl::Mutex mu(absl::kConstInit); +constinit std::atomic init_done = false; + +// The operation of interest: +void f(); + +void InitOnceWithFastPath() { + if (!init_done.load(std::memory_order_acquire)) { + absl::MutexLock lock(&mu); + if (!init_done.load(std::memory_order_relaxed)) { + f(); + init_done.store(true, std::memory_order_release); + } + } +} +``` + +Checking the flag now happens without holding the mutex lock, and if the +operation has already completed, we return immediately. After locking the mutex, +we need to check the flag again, since multiple threads can reach this point. + +*Atomic details.* Since the atomic flag variable is accessed concurrently, we +have to think about the memory order of the accesses. There are two separate +cases: The first, outer check outside the mutex lock, and the second, inner +check under the lock. The outer check and the flag update form an +acquire/release pair: *if* the load sees the value `true` (which must have been +written by the store operation), then it also sees everything that happened +before the store, namely the operation `f()`. By contrast, the inner check can +use relaxed memory ordering, since in that case the mutex operations provide the +necessary ordering: if the inner load sees the value `true`, it happened after +the `lock()`, which happened after the `unlock()`, which happened after the +store. + +The C++ standard library, and Abseil, provide a ready-made solution of this +algorithm called `std::call_once`/`absl::call_once`. (The interface is the same, +but the Abseil implementation is possibly better.) + +```c++ +// The "once" mechanism: +constinit absl::once_flag init_flag; + +// The operation of interest: +void f(); + +void InitOnceWithCallOnce() { + absl::call_once(once_flag, f); +} +``` + +Even though conceptually this is performing the same algorithm, this +implementation has some considerable advantages: The `once_flag` type is a small +and trivial, integer-like type and is trivially destructible. Not only does it +take up less space than a mutex, it also generates less code since it does not +have to run a destructor, which would need to be added to the program's global +destructor list. + +The final clou comes with the C++ semantics of a `static` variable declared at +block scope: According to [[stmt.dcl]](https://eel.is/c++draft/stmt.dcl#3): + +> Dynamic initialization of a block variable with static storage duration or +> thread storage duration is performed the first time control passes through its +> declaration; such a variable is considered initialized upon the completion of +> its initialization. [...] If control enters the declaration concurrently while +> the variable is being initialized, the concurrent execution shall wait for +> completion of the initialization. + +This is saying that the initialization of a local, `static` variable precisely +has the “once” semantics that we have been discussing. We can +therefore write the above example as follows: + +```c++ +// The operation of interest: +void f(); + +void InitOnceWithStatic() { + static int unused = (f(), 0); +} +``` + +This approach is by far the simplest and easiest, but the big difference is that +the mutex (or mutex-like object) in this implementation is no longer visible or +in the user’s control. This is perfectly fine if the initializer is +simple, but if the initializer itself attempts to lock any other mutex +(including by initializing another static variable!), then we have no control +over the lock ordering! + +Finally, you may have noticed the `constinit`s around the earlier code. Both +`constinit` and `constexpr` specifiers on a declaration mean that the variable +is *constant-initialized*, which means that no initialization is performed at +runtime (the initial value is already known at compile time). This in turn means +that a static variable guard mutex may not be needed, and static initialization +never blocks. The difference between the two is that a `constexpr`-specified +variable is also `const`, and a variable cannot be `constexpr` if it has a +non-trivial destructor. Such a destructor also means that the guard mutex is +needed after all, since the destructor must be registered to run at exit, +conditionally on initialization having happened. + +## Python, CPython, GIL + +With CPython, a Python program can call into native code. To this end, the +native code registers callback functions with the Python runtime via the CPython +API. In order to ensure that the internal state of the Python runtime remains +consistent, there is a single, shared mutex called the “global interpreter +lock”, or GIL for short. Upon entry of one of the user-provided callback +functions, the GIL is locked (or “held”), so that no other mutations +of the Python runtime state can occur until the native callback returns. + +Many native extensions do not interact with the Python runtime for at least some +part of them, and so it is common for native extensions to _release_ the GIL, do +some work, and then reacquire the GIL before returning. Similarly, when code is +generally not holding the GIL but needs to interact with the runtime briefly, it +will first reacquire the GIL. The GIL is reentrant, and constructions to acquire +and subsequently release the GIL are common, and often don't worry about whether +the GIL is already held. + +If the native code is written in C++ and contains local, `static` variables, +then we are now dealing with at least _two_ mutexes: the static variable guard +mutex, and the GIL from CPython. + +A common problem in such code is an operation with “only once” +semantics that also ends up requiring the GIL to be held at some point. As per +the above description of “once”-style techniques, one might find a +static variable: + +```c++ +// CPython callback, assumes that the GIL is held on entry. +PyObject* InvokeWidget(PyObject* self) { + static PyObject* impl = CreateWidget(); + return PyObject_CallOneArg(impl, self); +} +``` + +This seems reasonable, but bear in mind that there are two mutexes (the "guard +mutex" and "the GIL"), and we must think about the lock order. Otherwise, if the +callback is called from multiple threads, a deadlock may ensue. + +Let us consider what we can see here: On entry, the GIL is already locked, and +we are locking the guard mutex. This is one lock order. Inside the initializer +`CreateWidget`, with both mutexes already locked, the function can freely access +the Python runtime. + +However, it is entirely possible that `CreateWidget` will want to release the +GIL at one point and reacquire it later: + +```c++ +// Assumes that the GIL is held on entry. +// Ensures that the GIL is held on exit. +PyObject* CreateWidget() { + // ... + Py_BEGIN_ALLOW_THREADS // releases GIL + // expensive work, not accessing the Python runtime + Py_END_ALLOW_THREADS // acquires GIL, #! + // ... + return result; +} +``` + +Now we have a second lock order: the guard mutex is locked, and then the GIL is +locked (at `#!`). To see how this deadlocks, consider threads T1 and T2 both +having the runtime attempt to call `InvokeWidget`. T1 locks the GIL and +proceeds, locking the guard mutex and calling `CreateWidget`; T2 is blocked +waiting for the GIL. Then T1 releases the GIL to do “expensive +work”, and T2 awakes and locks the GIL. Now T2 is blocked trying to +acquire the guard mutex, but T1 is blocked reacquiring the GIL (at `#!`). + +In other words: if we want to support “once-called” functions that +can arbitrarily release and reacquire the GIL, as is very common, then the only +lock order that we can ensure is: guard mutex first, GIL second. + +To implement this, we must rewrite our code. Naively, we could always release +the GIL before a `static` variable with blocking initializer: + +```c++ +// CPython callback, assumes that the GIL is held on entry. +PyObject* InvokeWidget(PyObject* self) { + Py_BEGIN_ALLOW_THREADS // releases GIL + static PyObject* impl = CreateWidget(); + Py_END_ALLOW_THREADS // acquires GIL + + return PyObject_CallOneArg(impl, self); +} +``` + +But similar to the `InitOnceNaive` example above, this code cycles the GIL +(possibly descheduling the thread) even when the static variable has already +been initialized. If we want to avoid this, we need to abandon the use of a +static variable, since we do not control the guard mutex well enough. Instead, +we use an operation whose mutex locking is under our control, such as +`call_once`. For example: + +```c++ +// CPython callback, assumes that the GIL is held on entry. +PyObject* InvokeWidget(PyObject* self) { + static constinit PyObject* impl = nullptr; + static constinit std::atomic init_done = false; + static constinit absl::once_flag init_flag; + + if (!init_done.load(std::memory_order_acquire)) { + Py_BEGIN_ALLOW_THREADS // releases GIL + absl::call_once(init_flag, [&]() { + PyGILState_STATE s = PyGILState_Ensure(); // acquires GIL + impl = CreateWidget(); + PyGILState_Release(s); // releases GIL + init_done.store(true, std::memory_order_release); + }); + Py_END_ALLOW_THREADS // acquires GIL + } + + return PyObject_CallOneArg(impl, self); +} +``` + +The lock order is now always guard mutex first, GIL second. Unfortunately we +have to duplicate the “double-checked done flag”, effectively +leading to triple checking, because the flag state inside the `absl::once_flag` +is not accessible to the user. In other words, we cannot ask `init_flag` whether +it has been used yet. + +However, we can perform one last, minor optimisation: since we assume that the +GIL is held on entry, and again when the initializing operation returns, the GIL +actually serializes access to our done flag variable, which therefore does not +need to be atomic. (The difference to the previous, atomic code may be small, +depending on the architecture. For example, on x86-64, acquire/release on a bool +is nearly free ([demo](https://godbolt.org/z/P9vYWf4fE)).) + +```c++ +// CPython callback, assumes that the GIL is held on entry, and indeed anywhere +// directly in this function (i.e. the GIL can be released inside CreateWidget, +// but must be reaqcuired when that call returns). +PyObject* InvokeWidget(PyObject* self) { + static constinit PyObject* impl = nullptr; + static constinit bool init_done = false; // guarded by GIL + static constinit absl::once_flag init_flag; + + if (!init_done) { + Py_BEGIN_ALLOW_THREADS // releases GIL + // (multiple threads may enter here) + absl::call_once(init_flag, [&]() { + // (only one thread enters here) + PyGILState_STATE s = PyGILState_Ensure(); // acquires GIL + impl = CreateWidget(); + init_done = true; // (GIL is held) + PyGILState_Release(s); // releases GIL + }); + + Py_END_ALLOW_THREADS // acquires GIL + } + + return PyObject_CallOneArg(impl, self); +} +``` + +## Debugging tips + +* Build with symbols. +* Ctrl-C sends `SIGINT`, Ctrl-\\ + sends `SIGQUIT`. Both have their uses. +* Useful `gdb` commands: + * `py-bt` prints a Python backtrace if you are in a Python frame. + * `thread apply all bt 10` prints the top-10 frames for each thread. A + full backtrace can be prohibitively expensive, and the top few frames + are often good enough. + * `p PyGILState_Check()` shows whether a thread is holding the GIL. For + all threads, run `thread apply all p PyGILState_Check()` to find out + which thread is holding the GIL. + * The `static` variable guard mutex is accessed with functions like + `cxa_guard_acquire` (though this depends on ABI details and can vary). + The guard mutex itself contains information about which thread is + currently holding it. + +## Links + +* Article on + [double-checked locking](https://preshing.com/20130930/double-checked-locking-is-fixed-in-cpp11/) +* [The Deadlock Empire](https://deadlockempire.github.io/), hands-on exercises + to construct deadlocks diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index ddd7f39370..a256da54a9 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -62,7 +62,11 @@ will acquire the GIL before calling the Python callback. Similarly, the back into Python. When writing C++ code that is called from other C++ code, if that code accesses -Python state, it must explicitly acquire and release the GIL. +Python state, it must explicitly acquire and release the GIL. A separate +document on deadlocks [#f8]_ elaborates on a particularly subtle interaction +with C++'s block-scope static variable initializer guard mutexes. + +.. [#f8] See docs/advanced/deadlock.md The classes :class:`gil_scoped_release` and :class:`gil_scoped_acquire` can be used to acquire and release the global interpreter lock in the body of a C++ @@ -142,6 +146,9 @@ following checklist. destructors can sometimes get invoked in weird and unexpected circumstances as a result of exceptions. +- C++ static block-scope variable initialization that calls back into Python can + cause deadlocks; see [#f8]_ for a detailed discussion. + - You should try running your code in a debug build. That will enable additional assertions within pybind11 that will throw exceptions on certain GIL handling errors (reference counting operations). diff --git a/include/pybind11/gil_safe_call_once.h b/include/pybind11/gil_safe_call_once.h index 5f9e1b03c6..44e68f0294 100644 --- a/include/pybind11/gil_safe_call_once.h +++ b/include/pybind11/gil_safe_call_once.h @@ -46,6 +46,8 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) // get processed only when it is the main thread's turn again and it is running // normal Python code. However, this will be unnoticeable for quick call-once // functions, which is usually the case. +// +// For in-depth background, see docs/advanced/deadlock.md template class gil_safe_call_once_and_store { public: From f2907651fadc54c89492d4da03a120faa7b9260d Mon Sep 17 00:00:00 2001 From: Boris Dalstein Date: Sat, 12 Oct 2024 05:33:13 +0200 Subject: [PATCH 13/15] Fix #5399: iterator increment operator does not skip first item (#5400) * Fix #5399: iterator increment operator does not skip first item * Fix postfix increment operator: init() must be called before copying *this --- include/pybind11/pytypes.h | 20 +++++++++++++++----- tests/test_pytypes.cpp | 12 ++++++++++++ tests/test_pytypes.py | 5 +++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 7aafab6dcc..027e36098b 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1470,11 +1470,17 @@ class iterator : public object { PYBIND11_OBJECT_DEFAULT(iterator, object, PyIter_Check) iterator &operator++() { + init(); advance(); return *this; } iterator operator++(int) { + // Note: We must call init() first so that rv.value is + // the same as this->value just before calling advance(). + // Otherwise, dereferencing the returned iterator may call + // advance() again and return the 3rd item instead of the 1st. + init(); auto rv = *this; advance(); return rv; @@ -1482,15 +1488,12 @@ class iterator : public object { // NOLINTNEXTLINE(readability-const-return-type) // PR #3263 reference operator*() const { - if (m_ptr && !value.ptr()) { - auto &self = const_cast(*this); - self.advance(); - } + init(); return value; } pointer operator->() const { - operator*(); + init(); return &value; } @@ -1513,6 +1516,13 @@ class iterator : public object { friend bool operator!=(const iterator &a, const iterator &b) { return a->ptr() != b->ptr(); } private: + void init() const { + if (m_ptr && !value.ptr()) { + auto &self = const_cast(*this); + self.advance(); + } + } + void advance() { value = reinterpret_steal(PyIter_Next(m_ptr)); if (value.ptr() == nullptr && PyErr_Occurred()) { diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 19f65ce7eb..8df4cdd3f6 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -150,6 +150,18 @@ TEST_SUBMODULE(pytypes, m) { m.def("get_iterator", [] { return py::iterator(); }); // test_iterable m.def("get_iterable", [] { return py::iterable(); }); + m.def("get_first_item_from_iterable", [](const py::iterable &iter) { + // This tests the postfix increment operator + py::iterator it = iter.begin(); + py::iterator it2 = it++; + return *it2; + }); + m.def("get_second_item_from_iterable", [](const py::iterable &iter) { + // This tests the prefix increment operator + py::iterator it = iter.begin(); + ++it; + return *it; + }); m.def("get_frozenset_from_iterable", [](const py::iterable &iter) { return py::frozenset(iter); }); m.def("get_list_from_iterable", [](const py::iterable &iter) { return py::list(iter); }); diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 39d0b619b8..9fd24b34f1 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -52,6 +52,11 @@ def test_from_iterable(pytype, from_iter_func): def test_iterable(doc): assert doc(m.get_iterable) == "get_iterable() -> Iterable" + lins = [1, 2, 3] + i = m.get_first_item_from_iterable(lins) + assert i == 1 + i = m.get_second_item_from_iterable(lins) + assert i == 2 def test_float(doc): From 077e49fcd6d6e38bbde63b3b824b02487209e9fa Mon Sep 17 00:00:00 2001 From: cyyever Date: Sat, 12 Oct 2024 11:36:41 +0800 Subject: [PATCH 14/15] Export libc++ exceptions (#5390) * Export libc++ exceptions * Remove emscripten limit * Remove __apple_build_version__ condition from PYBIND11_EXPORT_EXCEPTION * Add a comment --- include/pybind11/detail/common.h | 19 +++++++++++-------- tests/test_exceptions.py | 2 +- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index fa47199221..c974d89959 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -164,14 +164,6 @@ # endif #endif -#if !defined(PYBIND11_EXPORT_EXCEPTION) -# if defined(__apple_build_version__) -# define PYBIND11_EXPORT_EXCEPTION PYBIND11_EXPORT -# else -# define PYBIND11_EXPORT_EXCEPTION -# endif -#endif - // For CUDA, GCC7, GCC8: // PYBIND11_NOINLINE_FORCED is incompatible with `-Wattributes -Werror`. // When defining PYBIND11_NOINLINE_FORCED, it is best to also use `-Wno-attributes`. @@ -329,6 +321,17 @@ PYBIND11_WARNING_POP # endif #endif +// For libc++, the exceptions should be exported, +// otherwise, the exception translation would be incorrect. +// IMPORTANT: This code block must stay BELOW the #include above (see PR #5390). +#if !defined(PYBIND11_EXPORT_EXCEPTION) +# if defined(_LIBCPP_EXCEPTION) +# define PYBIND11_EXPORT_EXCEPTION PYBIND11_EXPORT +# else +# define PYBIND11_EXPORT_EXCEPTION +# endif +#endif + // Must be after including or one of the other headers specified by the standard #if defined(__cpp_lib_char8_t) && __cpp_lib_char8_t >= 201811L # define PYBIND11_HAS_U8STRING diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 21449d58ce..47214a7029 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -76,7 +76,7 @@ def test_cross_module_exceptions(msg): # TODO: FIXME @pytest.mark.xfail( - "env.MACOS and (env.PYPY or pybind11_tests.compiler_info.startswith('Homebrew Clang')) or sys.platform.startswith('emscripten')", + "env.MACOS and env.PYPY", raises=RuntimeError, reason="See Issue #2847, PR #2999, PR #4324", ) From f7e14e985be167ca158fd3ee2fe5d8a4f175fa87 Mon Sep 17 00:00:00 2001 From: Francesco Ballarin Date: Sat, 12 Oct 2024 20:19:50 +0200 Subject: [PATCH 15/15] Address regression introduced in #5381 (#5396) * Incomplete attempt to address regression introduced in #5381 * style: pre-commit fixes * Revert "style: pre-commit fixes" This reverts commit 9d107d2f751d76b2c26e90cd08d2ac163022c873. * Revert "Incomplete attempt to address regression introduced in #5381" This reverts commit 8cf1cdbc96ac326ff6d64a36ea291472f74c016f. * Simpler fix for the regression introduced in #5381 * style: pre-commit fixes * Added if constexpr workaround This can probably be done better but at least this is a start. * style: pre-commit fixes * Replace if constexpr with template struct if constexpr was not added until C++ 17. I think this should do the same thing as before. * style: pre-commit fixes * Made comment clearer * Added test cases * style: pre-commit fixes * Fixed is_same_or_base_of reference * style: pre-commit fixes * Added static assert messages * style: pre-commit fixes * Replaced typedef with using * style: pre-commit fixes * Back out `ForwardClassPtr` (to be discussed separately). Tested locally with clang-tidy. * Shuffle new `static_assert()` and leave error messages blank (they are more distracting than helpful here). --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: gentlegiantJGC Co-authored-by: Ralf W. Grosse-Kunstleve --- include/pybind11/cast.h | 15 ++++++++++++--- tests/test_class.cpp | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index c44ff29d3d..f6a7e83be8 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1564,15 +1564,24 @@ struct function_call { handle init_self; }; +// See PR #5396 for the discussion that led to this +template +struct is_same_or_base_of : std::is_same {}; + +// Only evaluate is_base_of if Derived is complete. +// is_base_of raises a compiler error if Derived is incomplete. +template +struct is_same_or_base_of + : any_of, std::is_base_of> {}; + /// Helper class which loads arguments for C++ functions called from Python template class argument_loader { using indices = make_index_sequence; - template - using argument_is_args = std::is_base_of>; + using argument_is_args = is_same_or_base_of>; template - using argument_is_kwargs = std::is_base_of>; + using argument_is_kwargs = is_same_or_base_of>; // Get kwargs argument position, or -1 if not present: static constexpr auto kwargs_pos = constexpr_last(); diff --git a/tests/test_class.cpp b/tests/test_class.cpp index 9001d86b19..cb84c327a0 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -52,8 +52,24 @@ void bind_empty0(py::module_ &m) { } } // namespace pr4220_tripped_over_this + +namespace pr5396_forward_declared_class { +class ForwardClass; +class Args : public py::args {}; +} // namespace pr5396_forward_declared_class + } // namespace test_class +static_assert(py::detail::is_same_or_base_of::value, ""); +static_assert( + py::detail::is_same_or_base_of::value, + ""); +static_assert(!py::detail::is_same_or_base_of< + py::args, + test_class::pr5396_forward_declared_class::ForwardClass>::value, + ""); + TEST_SUBMODULE(class_, m) { m.def("obj_class_name", [](py::handle obj) { return py::detail::obj_class_name(obj.ptr()); });