Allow binding factory functions as constructors

jagerman · jagerman · commit 2eb05d6b0c4c · 2017-04-17T02:12:22.000-04:00
This allows you to use:

    cls.def_static("__init__", &amp;factory_function);

where `factory_function` is some pointer or holder-generating factory
function of the type that `cls` binds.

Internally, this still results in a method, but we handle the
C++-static-factory &lt;-&gt; python method translation by calling the factory
function, then stealing the resulting object internal pointer and
holder.
diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst
@@ -366,6 +366,30 @@ In other words, :func:`init` creates an anonymous function that invokes an
 in-place constructor. Memory allocation etc. is already take care of beforehand
 within pybind11.
 
+It is also possible to bind C++ factory functions as Python constructors
+(instead of or in addition to standard constructors) by using ``def_static``:
+
+.. code-block:: cpp
+
+    py::class_<Example>(m, "Example")
+        // Bind an existing factory function which returns a new Example pointer:
+        .def_static("__init__", &Example::create)
+        // Similar, but returns using an existing holder:
+        .def_static("__init__", []() {
+            return std::unique_ptr<Example>(new Example(arg, "another arg"));
+        })
+        // Can mix/overload these with regular constructors, too:
+        .def(py::init<double>())
+        ;
+
+.. note::
+
+    Unlike other named functions declared using ``def_static``, these
+    constructors are *not* static on the Python side, only on the C++ side.
+    Pybind internally converts the (non-static) Python constructor to a static
+    C++ call when one of these constructors is invoked, then sets up the Python
+    instance appropriately.
+
 .. _classes_with_non_public_destructors:
 
 Non-public destructors
diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h
@@ -131,8 +131,8 @@ struct argument_record {
 /// Internal data structure which holds metadata about a bound function (signature, overloads, etc.)
 struct function_record {
     function_record()
-        : is_constructor(false), is_stateless(false), is_operator(false),
-          has_args(false), has_kwargs(false), is_method(false) { }
+        : is_constructor(false), is_factory_constructor(false), is_stateless(false),
+          is_operator(false), has_args(false), has_kwargs(false), is_method(false) { }
 
     /// Function name
     char *name = nullptr; /* why no C++ strings? They generate heavier code.. */
@@ -161,6 +161,9 @@ struct function_record {
     /// True if name == '__init__'
     bool is_constructor : 1;
 
+    /// True if name == '__init__' and this was a `def_static` (factory function exposed as a constructor)
+    bool is_factory_constructor : 1;
+
     /// True if this is a stateless function pointer
     bool is_stateless : 1;
 
@@ -216,7 +219,7 @@ struct type_record {
     void *(*operator_new)(size_t) = ::operator new;
 
     /// Function pointer to class_<..>::init_holder
-    void (*init_holder)(PyObject *, const void *) = nullptr;
+    void (*init_holder)(PyObject *, const void *, PyObject *) = nullptr;
 
     /// Function pointer to class_<..>::dealloc
     void (*dealloc)(PyObject *) = nullptr;
diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
@@ -26,7 +26,7 @@ struct type_info {
     PyTypeObject *type;
     size_t type_size;
     void *(*operator_new)(size_t);
-    void (*init_holder)(PyObject *, const void *);
+    void (*init_holder)(PyObject *, const void *, PyObject *);
     void (*dealloc)(PyObject *);
     std::vector<PyObject *(*)(PyObject *, PyTypeObject *)> implicit_conversions;
     std::vector<std::pair<const std::type_info *, void *(*)(void *)>> implicit_casts;
@@ -352,7 +352,7 @@ class type_caster_generic {
                 throw cast_error("unhandled return_value_policy: should not happen!");
         }
 
-        tinfo->init_holder(inst.ptr(), existing_holder);
+        tinfo->init_holder(inst.ptr(), existing_holder, nullptr);
 
         internals.registered_instances.emplace(wrapper->value, inst.ptr());
 
diff --git a/include/pybind11/class_support.h b/include/pybind11/class_support.h
@@ -244,6 +244,36 @@ extern "C" inline void pybind11_object_dealloc(PyObject *self) {
     Py_TYPE(self)->tp_free(self);
 }
 
+/// Swaps the pybind internals of one instance into another.  `a` and `b` must be allocated,
+/// registered instances of the same type.  The caller should typically ensure that the instances are
+/// uniquely referenced (foreign references are cannot be updated).
+///
+/// The holder transfer must be done separately after the call.
+inline void instance_swap(instance_essentials<void> *from, instance_essentials<void> *to) {
+    auto type = Py_TYPE(to);
+    if (type != Py_TYPE(from))
+        pybind11_fail("instance_swap(): Cannot swap instances of different types");
+    if (!to->value || !from->value)
+        pybind11_fail("instance_swap(): Cannot swap unallocated instances");
+
+    auto &registered_instances = get_internals().registered_instances;
+    std::pair<const void *const, void *> *to_reg = nullptr, *from_reg = nullptr;
+    auto range = registered_instances.equal_range(to->value);
+    for (auto it = range.first; it != range.second; ++it)
+        if (type == Py_TYPE(it->second)) { to_reg = &*it; break; }
+    range = registered_instances.equal_range(from->value);
+    for (auto it = range.first; it != range.second; ++it)
+        if (type == Py_TYPE(it->second)) { from_reg = &*it; break; }
+    if (!to_reg || !from_reg)
+        pybind11_fail("instance_swap(): Cannot swap unregistered instances");
+
+    std::swap(to->value, from->value);
+    std::swap(to->weakrefs, from->weakrefs);
+    if (type->tp_dictoffset != 0)
+        std::swap(*_PyObject_GetDictPtr((PyObject *) to), *_PyObject_GetDictPtr((PyObject *) from));
+    std::swap(to_reg->second, from_reg->second);
+}
+
 /** Create a type which can be used as a common base for all classes with the same
     instance size, i.e. all classes with the same `sizeof(holder_type)`. This is
     needed in order to satisfy Python's requirements for multiple inheritance.
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
@@ -268,6 +268,13 @@ class cpp_function : public function {
         rec->is_constructor = !strcmp(rec->name, "__init__") || !strcmp(rec->name, "__setstate__");
         rec->nargs = (std::uint16_t) args;
 
+        if (rec->is_constructor && !rec->is_method) {
+            // Handle a def_static(__init__) constructor: we expose this to Python as an instance
+            // method, then deal with the static call internally.
+            rec->is_factory_constructor = true;
+            rec->is_method = true;
+        }
+
 #if PY_MAJOR_VERSION < 3
         if (rec->sibling && PyMethod_Check(rec->sibling.ptr()))
             rec->sibling = PyMethod_GET_FUNCTION(rec->sibling.ptr());
@@ -399,8 +406,10 @@ class cpp_function : public function {
         using namespace detail;
 
         /* Iterator over the list of potentially admissible overloads */
-        function_record *overloads = (function_record *) PyCapsule_GetPointer(self, nullptr),
-                        *it = overloads;
+        auto func_capsule = reinterpret_borrow<capsule>(self);
+        function_record *overloads = func_capsule,
+                        *it = overloads,
+                        *winner = nullptr; // Stores the one we actually use
 
         /* Need to know how many arguments + keyword arguments there are to pick the right overload */
         const size_t n_args_in = (size_t) PyTuple_GET_SIZE(args_in);
@@ -444,15 +453,21 @@ class cpp_function : public function {
                 if (func.has_args) --pos_args;   // (but don't count py::args
                 if (func.has_kwargs) --pos_args; //  or py::kwargs)
 
-                if (!func.has_args && n_args_in > pos_args)
+                // If this overload is a factory function masquerading as a constructor we need to
+                // skip the initial (uninitialized) self argument.
+                bool skip_first = func.is_factory_constructor;
+
+                const size_t n_args = n_args_in - skip_first;
+
+                if (!func.has_args && n_args > pos_args)
                     continue; // Too many arguments for this overload
 
-                if (n_args_in < pos_args && func.args.size() < pos_args)
+                if (n_args < pos_args && func.args.size() < pos_args)
                     continue; // Not enough arguments given, and not enough defaults to fill in the blanks
 
                 function_call call(func, parent);
 
-                size_t args_to_copy = std::min(pos_args, n_args_in);
+                size_t args_to_copy = std::min(pos_args, n_args);
                 size_t args_copied = 0;
 
                 // 1. Copy any position arguments given.
@@ -464,7 +479,7 @@ class cpp_function : public function {
                         break;
                     }
 
-                    call.args.push_back(PyTuple_GET_ITEM(args_in, args_copied));
+                    call.args.push_back(PyTuple_GET_ITEM(args_in, args_copied + skip_first));
                     call.args_convert.push_back(args_copied < func.args.size() ? func.args[args_copied].convert : true);
                 }
                 if (bad_kwarg)
@@ -524,7 +539,7 @@ class cpp_function : public function {
                         size_t args_size = n_args_in - args_copied;
                         extra_args = tuple(args_size);
                         for (size_t i = 0; i < args_size; ++i) {
-                            handle item = PyTuple_GET_ITEM(args_in, args_copied + i);
+                            handle item = PyTuple_GET_ITEM(args_in, args_copied + i + skip_first);
                             extra_args[i] = item.inc_ref().ptr();
                         }
                     }
@@ -563,8 +578,10 @@ class cpp_function : public function {
                     result = PYBIND11_TRY_NEXT_OVERLOAD;
                 }
 
-                if (result.ptr() != PYBIND11_TRY_NEXT_OVERLOAD)
+                if (result.ptr() != PYBIND11_TRY_NEXT_OVERLOAD) {
+                    winner = &func;
                     break;
+                }
 
                 if (overloaded) {
                     // The (overloaded) call failed; if the call has at least one argument that
@@ -591,8 +608,10 @@ class cpp_function : public function {
                         result = PYBIND11_TRY_NEXT_OVERLOAD;
                     }
 
-                    if (result.ptr() != PYBIND11_TRY_NEXT_OVERLOAD)
+                    if (result.ptr() != PYBIND11_TRY_NEXT_OVERLOAD) {
+                        winner = const_cast<function_record *>(&call.func);
                         break;
+                    }
                 }
             }
         } catch (error_already_set &e) {
@@ -638,20 +657,32 @@ class cpp_function : public function {
                 msg += "    "+ std::to_string(++ctr) + ". ";
 
                 bool wrote_sig = false;
-                if (overloads->is_constructor) {
-                    // For a constructor, rewrite `(self: Object, arg0, ...) -> NoneType` as `Object(arg0, ...)`
+                if (it2->is_constructor) {
                     std::string sig = it2->signature;
-                    size_t start = sig.find('(') + 7; // skip "(self: "
-                    if (start < sig.size()) {
-                        // End at the , for the next argument
-                        size_t end = sig.find(", "), next = end + 2;
+                    if (!it2->is_factory_constructor) {
+                        // For a constructor, rewrite `(self: Object, arg0, ...) -> NoneType` as `Object(arg0, ...)`
+                        size_t start = sig.find('(') + 7; // skip "(self: "
+                        if (start < sig.size()) {
+                            // End at the , for the next argument
+                            size_t end = sig.find(", "), next = end + 2;
+                            size_t ret = sig.rfind(" -> ");
+                            // Or the ), if there is no comma:
+                            if (end >= sig.size()) next = end = sig.find(')');
+                            if (start < end && next < sig.size()) {
+                                msg.append(sig, start, end - start);
+                                msg += '(';
+                                msg.append(sig, next, ret - next);
+                                wrote_sig = true;
+                            }
+                        }
+                    }
+                    else {
+                        // A factory function masquerading as a constructor; rewrite
+                        // `(arg0: whatever...) -> ClassName` as `ClassName(arg0: whatever...)`
                         size_t ret = sig.rfind(" -> ");
-                        // Or the ), if there is no comma:
-                        if (end >= sig.size()) next = end = sig.find(')');
-                        if (start < end && next < sig.size()) {
-                            msg.append(sig, start, end - start);
-                            msg += '(';
-                            msg.append(sig, next, ret - next);
+                        if (ret < sig.size()) {
+                            msg.append(sig, ret + 4, sig.npos);
+                            msg.append(sig, 0, ret);
                             wrote_sig = true;
                         }
                     }
@@ -690,12 +721,46 @@ class cpp_function : public function {
             msg += it->signature;
             PyErr_SetString(PyExc_TypeError, msg.c_str());
             return nullptr;
-        } else {
-            if (overloads->is_constructor) {
-                /* When a constructor ran successfully, the corresponding
-                   holder type (e.g. std::unique_ptr) must still be initialized. */
+        } else { // Call succeeded
+            if (winner->is_constructor) {
                 auto tinfo = get_type_info(Py_TYPE(parent.ptr()));
-                tinfo->init_holder(parent.ptr(), nullptr);
+                if (!winner->is_factory_constructor) {
+                    /* When an ordinary constructor ran successfully, the corresponding
+                       holder type (e.g. std::unique_ptr) must still be initialized. */
+                    tinfo->init_holder(parent.ptr(), nullptr, nullptr);
+                }
+                else {
+                    /* For a factory function exposed as a constructor, the corresponding pointer
+                       and holder must be transferred from the returned object into the allocated
+                       instance */
+                    auto *result_inst = (detail::instance_essentials<void> *) result.ptr(),
+                         *parent_inst = (detail::instance_essentials<void> *) parent.ptr();
+                    std::string failure;
+                    // Make sure the factory function gave us exactly the right type:
+                    if (Py_TYPE(result.ptr()) != tinfo->type)
+                        failure = std::string("static __init__() should return '") + tinfo->type->tp_name +
+                            "', not '" + Py_TYPE(result.ptr())->tp_name + "'";
+                    // The factory function must give back a unique reference:
+                    else if (result.ref_count() != 1)
+                        failure = "static __init__() returned an object with multiple references";
+                    // Guard against accidentally specifying a reference r.v. policy or similar:
+                    else if (!result_inst->holder_constructed && !result_inst->owned)
+                        failure = "static __init__() failed: cannot construct from an unowned reference";
+
+                    if (!failure.empty()) {
+                        result.dec_ref();
+                        PyErr_SetString(PyExc_TypeError, failure.c_str());
+                        return nullptr;
+                    }
+
+                    // Swap the pointer and other internals, then transfer the holder:
+                    detail::instance_swap(result_inst, parent_inst);
+                    tinfo->init_holder(parent.ptr(), nullptr, result.ptr());
+                    // We transfered the value out of result, so let it be destroyed:
+                    result.dec_ref();
+
+                    result = none().release();
+                }
             }
             return result.ptr();
         }
@@ -1131,10 +1196,15 @@ class class_ : public detail::generic_type {
         }
     }
 
-    /// Initialize holder object of an instance, possibly given a pointer to an existing holder
-    static void init_holder(PyObject *inst_, const void *holder_ptr) {
+    /// Initialize holder object of an instance, possibly given a pointer to an existing holder or
+    /// an alternative instance to transfer the holder from
+    static void init_holder(PyObject *inst_, const void *holder_in, PyObject *holder_from) {
         auto inst = (instance_type *) inst_;
-        init_holder_helper(inst, (const holder_type *) holder_ptr, inst->value);
+        const holder_type *holder_ptr = (const holder_type *) holder_in;
+        if (!holder_ptr && holder_from)
+            holder_ptr = &((instance_type *) holder_from)->holder;
+
+        init_holder_helper(inst, holder_ptr, inst->value);
     }
 
     static void dealloc(PyObject *inst_) {
diff --git a/tests/test_methods_and_attributes.cpp b/tests/test_methods_and_attributes.cpp
diff --git a/tests/test_methods_and_attributes.py b/tests/test_methods_and_attributes.py
