From c84b37b577f293c82672eaa022beca9aa549ad5d Mon Sep 17 00:00:00 2001
From: Wenzel Jakob <wenzel.jakob@epfl.ch>
Date: Wed, 7 Sep 2016 00:47:17 +0900
Subject: [PATCH] fix bogus return value policy fallbacks (fixes #389)

---
 include/pybind11/cast.h | 12 +++++++-----
 tests/test_issues.cpp   | 19 +++++++++++++++++++
 tests/test_issues.py    |  8 ++++++++
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
index c8c8f77ba..d28141a58 100644
--- a/include/pybind11/cast.h
+++ b/include/pybind11/cast.h
@@ -220,14 +220,16 @@ public:
             policy = return_value_policy::reference;
 
         if (policy == return_value_policy::copy) {
-            wrapper->value = copy_constructor(wrapper->value);
-            if (wrapper->value == nullptr)
+            if (copy_constructor)
+                wrapper->value = copy_constructor(wrapper->value);
+            else
                 throw cast_error("return_value_policy = copy, but the object is non-copyable!");
         } else if (policy == return_value_policy::move) {
-            wrapper->value = move_constructor(wrapper->value);
-            if (wrapper->value == nullptr)
+            if (move_constructor)
+                wrapper->value = move_constructor(wrapper->value);
+            else if (copy_constructor)
                 wrapper->value = copy_constructor(wrapper->value);
-            if (wrapper->value == nullptr)
+            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;
diff --git a/tests/test_issues.cpp b/tests/test_issues.cpp
index 16a44c74f..535ee39a0 100644
--- a/tests/test_issues.cpp
+++ b/tests/test_issues.cpp
@@ -172,6 +172,25 @@ void init_issues(py::module &m) {
     m2.def("get_NestA", [](const NestA &a) { return a.value; });
     m2.def("get_NestB", [](const NestB &b) { return b.value; });
     m2.def("get_NestC", [](const NestC &c) { return c.value; });
+
+    // Issue 389: r_v_p::move should fall-through to copy on non-movable objects
+    class MoveIssue1 {
+    public:
+        MoveIssue1(int v) : v{v} {}
+        MoveIssue1(const MoveIssue1 &c) { std::cerr << "copy ctor\n"; v=c.v; }
+        MoveIssue1(MoveIssue1 &&) = delete;
+        int v;
+    };
+    class MoveIssue2 {
+    public:
+        MoveIssue2(int v) : v{v} {}
+        MoveIssue2(MoveIssue2 &&) = default;
+        int v;
+    };
+    py::class_<MoveIssue1>(m2, "MoveIssue1").def(py::init<int>()).def_readwrite("value", &MoveIssue1::v);
+    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);
 }
 
 // MSVC workaround: trying to use a lambda here crashes MSCV
diff --git a/tests/test_issues.py b/tests/test_issues.py
index 38a5324fa..c1e0756fd 100644
--- a/tests/test_issues.py
+++ b/tests/test_issues.py
@@ -158,3 +158,11 @@ def test_nested():
     assert abase.value == 42
     del abase, b
     gc.collect()
+
+
+def test_move_fallback():
+    from pybind11_tests.issues import get_moveissue1, get_moveissue2
+    m2 = get_moveissue2(2)
+    assert m2.value == 2
+    m1 = get_moveissue1(1)
+    assert m1.value == 1