From ca77130be8ec51519cac21655afe77cdb39904c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Wed, 30 Dec 2015 21:03:57 +0100 Subject: [PATCH] Use object class to hold partially converted python objects. Using object class to hold converted object automatically deallocates object if an exception is thrown or scope is left before constructing complete Python object. Additionally added method object::release() that allows to release ownership of python object without decreasing its reference count. --- docs/reference.rst | 8 ++++- include/pybind11/cast.h | 71 ++++++++++++++++---------------------- include/pybind11/pytypes.h | 9 +++-- include/pybind11/stl.h | 46 +++++++++++------------- 4 files changed, 63 insertions(+), 71 deletions(-) diff --git a/docs/reference.rst b/docs/reference.rst index bf7a8e276..2bc62b5f0 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -59,7 +59,7 @@ Without reference counting Creates a :class:`handle` from the given raw Python object pointer. -.. function:: PyObject * handle::ptr() +.. function:: PyObject * handle::ptr() const Return the ``PyObject *`` underlying a :class:`handle`. @@ -167,6 +167,12 @@ With reference counting Move constructor; steals the object from ``other`` and preserves its reference count. +.. function:: PyObject* object::release() + + Release ownership of underlying ``PyObject *``. Returns raw Python object + pointer without decreasing its reference count and resets handle to + ``nullptr``-valued pointer. + .. function:: object::~object() Constructor, which automatically calls :func:`handle::dec_ref()`. diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 22f7ffbf6..a89103421 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -398,16 +398,15 @@ public: } static PyObject *cast(const type &src, return_value_policy policy, PyObject *parent) { - PyObject *o1 = type_caster::type>::cast(src.first, policy, parent); - PyObject *o2 = type_caster::type>::cast(src.second, policy, parent); - if (!o1 || !o2) { - Py_XDECREF(o1); - Py_XDECREF(o2); + object o1(type_caster::type>::cast(src.first, policy, parent), false); + object o2(type_caster::type>::cast(src.second, policy, parent), false); + if (!o1 || !o2) return nullptr; - } PyObject *tuple = PyTuple_New(2); - PyTuple_SetItem(tuple, 0, o1); - PyTuple_SetItem(tuple, 1, o2); + if (!tuple) + return nullptr; + PyTuple_SetItem(tuple, 0, o1.release()); + PyTuple_SetItem(tuple, 1, o2.release()); return tuple; } @@ -502,25 +501,19 @@ protected: /* Implementation: Convert a C++ tuple into a Python tuple */ template static PyObject *cast(const type &src, return_value_policy policy, PyObject *parent, index_sequence) { - std::array results {{ - type_caster::type>::cast(std::get(src), policy, parent)... + std::array results {{ + object(type_caster::type>::cast(std::get(src), policy, parent), false)... }}; - bool success = true; - for (auto result : results) - if (result == nullptr) - success = false; - if (success) { - PyObject *tuple = PyTuple_New(size); - int counter = 0; - for (auto result : results) - PyTuple_SetItem(tuple, counter++, result); - return tuple; - } else { - for (auto result : results) { - Py_XDECREF(result); - } + for (const auto & result : results) + if (!result) + return nullptr; + PyObject *tuple = PyTuple_New(size); + if (!tuple) return nullptr; - } + int counter = 0; + for (auto & result : results) + PyTuple_SetItem(tuple, counter++, result.release()); + return tuple; } protected: @@ -600,26 +593,20 @@ template <> inline void handle::cast() const { return; } template inline object handle::call(Args&&... args_) { const size_t size = sizeof...(Args); - std::array args{ - { detail::type_caster::type>::cast( - std::forward(args_), return_value_policy::reference, nullptr)... } + std::array args{ + { object(detail::type_caster::type>::cast( + std::forward(args_), return_value_policy::reference, nullptr), false)... } }; - bool fail = false; - for (auto result : args) - if (result == nullptr) - fail = true; - if (fail) { - for (auto result : args) { - Py_XDECREF(result); - } + for (const auto & result : args) + if (!result) + throw cast_error("handle::call(): unable to convert input arguments to Python objects"); + object tuple(PyTuple_New(size), false); + if (!tuple) throw cast_error("handle::call(): unable to convert input arguments to Python objects"); - } - PyObject *tuple = PyTuple_New(size); int counter = 0; - for (auto result : args) - PyTuple_SetItem(tuple, counter++, result); - PyObject *result = PyObject_CallObject(m_ptr, tuple); - Py_DECREF(tuple); + for (auto & result : args) + PyTuple_SetItem(tuple.ptr(), counter++, result.release()); + PyObject *result = PyObject_CallObject(m_ptr, tuple.ptr()); if (result == nullptr && PyErr_Occurred()) throw error_already_set(); return object(result, false); diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index c0bb47b0b..eedb73d96 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -29,8 +29,7 @@ public: handle() : m_ptr(nullptr) { } handle(const handle &other) : m_ptr(other.m_ptr) { } handle(PyObject *ptr) : m_ptr(ptr) { } - PyObject *ptr() { return m_ptr; } - const PyObject *ptr() const { return m_ptr; } + PyObject *ptr() const { return m_ptr; } void inc_ref() const { Py_XINCREF(m_ptr); } void dec_ref() const { Py_XDECREF(m_ptr); } int ref_count() const { return (int) Py_REFCNT(m_ptr); } @@ -60,6 +59,12 @@ public: object(object &&other) { m_ptr = other.m_ptr; other.m_ptr = nullptr; } ~object() { dec_ref(); } + PyObject * release() { + PyObject *tmp = m_ptr; + m_ptr = nullptr; + return tmp; + } + object& operator=(object &other) { Py_XINCREF(other.m_ptr); Py_XDECREF(m_ptr); diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index cee5be72d..c4f4b196c 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -43,17 +43,17 @@ public: } static PyObject *cast(const type &src, return_value_policy policy, PyObject *parent) { - PyObject *list = PyList_New(src.size()); + object list(PyList_New(src.size()), false); + if (!list) + return nullptr; size_t index = 0; for (auto const &value: src) { - PyObject *value_ = value_conv::cast(value, policy, parent); - if (!value_) { - Py_DECREF(list); + object value_ (value_conv::cast(value, policy, parent), false); + if (!value_) return nullptr; - } - PyList_SET_ITEM(list, index++, value_); // steals a reference + PyList_SET_ITEM(list.ptr(), index++, value_.release()); // steals a reference } - return list; + return list.release(); } PYBIND11_TYPE_CASTER(type, detail::descr("list<") + value_conv::name() + detail::descr(">")); }; @@ -77,17 +77,15 @@ public: } static PyObject *cast(const type &src, return_value_policy policy, PyObject *parent) { - PyObject *set = PySet_New(nullptr); + object set(PySet_New(nullptr), false); + if (!set) + return nullptr; for (auto const &value: src) { - PyObject *value_ = value_conv::cast(value, policy, parent); - if (!value_ || PySet_Add(set, value_) != 0) { - Py_XDECREF(value_); - Py_DECREF(set); + object value_(value_conv::cast(value, policy, parent), false); + if (!value_ || PySet_Add(set.ptr(), value_.ptr()) != 0) return nullptr; - } - Py_DECREF(value_); } - return set; + return set.release(); } PYBIND11_TYPE_CASTER(type, detail::descr("set<") + value_conv::name() + detail::descr(">")); }; @@ -116,20 +114,16 @@ public: } static PyObject *cast(const type &src, return_value_policy policy, PyObject *parent) { - PyObject *dict = PyDict_New(); + object dict(PyDict_New(), false); + if (!dict) + return nullptr; for (auto const &kv: src) { - PyObject *key = key_conv::cast(kv.first, policy, parent); - PyObject *value = value_conv::cast(kv.second, policy, parent); - if (!key || !value || PyDict_SetItem(dict, key, value) != 0) { - Py_XDECREF(key); - Py_XDECREF(value); - Py_DECREF(dict); + object key(key_conv::cast(kv.first, policy, parent), false); + object value(value_conv::cast(kv.second, policy, parent), false); + if (!key || !value || PyDict_SetItem(dict.ptr(), key.ptr(), value.ptr()) != 0) return nullptr; - } - Py_DECREF(key); - Py_DECREF(value); } - return dict; + return dict.release(); } PYBIND11_TYPE_CASTER(type, detail::descr("dict<") + key_conv::name() + detail::descr(", ") + value_conv::name() + detail::descr(">"));