Merge pull request #394 from jagerman/fix-ref-heap-casts
Fix ref heap casts
diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
index 438f7cf..b30b5be 100644
--- a/include/pybind11/cast.h
+++ b/include/pybind11/cast.h
@@ -264,24 +264,25 @@
/// Generic type caster for objects stored on the heap
template <typename type> class type_caster_base : public type_caster_generic {
+ using itype = intrinsic_t<type>;
public:
static PYBIND11_DESCR name() { return type_descr(_<type>()); }
type_caster_base() : type_caster_generic(typeid(type)) { }
- static handle cast(const type &src, return_value_policy policy, handle parent) {
+ static handle cast(const itype &src, return_value_policy policy, handle parent) {
if (policy == return_value_policy::automatic || policy == return_value_policy::automatic_reference)
policy = return_value_policy::copy;
return cast(&src, policy, parent);
}
- static handle cast(type &&src, return_value_policy policy, handle parent) {
+ static handle cast(itype &&src, return_value_policy policy, handle parent) {
if (policy == return_value_policy::automatic || policy == return_value_policy::automatic_reference)
policy = return_value_policy::move;
return cast(&src, policy, parent);
}
- static handle cast(const type *src, return_value_policy policy, handle parent) {
+ static handle cast(const itype *src, return_value_policy policy, handle parent) {
return type_caster_generic::cast(
src, policy, parent, src ? &typeid(*src) : nullptr, &typeid(type),
make_copy_constructor(src), make_move_constructor(src));
@@ -289,8 +290,8 @@
template <typename T> using cast_op_type = pybind11::detail::cast_op_type<T>;
- operator type*() { return (type *) value; }
- operator type&() { if (!value) throw reference_cast_error(); return *((type *) value); }
+ operator itype*() { return (type *) value; }
+ operator itype&() { if (!value) throw reference_cast_error(); return *((itype *) value); }
protected:
typedef void *(*Constructor)(const void *stream);
@@ -859,10 +860,22 @@
>::type> : std::true_type {};
template <typename T> using move_never = std::integral_constant<bool, !move_always<T>::value && !move_if_unreferenced<T>::value>;
+// Detect whether returning a `type` from a cast on type's type_caster is going to result in a
+// reference or pointer to a local variable of the type_caster. Basically, only
+// non-reference/pointer `type`s and reference/pointers from a type_caster_generic are safe;
+// everything else returns a reference/pointer to a local variable.
+template <typename type> using cast_is_temporary_value_reference = bool_constant<
+ (std::is_reference<type>::value || std::is_pointer<type>::value) &&
+ !std::is_base_of<type_caster_generic, make_caster<type>>::value
+>;
+
+
NAMESPACE_END(detail)
template <typename T> T cast(const handle &handle) {
using type_caster = detail::make_caster<T>;
+ static_assert(!detail::cast_is_temporary_value_reference<T>::value,
+ "Unable to cast type to reference: value is local to type caster");
type_caster conv;
if (!conv.load(handle, true)) {
#if defined(NDEBUG)
diff --git a/tests/test_issues.cpp b/tests/test_issues.cpp
index 7c10643..c4d89c1 100644
--- a/tests/test_issues.cpp
+++ b/tests/test_issues.cpp
@@ -191,6 +191,38 @@
py::class_<MoveIssue2>(m2, "MoveIssue2").def(py::init<int>()).def_readwrite("value", &MoveIssue2::v);
m2.def("get_moveissue1", [](int i) -> MoveIssue1 * { return new MoveIssue1(i); }, py::return_value_policy::move);
m2.def("get_moveissue2", [](int i) { return MoveIssue2(i); }, py::return_value_policy::move);
+
+ // Issue 392: overridding reference-returning functions
+ class OverrideTest {
+ public:
+ struct A { int value = 99; };
+ int v;
+ A a;
+ explicit OverrideTest(int v) : v{v} {}
+ virtual int int_value() { return v; }
+ virtual int &int_ref() { return v; }
+ virtual A A_value() { return a; }
+ virtual A &A_ref() { return a; }
+ };
+ class PyOverrideTest : public OverrideTest {
+ public:
+ using OverrideTest::OverrideTest;
+ int int_value() override { PYBIND11_OVERLOAD(int, OverrideTest, int_value); }
+ // Not allowed (uncommenting should hit a static_assert failure): we can't get a reference
+ // to a python numeric value, since we only copy values in the numeric type caster:
+// int &int_ref() override { PYBIND11_OVERLOAD(int &, OverrideTest, int_ref); }
+ A A_value() override { PYBIND11_OVERLOAD(A, OverrideTest, A_value); }
+ A &A_ref() override { PYBIND11_OVERLOAD(A &, OverrideTest, A_ref); }
+ };
+ py::class_<OverrideTest::A>(m2, "OverrideTest_A")
+ .def_readwrite("value", &OverrideTest::A::value);
+ py::class_<OverrideTest, PyOverrideTest>(m2, "OverrideTest")
+ .def(py::init<int>())
+ .def("int_value", &OverrideTest::int_value)
+// .def("int_ref", &OverrideTest::int_ref)
+ .def("A_value", &OverrideTest::A_value)
+ .def("A_ref", &OverrideTest::A_ref);
+
}
// MSVC workaround: trying to use a lambda here crashes MSCV
diff --git a/tests/test_issues.py b/tests/test_issues.py
index c1e0756..86bd6a5 100644
--- a/tests/test_issues.py
+++ b/tests/test_issues.py
@@ -166,3 +166,18 @@
assert m2.value == 2
m1 = get_moveissue1(1)
assert m1.value == 1
+
+def test_override_ref():
+ from pybind11_tests.issues import OverrideTest
+ o = OverrideTest(42)
+
+ # Not allowed (see associated .cpp comment)
+ #i = o.int_ref()
+ #assert o.int_ref() == 42
+ assert o.int_value() == 42
+
+ assert o.A_value().value == 99
+ a = o.A_ref()
+ assert a.value == 99
+ a.value = 7
+ assert a.value == 7