From 7939f4b3fe421f1457841225c048330878979f6f Mon Sep 17 00:00:00 2001
From: Dean Moldovan <dean0x7d@gmail.com>
Date: Mon, 4 Sep 2017 13:49:19 +0200
Subject: [PATCH] Fix application of keep_alive policy to constructors
 (regression)

---
 docs/advanced/functions.rst  | 11 +++++++++++
 docs/changelog.rst           |  6 ++++++
 include/pybind11/cast.h      |  3 +++
 include/pybind11/pybind11.h  | 16 ++++++++++++----
 tests/test_call_policies.cpp |  3 ++-
 tests/test_call_policies.py  | 19 +++++++++++++++++++
 6 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/docs/advanced/functions.rst b/docs/advanced/functions.rst
index cdf318c0..c7892b5d 100644
--- a/docs/advanced/functions.rst
+++ b/docs/advanced/functions.rst
@@ -191,6 +191,17 @@ container:
     py::class_<List>(m, "List")
         .def("append", &List::append, py::keep_alive<1, 2>());
 
+For consistency, the argument indexing is identical for constructors. Index
+``1`` still refers to the implicit ``this`` pointer, i.e. the object which is
+being constructed. Index ``0`` refers to the return type which is presumed to
+be ``void`` when a constructor is viewed like a function. The following example
+ties the lifetime of the constructor element to the constructed object:
+
+.. code-block:: cpp
+
+    py::class_<Nurse>(m, "Nurse")
+        .def(py::init<Patient &>(), py::keep_alive<1, 2>());
+
 .. note::
 
     ``keep_alive`` is analogous to the ``with_custodian_and_ward`` (if Nurse,
diff --git a/docs/changelog.rst b/docs/changelog.rst
index af82dc5b..b9d630d6 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -11,6 +11,12 @@ v2.3.0 (Not yet released)
 
 * TBD
 
+v2.2.1 (Not yet released)
+-----------------------------------------------------
+
+* Fixed a regression where the ``py::keep_alive`` policy could not be applied
+  to constructors. `#1065 <https://github.com/pybind/pybind11/pull/1065>`_.
+
 v2.2.0 (August 31, 2017)
 -----------------------------------------------------
 
diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
index 2a419371..0e4fbde1 100644
--- a/include/pybind11/cast.h
+++ b/include/pybind11/cast.h
@@ -1800,6 +1800,9 @@ struct function_call {
 
     /// The parent, if any
     handle parent;
+
+    /// If this is a call to an initializer, this argument contains `self`
+    handle init_self;
 };
 
 
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
index 285fc534..3fcb99ff 100644
--- a/include/pybind11/pybind11.h
+++ b/include/pybind11/pybind11.h
@@ -511,6 +511,7 @@ protected:
                     if (self_value_and_holder)
                         self_value_and_holder.type->dealloc(self_value_and_holder);
 
+                    call.init_self = PyTuple_GET_ITEM(args_in, 0);
                     call.args.push_back(reinterpret_cast<PyObject *>(&self_value_and_holder));
                     call.args_convert.push_back(false);
                     ++args_copied;
@@ -1457,10 +1458,17 @@ inline void keep_alive_impl(handle nurse, handle patient) {
 }
 
 PYBIND11_NOINLINE inline void keep_alive_impl(size_t Nurse, size_t Patient, function_call &call, handle ret) {
-    keep_alive_impl(
-        Nurse   == 0 ? ret : Nurse   <= call.args.size() ? call.args[Nurse   - 1] : handle(),
-        Patient == 0 ? ret : Patient <= call.args.size() ? call.args[Patient - 1] : handle()
-    );
+    auto get_arg = [&](size_t n) {
+        if (n == 0)
+            return ret;
+        else if (n == 1 && call.init_self)
+            return call.init_self;
+        else if (n <= call.args.size())
+            return call.args[n - 1];
+        return handle();
+    };
+
+    keep_alive_impl(get_arg(Nurse), get_arg(Patient));
 }
 
 inline std::pair<decltype(internals::registered_types_py)::iterator, bool> all_type_info_get_cache(PyTypeObject *type) {
diff --git a/tests/test_call_policies.cpp b/tests/test_call_policies.cpp
index 65e54a0c..8642188f 100644
--- a/tests/test_call_policies.cpp
+++ b/tests/test_call_policies.cpp
@@ -32,7 +32,7 @@ bool DependentGuard::enabled = false;
 TEST_SUBMODULE(call_policies, m) {
     // Parent/Child are used in:
     // test_keep_alive_argument, test_keep_alive_return_value, test_alive_gc_derived,
-    // test_alive_gc_multi_derived, test_return_none
+    // test_alive_gc_multi_derived, test_return_none, test_keep_alive_constructor
     class Child {
     public:
         Child() { py::print("Allocating child."); }
@@ -51,6 +51,7 @@ TEST_SUBMODULE(call_policies, m) {
     };
     py::class_<Parent>(m, "Parent")
         .def(py::init<>())
+        .def(py::init([](Child *) { return new Parent(); }), py::keep_alive<1, 2>())
         .def("addChild", &Parent::addChild)
         .def("addChildKeepAlive", &Parent::addChild, py::keep_alive<1, 2>())
         .def("returnChild", &Parent::returnChild)
diff --git a/tests/test_call_policies.py b/tests/test_call_policies.py
index 6c60f817..7c835599 100644
--- a/tests/test_call_policies.py
+++ b/tests/test_call_policies.py
@@ -156,6 +156,25 @@ def test_return_none(capture):
     assert capture == "Releasing parent."
 
 
+def test_keep_alive_constructor(capture):
+    n_inst = ConstructorStats.detail_reg_inst()
+
+    with capture:
+        p = m.Parent(m.Child())
+        assert ConstructorStats.detail_reg_inst() == n_inst + 2
+    assert capture == """
+        Allocating child.
+        Allocating parent.
+    """
+    with capture:
+        del p
+        assert ConstructorStats.detail_reg_inst() == n_inst
+    assert capture == """
+        Releasing parent.
+        Releasing child.
+    """
+
+
 def test_call_guard():
     assert m.unguarded_call() == "unguarded"
     assert m.guarded_call() == "guarded"