Skip to content

always_construct_holder feature to support intrusively reference-counted types #561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion docs/advanced/smart_ptrs.rst
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ Custom smart pointers
pybind11 supports ``std::unique_ptr`` and ``std::shared_ptr`` right out of the
box. For any other custom smart pointer, transparent conversions can be enabled
using a macro invocation similar to the following. It must be declared at the
level before any binding code:
top namespace level before any binding code:

.. code-block:: cpp

Expand All @@ -134,6 +134,19 @@ placeholder name that is used as a template parameter of the second argument.
Thus, feel free to use any identifier, but use it consistently on both sides;
also, don't use the name of a type that already exists in your codebase.

The macro also accepts a third optional boolean parameter that is set to false
by default. Specify

.. code-block:: cpp

PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr<T>, true);

if ``SmartPtr<T>`` can always be initialized from a ``T*`` pointer without the
risk of inconsistencies (such as multiple independent ``SmartPtr`` instances
believing that they are the sole owner of the ``T*`` pointer). A common
situation where ``true`` should be passed is when the ``T`` instances use
*intrusive* reference counting.

Please take a look at the :ref:`macro_notes` before using this feature.

.. seealso::
Expand Down
6 changes: 5 additions & 1 deletion include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -952,10 +952,14 @@ template <typename type, typename holder_type> class type_caster_holder : public
template <typename T>
class type_caster<std::shared_ptr<T>> : public type_caster_holder<T, std::shared_ptr<T>> { };

template <typename T, bool Value = false> struct always_construct_holder { static constexpr bool value = Value; };

/// Create a specialization for custom holder types (silently ignores std::shared_ptr)
#define PYBIND11_DECLARE_HOLDER_TYPE(type, holder_type) \
#define PYBIND11_DECLARE_HOLDER_TYPE(type, holder_type, ...) \
namespace pybind11 { namespace detail { \
template <typename type> \
struct always_construct_holder<holder_type> : always_construct_holder<void, ##__VA_ARGS__> { }; \
template <typename type> \
class type_caster<holder_type, enable_if_t<!is_shared_ptr<holder_type>::value>> \
: public type_caster_holder<type, holder_type> { }; \
}}
Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -1151,7 +1151,7 @@ class class_ : public detail::generic_type {
if (holder_ptr) {
new (&inst->holder) holder_type(*holder_ptr);
inst->holder_constructed = true;
} else if (inst->owned) {
} else if (inst->owned || detail::always_construct_holder<holder_type>::value) {
new (&inst->holder) holder_type(inst->value);
inst->holder_constructed = true;
}
Expand All @@ -1161,7 +1161,7 @@ class class_ : public detail::generic_type {
template <typename T = holder_type,
detail::enable_if_t<!std::is_copy_constructible<T>::value, int> = 0>
static void init_holder_helper(instance_type *inst, const holder_type * /* unused */, const void * /* dummy */) {
if (inst->owned) {
if (inst->owned || detail::always_construct_holder<holder_type>::value) {
new (&inst->holder) holder_type(inst->value);
inst->holder_constructed = true;
}
Expand Down
18 changes: 17 additions & 1 deletion tests/test_smart_ptr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ class MyObject4 {
};

/// Make pybind aware of the ref-counted wrapper type (s)
PYBIND11_DECLARE_HOLDER_TYPE(T, ref<T>); // Required for custom holder type

// ref<T> is a wrapper for 'Object' which uses intrusive reference counting
// It is always possible to construct a ref<T> from an Object* pointer without
// possible incosistencies, hence the 'true' argument at the end.
PYBIND11_DECLARE_HOLDER_TYPE(T, ref<T>, true);
PYBIND11_DECLARE_HOLDER_TYPE(T, std::shared_ptr<T>); // Not required any more for std::shared_ptr,
// but it should compile without error

Expand Down Expand Up @@ -125,6 +129,18 @@ test_initializer smart_ptr([](py::module &m) {
py::class_<MyObject1, ref<MyObject1>>(m, "MyObject1", obj)
.def(py::init<int>());

m.def("test_object1_refcounting",
[]() -> bool {
ref<MyObject1> o = new MyObject1(0);
bool good = o->getRefCount() == 1;
py::object o2 = py::cast(o, py::return_value_policy::reference);
// always request (partial) ownership for objects with intrusive
// reference counting even when using the 'reference' RVP
good &= o->getRefCount() == 2;
return good;
}
);

m.def("make_object_1", &make_object_1);
m.def("make_object_2", &make_object_2);
m.def("make_myobject1_1", &make_myobject1_1);
Expand Down
5 changes: 5 additions & 0 deletions tests/test_smart_ptr.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ def test_smart_ptr(capture):
assert cstats.move_assignments == 0


def test_smart_ptr_refcounting():
from pybind11_tests import test_object1_refcounting
assert test_object1_refcounting()


def test_unique_nodelete():
from pybind11_tests import MyObject4
o = MyObject4(23)
Expand Down