From 0abc2240e3a08930f723cee7aebd19d4c72f7665 Mon Sep 17 00:00:00 2001 From: Blake Thompson Date: Wed, 13 Nov 2019 22:26:54 -0600 Subject: [PATCH 1/7] Added a way for non custom types to take advantage of smart pointers --- include/pybind11/cast.h | 34 ++++++++++++++++++++++++++++------ include/pybind11/detail/init.h | 2 +- tests/test_smart_ptr.cpp | 4 +++- tests/test_stl.cpp | 3 +++ tests/test_stl.py | 6 ++++++ 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 90407eb984..c3b1547f6d 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -16,6 +16,7 @@ #include "detail/internals.h" #include #include +#include #include #include @@ -1453,9 +1454,11 @@ template class type_caster> /// Helper class which abstracts away certain actions. Users can provide specializations for /// custom holders, but it's only necessary if the type has a non-standard interface. -template +template struct holder_helper { - static auto get(const T &p) -> decltype(p.get()) { return p.get(); } + static auto get(const holder_type &p) -> decltype(p.get()) { return p.get(); } + static auto get(holder_type &p) -> decltype(p.get()) { return p.get(); } + static holder_type create(type && val) { return holder_type(new type(std::forward(val))); } }; /// Type caster for holder types like std::shared_ptr, etc. @@ -1463,17 +1466,28 @@ template struct copyable_holder_caster : public type_caster_base { public: using base = type_caster_base; - static_assert(std::is_base_of>::value, - "Holder classes are only supported for custom types"); using base::base; using base::cast; using base::typeinfo; using base::value; + template , type_caster>::value, int> = 0> bool load(handle src, bool convert) { return base::template load_impl>(src, convert); } + template , type_caster>::value, int> = 0> + bool load(handle src, bool convert) { + using value_conv = make_caster; + value_conv caster; + if (!caster.load(src, convert)) { + return false; + } + holder = holder_helper::create(std::forward(cast_op(std::move(caster)))); + value = reinterpret_cast(holder_helper::get(holder)); + return true; + } + explicit operator type*() { return this->value; } explicit operator type&() { return *(this->value); } explicit operator holder_type*() { return std::addressof(holder); } @@ -1486,11 +1500,19 @@ struct copyable_holder_caster : public type_caster_base { explicit operator holder_type&() { return holder; } #endif + template , type_caster>::value, int> = 0> static handle cast(const holder_type &src, return_value_policy, handle) { - const auto *ptr = holder_helper::get(src); + const auto *ptr = holder_helper::get(src); return type_caster_base::cast_holder(ptr, &src); } + template , type_caster>::value, int> = 0> + static handle cast(const holder_type &src, return_value_policy policy, handle parent) { + using value_conv = make_caster; + const auto *ptr = holder_helper::get(src); + return value_conv::cast(*ptr, policy, parent); + } + protected: friend class type_caster_generic; void check_holder_compat() { @@ -1545,7 +1567,7 @@ struct move_only_holder_caster { "Holder classes are only supported for custom types"); static handle cast(holder_type &&src, return_value_policy, handle) { - auto *ptr = holder_helper::get(src); + auto *ptr = holder_helper::get(src); return type_caster_base::cast_holder(ptr, std::addressof(src)); } static constexpr auto name = type_caster_base::name; diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index acfe00bdb7..0f8e37ba99 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -131,7 +131,7 @@ void construct(value_and_holder &v_h, Alias *alias_ptr, bool) { // derived type (through those holder's implicit conversion from derived class holder constructors). template void construct(value_and_holder &v_h, Holder holder, bool need_alias) { - auto *ptr = holder_helper>::get(holder); + auto *ptr = holder_helper>::get(holder); // If we need an alias, check that the held pointer is actually an alias instance if (Class::has_alias && need_alias && !is_alias(ptr)) throw type_error("pybind11::init(): construction failed: returned holder-wrapped instance " diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 87c9be8c2b..de70c34f67 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -24,8 +24,10 @@ PYBIND11_DECLARE_HOLDER_TYPE(T, ref, true); // Make pybind11 aware of the non-standard getter member function namespace pybind11 { namespace detail { template - struct holder_helper> { + struct holder_helper> { static const T *get(const ref &p) { return p.get_ptr(); } + static T *get(ref &p) { return p.get_ptr(); } + static ref create(T && val) { return ref(&val); } }; }} diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 207c9fb2bf..c1b5a7e500 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -53,7 +53,10 @@ namespace std { TEST_SUBMODULE(stl, m) { // test_vector m.def("cast_vector", []() { return std::vector{1}; }); + m.def("cast_vector_shared", []() { return std::make_shared>(std::vector{1}); }); + m.def("load_vector", [](const std::vector &v) { return v.at(0) == 1 && v.at(1) == 2; }); + m.def("load_vector_shared", [](std::shared_ptr> v) { return v->at(0) == 1 && v->at(1) == 2; }); // `std::vector` is special because it returns proxy objects instead of references m.def("cast_bool_vector", []() { return std::vector{true, false}; }); m.def("load_bool_vector", [](const std::vector &v) { diff --git a/tests/test_stl.py b/tests/test_stl.py index 2335cb9fdf..cd70b7f04b 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -13,6 +13,12 @@ def test_vector(doc): assert m.load_vector(lst) assert m.load_vector(tuple(lst)) + lst = m.cast_vector_shared() + assert lst == [1] + lst.append(2) + assert m.load_vector_shared(lst) + assert m.load_vector_shared(tuple(lst)) + assert m.cast_bool_vector() == [True, False] assert m.load_bool_vector([True, False]) From 4d4fa466c400f075027e85ea26d1749dde7a1d87 Mon Sep 17 00:00:00 2001 From: Blake Thompson Date: Thu, 14 Nov 2019 11:30:01 -0600 Subject: [PATCH 2/7] Added a few more tests --- tests/test_stl.cpp | 4 +++ tests/test_stl.py | 3 ++ tests/test_stl_binders.cpp | 3 ++ tests/test_stl_binders.py | 59 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index c1b5a7e500..d7ac1d6c84 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -211,6 +211,7 @@ TEST_SUBMODULE(stl, m) { result_type operator()(std::string) { return "std::string"; } result_type operator()(double) { return "double"; } result_type operator()(std::nullptr_t) { return "std::nullptr_t"; } + result_type operator()(std::shared_ptr>) { return "std::shared_ptr>"; } }; // test_variant @@ -224,6 +225,9 @@ TEST_SUBMODULE(stl, m) { using V = variant; return py::make_tuple(V(5), V("Hello")); }); + m.def("load_variant_with_shared", [](variant>> v) { + return py::detail::visit_helper::call(visitor(), v); + }); #endif // #528: templated constructor diff --git a/tests/test_stl.py b/tests/test_stl.py index cd70b7f04b..438460811d 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -165,6 +165,9 @@ def test_variant(doc): assert m.load_variant_2pass(1) == "int" assert m.load_variant_2pass(1.0) == "double" + assert m.load_variant_with_shared(1) == "int" + assert m.load_variant_with_shared([1, 2]) == "std::shared_ptr>" + assert m.cast_variant() == (5, "Hello") assert doc(m.load_variant) == "load_variant(arg0: Union[int, str, float, None]) -> str" diff --git a/tests/test_stl_binders.cpp b/tests/test_stl_binders.cpp index 8688874091..e8946d15b9 100644 --- a/tests/test_stl_binders.cpp +++ b/tests/test_stl_binders.cpp @@ -66,6 +66,9 @@ TEST_SUBMODULE(stl_binders, m) { // test_vector_int py::bind_vector>(m, "VectorInt", py::buffer_protocol()); + // test_vector_double_shared + py::bind_vector, std::shared_ptr>>(m, "VectorDoubleShared"); + // test_vector_custom py::class_(m, "El") .def(py::init()); diff --git a/tests/test_stl_binders.py b/tests/test_stl_binders.py index b83a587f26..89533e8cc9 100644 --- a/tests/test_stl_binders.py +++ b/tests/test_stl_binders.py @@ -64,6 +64,65 @@ def test_vector_int(): del v_int2[-1] assert v_int2 == m.VectorInt([0, 99, 2, 3, 4, 5, 6, 7, 0, 1, 2, 3, 88]) + +def test_vector_double_shared(): + v_dbl = m.VectorDoubleShared([0.0, 0.0]) + assert len(v_dbl) == 2 + assert bool(v_dbl) is True + + # test construction from a generator + v_dbl1 = m.VectorDoubleShared(x for x in range(5)) + assert v_dbl1 == m.VectorDoubleShared([0.0, 1.0, 2.0, 3.0, 4.0]) + + v_dbl2 = m.VectorDoubleShared([0.0, 0.0]) + assert v_dbl == v_dbl2 + v_dbl2[1] = 1 + assert v_dbl != v_dbl2 + + v_dbl2.append(2) + v_dbl2.insert(0, 1.0) + v_dbl2.insert(0, 2.0) + v_dbl2.insert(0, 3.0) + v_dbl2.insert(6, 3.0) + assert str(v_dbl2) == "VectorDoubleShared[3, 2, 1, 0, 1, 2, 3]" + with pytest.raises(IndexError): + v_dbl2.insert(8, 4) + + v_dbl.append(99.2) + v_dbl2[2:-2] = v_dbl + assert v_dbl2 == m.VectorDoubleShared([3, 2, 0, 0, 99.2, 2, 3]) + del v_dbl2[1:3] + assert v_dbl2 == m.VectorDoubleShared([3, 0, 99.2, 2, 3]) + del v_dbl2[0] + assert v_dbl2 == m.VectorDoubleShared([0, 99.2, 2, 3]) + + v_dbl2.extend(m.VectorDoubleShared([4, 5])) + assert v_dbl2 == m.VectorDoubleShared([0, 99.2, 2, 3, 4, 5]) + + v_dbl2.extend([6, 7]) + assert v_dbl2 == m.VectorDoubleShared([0, 99.2, 2, 3, 4, 5, 6, 7]) + + # test error handling, and that the vector is unchanged + with pytest.raises(RuntimeError): + v_dbl2.extend([8, 'a']) + + assert v_dbl2 == m.VectorDoubleShared([0, 99.2, 2, 3, 4, 5, 6, 7]) + + # test extending from a generator + v_dbl2.extend(x for x in range(5)) + assert v_dbl2 == m.VectorDoubleShared([0, 99.2, 2, 3, 4, 5, 6, 7, 0, 1, 2, 3, 4]) + + # test negative indexing + assert v_dbl2[-1] == 4 + + # insert with negative index + v_dbl2.insert(-1, 88) + assert v_dbl2 == m.VectorDoubleShared([0, 99.2, 2, 3, 4, 5, 6, 7, 0, 1, 2, 3, 88, 4]) + + # delete negative index + del v_dbl2[-1] + assert v_dbl2 == m.VectorDoubleShared([0, 99.2, 2, 3, 4, 5, 6, 7, 0, 1, 2, 3, 88]) + # related to the PyPy's buffer protocol. @pytest.unsupported_on_pypy def test_vector_buffer(): From 3900885bede170e14f1040b0b7374cbb0933e98e Mon Sep 17 00:00:00 2001 From: Blake Thompson Date: Thu, 14 Nov 2019 13:23:51 -0600 Subject: [PATCH 3/7] Added support for custom casters with unique ptrs, adjusted return value policies --- include/pybind11/cast.h | 47 ++++++++++++++++++++++++---------------- tests/test_smart_ptr.cpp | 3 +++ tests/test_smart_ptr.py | 11 ++++++++++ tests/test_stl.cpp | 10 ++++++--- tests/test_stl.py | 21 +++++++++++++----- 5 files changed, 64 insertions(+), 28 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index c3b1547f6d..73c4daf6e3 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1452,6 +1452,22 @@ template class type_caster> template class type_caster> : public tuple_caster {}; +// When a value returned from a C++ function is being cast back to Python, we almost always want to +// force `policy = move`, regardless of the return value policy the function/method was declared +// with. +template struct return_value_policy_override { + static return_value_policy policy(return_value_policy p) { return p; } +}; + +template struct return_value_policy_override>::value, void>> { + static return_value_policy policy(return_value_policy p) { + return !std::is_lvalue_reference::value && + !std::is_pointer::value + ? return_value_policy::move : p; + } +}; + /// Helper class which abstracts away certain actions. Users can provide specializations for /// custom holders, but it's only necessary if the type has a non-standard interface. template @@ -1503,11 +1519,12 @@ struct copyable_holder_caster : public type_caster_base { template , type_caster>::value, int> = 0> static handle cast(const holder_type &src, return_value_policy, handle) { const auto *ptr = holder_helper::get(src); - return type_caster_base::cast_holder(ptr, &src); + return type_caster_base::cast_holder(ptr, std::addressof(src)); } template , type_caster>::value, int> = 0> static handle cast(const holder_type &src, return_value_policy policy, handle parent) { + policy = return_value_policy_override::policy(policy); using value_conv = make_caster; const auto *ptr = holder_helper::get(src); return value_conv::cast(*ptr, policy, parent); @@ -1563,13 +1580,21 @@ class type_caster> : public copyable_holder_caster struct move_only_holder_caster { - static_assert(std::is_base_of, type_caster>::value, - "Holder classes are only supported for custom types"); + template , type_caster>::value, int> = 0> static handle cast(holder_type &&src, return_value_policy, handle) { auto *ptr = holder_helper::get(src); return type_caster_base::cast_holder(ptr, std::addressof(src)); } + + template , type_caster>::value, int> = 0> + static handle cast(const holder_type &src, return_value_policy policy, handle parent) { + policy = return_value_policy_override::policy(policy); + using value_conv = make_caster; + const auto *ptr = holder_helper::get(src); + return value_conv::cast(*ptr, policy, parent); + } + static constexpr auto name = type_caster_base::name; }; @@ -1666,22 +1691,6 @@ template using cast_is_temporary_value_reference = bool_constant !std::is_same, void>::value >; -// When a value returned from a C++ function is being cast back to Python, we almost always want to -// force `policy = move`, regardless of the return value policy the function/method was declared -// with. -template struct return_value_policy_override { - static return_value_policy policy(return_value_policy p) { return p; } -}; - -template struct return_value_policy_override>::value, void>> { - static return_value_policy policy(return_value_policy p) { - return !std::is_lvalue_reference::value && - !std::is_pointer::value - ? return_value_policy::move : p; - } -}; - // Basic python -> C++ casting; throws if casting fails template type_caster &load_type(type_caster &conv, const handle &handle) { if (!conv.load(handle, true)) { diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index de70c34f67..605c9c88ea 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -90,6 +90,9 @@ PYBIND11_DECLARE_HOLDER_TYPE(T, unique_ptr_with_addressof_operator); TEST_SUBMODULE(smart_ptr, m) { // test_smart_ptr + m.def("cast_shared_int", []() { return std::make_shared(1); }); + m.def("cast_unique_int", []() { return std::make_unique(1); }); + m.def("load_shared_int", [](std::shared_ptr x) { return *x == 1; }); // Object implementation in `object.h` py::class_> obj(m, "Object"); diff --git a/tests/test_smart_ptr.py b/tests/test_smart_ptr.py index c6627043bd..69f47a61b0 100644 --- a/tests/test_smart_ptr.py +++ b/tests/test_smart_ptr.py @@ -102,6 +102,17 @@ def test_smart_ptr(capture): assert cstats.move_assignments == 0 +def test_shared_ptr(): + v_int = m.cast_shared_int() + assert v_int == 1 + assert m.load_shared_int(v_int) + + +def test_unique_ptr(): + v_int = m.cast_unique_int() + assert v_int == 1 + + def test_smart_ptr_refcounting(): assert m.test_object1_refcounting() diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index d7ac1d6c84..352e8b2b25 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -53,10 +53,7 @@ namespace std { TEST_SUBMODULE(stl, m) { // test_vector m.def("cast_vector", []() { return std::vector{1}; }); - m.def("cast_vector_shared", []() { return std::make_shared>(std::vector{1}); }); - m.def("load_vector", [](const std::vector &v) { return v.at(0) == 1 && v.at(1) == 2; }); - m.def("load_vector_shared", [](std::shared_ptr> v) { return v->at(0) == 1 && v->at(1) == 2; }); // `std::vector` is special because it returns proxy objects instead of references m.def("cast_bool_vector", []() { return std::vector{true, false}; }); m.def("load_bool_vector", [](const std::vector &v) { @@ -66,6 +63,13 @@ TEST_SUBMODULE(stl, m) { static std::vector lvv{2}; m.def("cast_ptr_vector", []() { return &lvv; }); + // test_vector_shared + m.def("cast_vector_shared", []() { return std::make_shared>(std::vector{1}); }); + m.def("load_vector_shared", [](std::shared_ptr> v) { return v->at(0) == 1 && v->at(1) == 2; }); + + // test_vector_unique + m.def("cast_vector_unique", []() { return std::make_unique>(std::vector{1}); }); + // test_deque m.def("cast_deque", []() { return std::deque{1}; }); m.def("load_deque", [](const std::deque &v) { return v.at(0) == 1 && v.at(1) == 2; }); diff --git a/tests/test_stl.py b/tests/test_stl.py index 438460811d..9871c811a8 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -13,12 +13,6 @@ def test_vector(doc): assert m.load_vector(lst) assert m.load_vector(tuple(lst)) - lst = m.cast_vector_shared() - assert lst == [1] - lst.append(2) - assert m.load_vector_shared(lst) - assert m.load_vector_shared(tuple(lst)) - assert m.cast_bool_vector() == [True, False] assert m.load_bool_vector([True, False]) @@ -29,6 +23,21 @@ def test_vector(doc): assert m.cast_ptr_vector() == ["lvalue", "lvalue"] +def test_vector_shared(doc): + """std::shared_ptr <-> list""" + lst = m.cast_vector_shared() + assert lst == [1] + lst.append(2) + assert m.load_vector_shared(lst) + assert m.load_vector_shared(tuple(lst)) + + +def test_vector_unique(doc): + """std::unique_ptr -> list""" + lst = m.cast_vector_unique() + assert lst == [1] + + def test_deque(doc): """std::deque <-> list""" lst = m.cast_deque() From 66d154bb82dd2e50f323073467b16acf089eb0e9 Mon Sep 17 00:00:00 2001 From: Blake Thompson Date: Fri, 15 Nov 2019 09:48:03 -0600 Subject: [PATCH 4/7] Add additional include --- tests/test_smart_ptr.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 605c9c88ea..a077a645e4 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -14,6 +14,7 @@ #include "pybind11_tests.h" #include "object.h" +#include // Make pybind aware of the ref-counted wrapper type (s): From 8d645076dc8f0063a5234c7736d444cb70e64642 Mon Sep 17 00:00:00 2001 From: Blake Thompson Date: Mon, 18 Nov 2019 09:48:51 -0600 Subject: [PATCH 5/7] Remove make unique as it is cpp14 --- tests/test_smart_ptr.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index a077a645e4..7fc1231103 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -14,7 +14,6 @@ #include "pybind11_tests.h" #include "object.h" -#include // Make pybind aware of the ref-counted wrapper type (s): @@ -92,7 +91,7 @@ TEST_SUBMODULE(smart_ptr, m) { // test_smart_ptr m.def("cast_shared_int", []() { return std::make_shared(1); }); - m.def("cast_unique_int", []() { return std::make_unique(1); }); + m.def("cast_unique_int", []() { return std::unique_ptr(new int(1)); }); m.def("load_shared_int", [](std::shared_ptr x) { return *x == 1; }); // Object implementation in `object.h` From 98504da4be78eac0cf7bd57821422e8412e9b7d4 Mon Sep 17 00:00:00 2001 From: Blake Thompson Date: Mon, 18 Nov 2019 10:16:59 -0600 Subject: [PATCH 6/7] Remove make unique as it is cpp14 part 2 --- tests/test_stl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 352e8b2b25..5f044bc2d3 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -68,7 +68,7 @@ TEST_SUBMODULE(stl, m) { m.def("load_vector_shared", [](std::shared_ptr> v) { return v->at(0) == 1 && v->at(1) == 2; }); // test_vector_unique - m.def("cast_vector_unique", []() { return std::make_unique>(std::vector{1}); }); + m.def("cast_vector_unique", []() { return std::unique_ptr>(new std::vector{1}); }); // test_deque m.def("cast_deque", []() { return std::deque{1}; }); From dbdf62e1f88576ad5311aa5f007e82095f47379b Mon Sep 17 00:00:00 2001 From: Blake Thompson Date: Wed, 20 Nov 2019 14:57:03 -0600 Subject: [PATCH 7/7] Import new as we are using align_val_t --- include/pybind11/cast.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 73c4daf6e3..9ee9dd1926 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include