From 243b1790a5c9ddfa54957565a559e3c2eff93cac Mon Sep 17 00:00:00 2001 From: Matthias Wittgen Date: Thu, 31 Jul 2025 18:21:27 -0700 Subject: [PATCH] Switch to pybind11 version 3 and use smart holders. --- include/lsst/cpputils/python.h | 2 +- include/lsst/cpputils/python/PySharedPtr.h | 94 ------------------- tests/inheritance.cc | 11 +-- ...st_pySharedPtr.py => test_smart_holder.py} | 7 +- 4 files changed, 10 insertions(+), 104 deletions(-) delete mode 100644 include/lsst/cpputils/python/PySharedPtr.h rename tests/{test_pySharedPtr.py => test_smart_holder.py} (95%) diff --git a/include/lsst/cpputils/python.h b/include/lsst/cpputils/python.h index 9f598f8..386c81f 100644 --- a/include/lsst/cpputils/python.h +++ b/include/lsst/cpputils/python.h @@ -53,7 +53,7 @@ Add `__eq__` and `__ne__` methods based on two std::shared_ptr pointing to th lsst::afw::table records are considered equal if two `std::shared_ptr` point to the same record. This is wrapped as follows for `lsst::afw::table::BaseRecord`, where `cls` is an instance of -`pybind11::class_>)`: +`pybind11::classh)`: utils::addSharedPtrEquality(cls); diff --git a/include/lsst/cpputils/python/PySharedPtr.h b/include/lsst/cpputils/python/PySharedPtr.h deleted file mode 100644 index 12bc178..0000000 --- a/include/lsst/cpputils/python/PySharedPtr.h +++ /dev/null @@ -1,94 +0,0 @@ -// -*- lsst-c++ -*- -/* - * LSST Data Management System - * See COPYRIGHT file at the top of the source tree. - * - * This product includes software developed by the - * LSST Project (http://www.lsst.org/). - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the LSST License Statement and - * the GNU General Public License along with this program. If not, - * see . - */ - -#ifndef LSST_CPPUTILS_PYTHON_PYSHAREDPTR_H -#define LSST_CPPUTILS_PYTHON_PYSHAREDPTR_H - -#include "pybind11/pybind11.h" - -#include - -namespace lsst { -namespace cpputils { -namespace python { - -/* Workaround for C++ objects outliving Python objects - * by Xfel and florianwechsung, https://github.com/pybind/pybind11/issues/1389 - */ -/** - * A shared pointer that tracks both a C++ object and its associated PyObject. - * - * Each group of PySharedPtr for a given object collectively counts as one - * reference to that object for the purpose of Python garbage collection. - * - * A PySharedPtr is implicitly convertible to and from a std::shared_ptr to - * minimize API impact. Any `shared_ptr` created this way will (I think) keep - * the Python reference alive, as described above. - */ -template -class PySharedPtr final { -public: - using element_type = T; - - /** - * Create a pointer that counts as an extra reference in the - * Python environment. - * - * @param ptr a pointer to a pybind11-managed object. - */ - explicit PySharedPtr(T* const ptr) : _impl() { - if (ptr != nullptr) { - // `cast` returns new PyObject only if `*ptr` not yet associated with one - PyObject* pyObj = pybind11::cast(ptr).ptr(); - Py_INCREF(pyObj); - // if any operation with shared_ptr fails, pyObj will get decremented correctly - std::shared_ptr manager(pyObj, [](PyObject* const obj) noexcept { - if (obj != nullptr) Py_DECREF(obj); - }); - _impl = std::shared_ptr(manager, ptr); - } - } - - PySharedPtr(PySharedPtr const&) noexcept = default; - PySharedPtr(PySharedPtr&&) noexcept = default; - PySharedPtr& operator=(PySharedPtr const&) noexcept = default; - PySharedPtr& operator=(PySharedPtr&&) noexcept = default; - ~PySharedPtr() noexcept = default; - - PySharedPtr(std::shared_ptr r) noexcept : _impl(std::move(r)) {} - operator std::shared_ptr() noexcept { return _impl; } - - T* get() const noexcept { return _impl.get(); } - -private: - std::shared_ptr _impl; -}; - -} // namespace python -} // namespace cpputils -} // namespace lsst - -// Macro must be called in the global namespace -PYBIND11_DECLARE_HOLDER_TYPE(T, lsst::cpputils::python::PySharedPtr); - -#endif diff --git a/tests/inheritance.cc b/tests/inheritance.cc index 3983488..c2435d0 100644 --- a/tests/inheritance.cc +++ b/tests/inheritance.cc @@ -24,8 +24,6 @@ #include #include -#include "lsst/cpputils/python/PySharedPtr.h" - namespace py = pybind11; using namespace pybind11::literals; @@ -41,6 +39,7 @@ class CppBase { std::string nonOverridable() const noexcept { return "42"; } virtual std::string overridable() const { return ""; } virtual std::string abstract() const = 0; + virtual ~CppBase() = default; }; class CppDerived : public CppBase { @@ -50,7 +49,7 @@ class CppDerived : public CppBase { }; template -class Trampoline : public Base { +class Trampoline : public Base, pybind11::trampoline_self_life_support { public: using Base::Base; @@ -72,11 +71,11 @@ std::string printFromCpp(CppBase const& obj) { } PYBIND11_MODULE(_inheritance, mod) { - py::class_, PySharedPtr>(mod, "CppBase").def(py::init<>()); - py::class_, CppBase, PySharedPtr>(mod, "CppDerived") + py::classh>(mod, "CppBase").def(py::init<>()); + py::classh, CppBase>(mod, "CppDerived") .def(py::init<>()); - py::class_>(mod, "CppStorage") + py::classh(mod, "CppStorage") .def(py::init>()); mod.def("getFromStorage", &getFromStorage, "holder"_a); diff --git a/tests/test_pySharedPtr.py b/tests/test_smart_holder.py similarity index 95% rename from tests/test_pySharedPtr.py rename to tests/test_smart_holder.py index 54bc185..abaa727 100644 --- a/tests/test_pySharedPtr.py +++ b/tests/test_smart_holder.py @@ -25,8 +25,8 @@ import _inheritance -class PySharedPtrTestSuite(unittest.TestCase): - """Test the ability of PySharedPtr to safely pass hybrid objects +class SmartHolderTestSuite(unittest.TestCase): + """Test the ability of pybind11 to safely pass hybrid objects between C++ and Python.""" class PyDerived(_inheritance.CppBase): @@ -52,7 +52,8 @@ def abstract(self): return "py-abstract" def checkGarbageCollection(self, concreteClass, returns): - """Generic test for whether a C++/Python class survives garbage collection. + """Generic test for whether a C++/Python class survives garbage + collection. Parameters ----------