Use std::addressof to obtain holder address instead of operator&

This commit is contained in:
Khachajantc Michael 2018-06-20 18:33:50 +03:00 committed by Wenzel Jakob
parent 526b708f9d
commit f52b46c5f7
4 changed files with 115 additions and 7 deletions

View File

@ -1392,7 +1392,7 @@ public:
explicit operator type*() { return this->value; } explicit operator type*() { return this->value; }
explicit operator type&() { return *(this->value); } explicit operator type&() { return *(this->value); }
explicit operator holder_type*() { return &holder; } explicit operator holder_type*() { return std::addressof(holder); }
// Workaround for Intel compiler bug // Workaround for Intel compiler bug
// see pybind11 issue 94 // see pybind11 issue 94
@ -1462,7 +1462,7 @@ struct move_only_holder_caster {
static handle cast(holder_type &&src, return_value_policy, handle) { static handle cast(holder_type &&src, return_value_policy, handle) {
auto *ptr = holder_helper<holder_type>::get(src); auto *ptr = holder_helper<holder_type>::get(src);
return type_caster_base<type>::cast_holder(ptr, &src); return type_caster_base<type>::cast_holder(ptr, std::addressof(src));
} }
static PYBIND11_DESCR name() { return type_caster_base<type>::name(); } static PYBIND11_DESCR name() { return type_caster_base<type>::name(); }
}; };

View File

@ -1267,25 +1267,25 @@ private:
auto sh = std::dynamic_pointer_cast<typename holder_type::element_type>( auto sh = std::dynamic_pointer_cast<typename holder_type::element_type>(
v_h.value_ptr<type>()->shared_from_this()); v_h.value_ptr<type>()->shared_from_this());
if (sh) { if (sh) {
new (&v_h.holder<holder_type>()) holder_type(std::move(sh)); new (std::addressof(v_h.holder<holder_type>())) holder_type(std::move(sh));
v_h.set_holder_constructed(); v_h.set_holder_constructed();
} }
} catch (const std::bad_weak_ptr &) {} } catch (const std::bad_weak_ptr &) {}
if (!v_h.holder_constructed() && inst->owned) { if (!v_h.holder_constructed() && inst->owned) {
new (&v_h.holder<holder_type>()) holder_type(v_h.value_ptr<type>()); new (std::addressof(v_h.holder<holder_type>())) holder_type(v_h.value_ptr<type>());
v_h.set_holder_constructed(); v_h.set_holder_constructed();
} }
} }
static void init_holder_from_existing(const detail::value_and_holder &v_h, static void init_holder_from_existing(const detail::value_and_holder &v_h,
const holder_type *holder_ptr, std::true_type /*is_copy_constructible*/) { const holder_type *holder_ptr, std::true_type /*is_copy_constructible*/) {
new (&v_h.holder<holder_type>()) holder_type(*reinterpret_cast<const holder_type *>(holder_ptr)); new (std::addressof(v_h.holder<holder_type>())) holder_type(*reinterpret_cast<const holder_type *>(holder_ptr));
} }
static void init_holder_from_existing(const detail::value_and_holder &v_h, static void init_holder_from_existing(const detail::value_and_holder &v_h,
const holder_type *holder_ptr, std::false_type /*is_copy_constructible*/) { const holder_type *holder_ptr, std::false_type /*is_copy_constructible*/) {
new (&v_h.holder<holder_type>()) holder_type(std::move(*const_cast<holder_type *>(holder_ptr))); new (std::addressof(v_h.holder<holder_type>())) holder_type(std::move(*const_cast<holder_type *>(holder_ptr)));
} }
/// Initialize holder object, variant 2: try to construct from existing holder object, if possible /// Initialize holder object, variant 2: try to construct from existing holder object, if possible
@ -1295,7 +1295,7 @@ private:
init_holder_from_existing(v_h, holder_ptr, std::is_copy_constructible<holder_type>()); init_holder_from_existing(v_h, holder_ptr, std::is_copy_constructible<holder_type>());
v_h.set_holder_constructed(); v_h.set_holder_constructed();
} else if (inst->owned || detail::always_construct_holder<holder_type>::value) { } else if (inst->owned || detail::always_construct_holder<holder_type>::value) {
new (&v_h.holder<holder_type>()) holder_type(v_h.value_ptr<type>()); new (std::addressof(v_h.holder<holder_type>())) holder_type(v_h.value_ptr<type>());
v_h.set_holder_constructed(); v_h.set_holder_constructed();
} }
} }

View File

