Fix application of keep_alive policy to constructors (regression)

docs/advanced/functions.rst

@@ -191,6 +191,17 @@ container:
     py::class_<List>(m, "List")
     py::class_<List>(m, "List")
         .def("append", &List::append, py::keep_alive<1, 2>());
         .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::
 .. note::
 
 
     ``keep_alive`` is analogous to the ``with_custodian_and_ward`` (if Nurse,
     ``keep_alive`` is analogous to the ``with_custodian_and_ward`` (if Nurse,

docs/changelog.rst

@@ -11,6 +11,12 @@ v2.3.0 (Not yet released)
 
 
 * TBD
 * 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)
 v2.2.0 (August 31, 2017)
 -----------------------------------------------------
 -----------------------------------------------------
 
 

include/pybind11/cast.h

@@ -1800,6 +1800,9 @@ struct function_call {
 
 
     /// The parent, if any
     /// The parent, if any
     handle parent;
     handle parent;
+
+    /// If this is a call to an initializer, this argument contains `self`
+    handle init_self;
 };
 };
 
 
 
 

include/pybind11/pybind11.h

@@ -511,6 +511,7 @@ protected:
                     if (self_value_and_holder)
                     if (self_value_and_holder)
                         self_value_and_holder.type->dealloc(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.push_back(reinterpret_cast<PyObject *>(&self_value_and_holder));
                     call.args_convert.push_back(false);
                     call.args_convert.push_back(false);
                     ++args_copied;
                     ++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) {
 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) {
 inline std::pair<decltype(internals::registered_types_py)::iterator, bool> all_type_info_get_cache(PyTypeObject *type) {

tests/test_call_policies.cpp

@@ -32,7 +32,7 @@ bool DependentGuard::enabled = false;
 TEST_SUBMODULE(call_policies, m) {
 TEST_SUBMODULE(call_policies, m) {
     // Parent/Child are used in:
     // Parent/Child are used in:
     // test_keep_alive_argument, test_keep_alive_return_value, test_alive_gc_derived,
     // 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 {
     class Child {
     public:
     public:
         Child() { py::print("Allocating child."); }
         Child() { py::print("Allocating child."); }
@@ -51,6 +51,7 @@ TEST_SUBMODULE(call_policies, m) {
     };
     };
     py::class_<Parent>(m, "Parent")
     py::class_<Parent>(m, "Parent")
         .def(py::init<>())
         .def(py::init<>())
+        .def(py::init([](Child *) { return new Parent(); }), py::keep_alive<1, 2>())
         .def("addChild", &Parent::addChild)
         .def("addChild", &Parent::addChild)
         .def("addChildKeepAlive", &Parent::addChild, py::keep_alive<1, 2>())
         .def("addChildKeepAlive", &Parent::addChild, py::keep_alive<1, 2>())
         .def("returnChild", &Parent::returnChild)
         .def("returnChild", &Parent::returnChild)

tests/test_call_policies.py

@@ -156,6 +156,25 @@ def test_return_none(capture):
     assert capture == "Releasing parent."
     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():
 def test_call_guard():
     assert m.unguarded_call() == "unguarded"
     assert m.unguarded_call() == "unguarded"
     assert m.guarded_call() == "guarded"
     assert m.guarded_call() == "guarded"