From 24a2054dbcf8bae3e00927466c1942103f9df0f6 Mon Sep 17 00:00:00 2001
From: Ben North <ben@redfrontdoor.org>
Date: Thu, 20 Oct 2016 21:09:25 +0100
Subject: [PATCH] Fix wrapper's 'value' and 'owned' if ctor missing

type_caster_generic::cast(): The values of

    wrapper->value
    wrapper->owned

are incorrect in the case that a return value policy of 'copy' is
requested but there is no copy-constructor.  (Similarly 'move'.)  In
particular, if the source object is a static instance, the destructor of
the 'object' 'inst' leads to class_::dealloc() which incorrectly
attempts to 'delete' the static instance.

This commit re-arranges the code to be clearer as to what the values of
'value' and 'owned' should be in the various cases.  Behaviour is
different to previous code only in two situations:

policy = copy but no copy-ctor: Old code leaves 'value = src, owned =
true', which leads to trouble.  New code leaves 'value = nullptr, owned
= false', which is correct.

policy = move but no move- or copy-ctor: old code leaves 'value = src,
owned = true', which leads to trouble.  New code leaves 'value =
nullptr, owned = false', which is correct.
---
 include/pybind11/cast.h | 45 ++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
index f3690262e..dbbeb9225 100644
--- a/include/pybind11/cast.h
+++ b/include/pybind11/cast.h
@@ -258,31 +258,48 @@ public:
 
         auto wrapper = (instance<void> *) inst.ptr();
 
-        wrapper->value = src;
-        wrapper->owned = true;
+        wrapper->value = nullptr;
+        wrapper->owned = false;
 
-        if (policy == return_value_policy::automatic)
-            policy = return_value_policy::take_ownership;
-        else if (policy == return_value_policy::automatic_reference)
-            policy = return_value_policy::reference;
+        switch (policy) {
+        case return_value_policy::automatic:
+        case return_value_policy::take_ownership:
+            wrapper->value = src;
+            wrapper->owned = true;
+            break;
 
-        if (policy == return_value_policy::copy) {
+        case return_value_policy::automatic_reference:
+        case return_value_policy::reference:
+            wrapper->value = src;
+            wrapper->owned = false;
+            break;
+
+        case return_value_policy::copy:
             if (copy_constructor)
-                wrapper->value = copy_constructor(wrapper->value);
+                wrapper->value = copy_constructor(src);
             else
                 throw cast_error("return_value_policy = copy, but the object is non-copyable!");
-        } else if (policy == return_value_policy::move) {
+            wrapper->owned = true;
+            break;
+
+        case return_value_policy::move:
             if (move_constructor)
-                wrapper->value = move_constructor(wrapper->value);
+                wrapper->value = move_constructor(src);
             else if (copy_constructor)
-                wrapper->value = copy_constructor(wrapper->value);
+                wrapper->value = copy_constructor(src);
             else
                 throw cast_error("return_value_policy = move, but the object is neither movable nor copyable!");
-        } else if (policy == return_value_policy::reference) {
-            wrapper->owned = false;
-        } else if (policy == return_value_policy::reference_internal) {
+            wrapper->owned = true;
+            break;
+
+        case return_value_policy::reference_internal:
+            wrapper->value = src;
             wrapper->owned = false;
             detail::keep_alive_impl(inst, parent);
+            break;
+
+        default:
+            throw cast_error("unhandled return_value_policy: should not happen!");
         }
 
         tinfo->init_holder(inst.ptr(), existing_holder);