@ -55,6 +55,35 @@ public:
}; };
PYBIND11_DECLARE_HOLDER_TYPE(T, custom_unique_ptr<T>); PYBIND11_DECLARE_HOLDER_TYPE(T, custom_unique_ptr<T>);
// Simple custom holder that works like shared_ptr and has operator& overload
// To obtain address of an instance of this holder pybind should use std::addressof
// Attempt to get address via operator& may leads to segmentation fault
template <typename T>
class shared_ptr_with_addressof_operator {
std::shared_ptr<T> impl;
public:
shared_ptr_with_addressof_operator( ) = default;
shared_ptr_with_addressof_operator(T* p) : impl(p) { }
T* get() const { return impl.get(); }
T** operator&() { throw std::logic_error("Call of overloaded operator& is not expected"); }
};
PYBIND11_DECLARE_HOLDER_TYPE(T, shared_ptr_with_addressof_operator<T>);
// Simple custom holder that works like unique_ptr and has operator& overload
// To obtain address of an instance of this holder pybind should use std::addressof
// Attempt to get address via operator& may leads to segmentation fault
template <typename T>
class unique_ptr_with_addressof_operator {
std::unique_ptr<T> impl;
public:
unique_ptr_with_addressof_operator() = default;
unique_ptr_with_addressof_operator(T* p) : impl(p) { }
T* get() const { return impl.get(); }
T* release_ptr() { return impl.release(); }
T** operator&() { throw std::logic_error("Call of overloaded operator& is not expected"); }
};
PYBIND11_DECLARE_HOLDER_TYPE(T, unique_ptr_with_addressof_operator<T>);
TEST_SUBMODULE(smart_ptr, m) { TEST_SUBMODULE(smart_ptr, m) {
@ -234,6 +263,41 @@ TEST_SUBMODULE(smart_ptr, m) {
py::class_<C, custom_unique_ptr<C>>(m, "TypeWithMoveOnlyHolder") py::class_<C, custom_unique_ptr<C>>(m, "TypeWithMoveOnlyHolder")
.def_static("make", []() { return custom_unique_ptr<C>(new C); }); .def_static("make", []() { return custom_unique_ptr<C>(new C); });
// test_holder_with_addressof_operator
struct TypeForHolderWithAddressOf {
TypeForHolderWithAddressOf() { print_created(this); }
TypeForHolderWithAddressOf(const TypeForHolderWithAddressOf &) { print_copy_created(this); }
TypeForHolderWithAddressOf(TypeForHolderWithAddressOf &&) { print_move_created(this); }
~TypeForHolderWithAddressOf() { print_destroyed(this); }
std::string toString() const {
return "TypeForHolderWithAddressOf[" + std::to_string(value) + "]";
}
int value = 42;
};
using HolderWithAddressOf = shared_ptr_with_addressof_operator<TypeForHolderWithAddressOf>;
py::class_<TypeForHolderWithAddressOf, HolderWithAddressOf>(m, "TypeForHolderWithAddressOf")
.def_static("make", []() { return HolderWithAddressOf(new TypeForHolderWithAddressOf); })
.def("get", [](const HolderWithAddressOf &self) { return self.get(); })
.def("print_object_1", [](const TypeForHolderWithAddressOf *obj) { py::print(obj->toString()); })
.def("print_object_2", [](HolderWithAddressOf obj) { py::print(obj.get()->toString()); })
.def("print_object_3", [](const HolderWithAddressOf &obj) { py::print(obj.get()->toString()); })
.def("print_object_4", [](const HolderWithAddressOf *obj) { py::print((*obj).get()->toString()); });
// test_move_only_holder_with_addressof_operator
struct TypeForMoveOnlyHolderWithAddressOf {
TypeForMoveOnlyHolderWithAddressOf(int value) : value{value} { print_created(this); }
~TypeForMoveOnlyHolderWithAddressOf() { print_destroyed(this); }
std::string toString() const {
return "MoveOnlyHolderWithAddressOf[" + std::to_string(value) + "]";
}
int value;
};
using MoveOnlyHolderWithAddressOf = unique_ptr_with_addressof_operator<TypeForMoveOnlyHolderWithAddressOf>;
py::class_<TypeForMoveOnlyHolderWithAddressOf, MoveOnlyHolderWithAddressOf>(m, "TypeForMoveOnlyHolderWithAddressOf")
.def_static("make", []() { return MoveOnlyHolderWithAddressOf(new TypeForMoveOnlyHolderWithAddressOf(0)); })
.def_readwrite("value", &TypeForMoveOnlyHolderWithAddressOf::value)
.def("print_object", [](const TypeForMoveOnlyHolderWithAddressOf *obj) { py::print(obj->toString()); });
// test_smart_ptr_from_default // test_smart_ptr_from_default
struct HeldByDefaultHolder { }; struct HeldByDefaultHolder { };
py::class_<HeldByDefaultHolder>(m, "HeldByDefaultHolder") py::class_<HeldByDefaultHolder>(m, "HeldByDefaultHolder")

View File

@ -203,6 +203,50 @@ def test_move_only_holder():
assert stats.alive() == 0 assert stats.alive() == 0
def test_holder_with_addressof_operator():
# this test must not throw exception from c++
a = m.TypeForHolderWithAddressOf.make()
a.print_object_1()
a.print_object_2()
a.print_object_3()
a.print_object_4()
stats = ConstructorStats.get(m.TypeForHolderWithAddressOf)
assert stats.alive() == 1
np = m.TypeForHolderWithAddressOf.make()
assert stats.alive() == 2
del a
assert stats.alive() == 1
del np
assert stats.alive() == 0
b = m.TypeForHolderWithAddressOf.make()
c = b
assert b.get() is c.get()
assert stats.alive() == 1
del b
assert stats.alive() == 1
del c
assert stats.alive() == 0
def test_move_only_holder_with_addressof_operator():
a = m.TypeForMoveOnlyHolderWithAddressOf.make()
a.print_object()
stats = ConstructorStats.get(m.TypeForMoveOnlyHolderWithAddressOf)
assert stats.alive() == 1
a.value = 42
assert a.value == 42
del a
assert stats.alive() == 0
def test_smart_ptr_from_default(): def test_smart_ptr_from_default():
instance = m.HeldByDefaultHolder() instance = m.HeldByDefaultHolder()
with pytest.raises(RuntimeError) as excinfo: with pytest.raises(RuntimeError) as excinfo: