Undocumented required default constructor for types with type casters

[ezyang]

Issue description

When defining a custom type caster as per http://pybind11.readthedocs.io/en/master/advanced/cast/custom.html it is required that the type you are casting to/from has a default constructor. Upon careful consideration of the sample code, it is not surprising that this is the case, but it is not documented, and it gives a rather opaque template expansion error:

call to implicitly-deleted default constructor of 'value_conv'

Reproducible example code

Do the same example as in the docs, but delete the default constructor.

struct inty { long long_value; inty(long v) : long_value(v) {} };

[dean0x7d]

Thanks for reporting. This is now documented in 93528f5. Note that this limitation should be resolved with #864 in v2.3.

There are ways to get around this limitation even with the current implementation of type_caster, but it involves opening up even more type_caster implementation details -- you may have seen these in PYBIND11_TYPE_CASTER. Inlining that macro and replacing the storage type of T value with std::optional<T> value or std::unique_ptr<T> value would do the trick, but note that this may break in future versions.

[jagerman]

To elaborate on the workaround a little (because I was asked externally), what you'd do is basically expand the PYBIND11_TYPE_CASTER macro in your own code, then make some modifications to change the value member into something like a unique_ptr<MyType>. (A C++17 std::optional<MyType> would work as well). Then you make some modifications to the rest of the expanded macro code to deal with this. Here's an example:

#include <pybind11/pybind11.h>

namespace py = pybind11;

struct Foo {
    Foo() = delete;
    Foo(int i) {}
};

namespace pybind11 { namespace detail {
template <> struct type_caster<Foo> {
/// Modified PYBIND11_TYPE_CASTER code:
protected:
    std::unique_ptr<Foo> value_ptr;
public:
    static constexpr auto name = _("Foo");
    template <typename T_, enable_if_t<std::is_same<Foo, remove_cv_t<T_>>::value, int> = 0>
    static handle cast(T_ *src, return_value_policy policy, handle parent) {
        if (!src) return none().release();
        if (policy == return_value_policy::take_ownership) {
            auto h = cast(std::move(*src), policy, parent); delete src; return h;
        } else {
            return cast(*src, policy, parent);
        }
    }
    operator Foo*() { return value_ptr.get(); }
    operator Foo&() {
        if (!value_ptr) throw std::runtime_error("Cannot extract value from nullptr");
        return *value_ptr;
    }
    operator Foo&&() && { return std::move((Foo &) *this); }
    template <typename T_> using cast_op_type = pybind11::detail::movable_cast_op_type<T_>;
/// End of modified PYBIND11_TYPE_CASTER code.

    bool load(handle src, bool) {
        // ... use `src` to extract and construct a Foo value inside the unique ptr, e.g.:
        value_ptr.reset(new Foo(123));
        return true;
    }

    static handle cast(Foo f, return_value_policy /* policy */, handle /* parent */) {
        // ... use `f` to create and return an appropriate Python handle
        return none().release();
    }
};
}}


PYBIND11_MODULE(tcnd, m) {
    m.def("f", []() { return Foo(123); });
    m.def("g", [](Foo &) { return 999; });
}

Note that, as @dean0x7d mentioned, there is a chance that this will break in v2.3 as the type caster system is expected to undergo an overhaul. We'll maintain compatibility with type casters using the currently documented approach (i.e. with the macro), but there might be some necessary changes to the macro itself for compatibility that you would need to incorporate to make this sort of code compatible with 2.3.

(I think, however, that as @dean0x7d mentioned, the new approach should eliminate the limitation, so a better (and probably simpler) approach for making the code work with 2.3 will be to just update custom casters to the new caster mechanism that will be added).

[kurt-skewton]

Thank you very much for further elaboration. I asked you as well as dean0x7d privately, but more than happy to continue here. I am trying your code, but having issues compiling the line:

static constexpr auto name = _("Foo");

I apologize for my ignorance, but what is the underscore here?

FWIW, I plan to use this to custom type cast types that look as follows: std::shared_ptr<some_array<double, 2>>, ideally.

Thanks

edit: I guess that my problem is that this does not compile with VS 2017 for me. I am getting:

1>------ Build started: Project: cmake_example, Configuration: Debug x64 ------
1>main.cpp
1>C:\dev\cmake_example\src\main.cpp(20): warning C4244: '*=': conversion from '__int64' to 'int', possible loss of data
1>C:\dev\cmake_example\src\main.cpp(42): error C2131: expression did not evaluate to a constant
1>C:\dev\cmake_example\src\main.cpp(42): note: failure was caused by call of undefined function or one not declared 'constexpr'
1>C:\dev\cmake_example\src\main.cpp(42): note: see usage of 'pybind11::detail::_'
1>Done building project "cmake_example.vcxproj" -- FAILED.
2>------ Skipped Build: Project: ALL_BUILD, Configuration: Debug x64 ------
2>Project not selected to build for this solution configuration
========== Build: 0 succeeded, 1 failed, 1 up-to-date, 1 skipped ==========

[kurt-skewton]

After fiddling around a bit, I replaced static constexpr auto name = _("Foo"); by
static PYBIND11_DESCR name() { return type_descr(_("Foo")); }
in the inlined macro and that compiled.

The code is throwing an exception however. The code now looks like this. Note that I had to use & in the reset statement at the end of the load function. When I debug into the load function, I am getting the required shared_ptr to the array. However, something then happens to to value_ptr downstream which I don't get. Any clue what is going on? Thanks.

namespace pybind11 {
	namespace detail {
		template <> struct type_caster<std::shared_ptr<monty::ndarray<double, 2>>> {
			/// Modified PYBIND11_TYPE_CASTER code:
		protected:
			std::unique_ptr<std::shared_ptr<monty::ndarray<double, 2>>> value_ptr;
		public:
			static PYBIND11_DESCR name() { return type_descr(_("std::shared_ptr<monty::ndarray<double, 2>>")); }
			template <typename T_, enable_if_t<std::is_same<std::shared_ptr<monty::ndarray<double, 2>>, remove_cv_t<T_>>::value, int> = 0>
			static handle cast(T_ *src, return_value_policy policy, handle parent) {
				if (!src) return none().release();
				if (policy == return_value_policy::take_ownership) {
					auto h = cast(std::move(*src), policy, parent); delete src; return h;
				}
				else {
					return cast(*src, policy, parent);
				}
			}
			operator std::shared_ptr<monty::ndarray<double, 2>>*() { return value_ptr.get(); }
			operator std::shared_ptr<monty::ndarray<double, 2>>&() {
				if (!value_ptr) throw std::runtime_error("Cannot extract value from nullptr");
				return *value_ptr;
			}
			operator std::shared_ptr<monty::ndarray<double, 2>> && () && { return std::move((std::shared_ptr<monty::ndarray<double, 2>> &)*this); }
			template <typename T_> using cast_op_type = pybind11::detail::movable_cast_op_type<T_>;
			/// End of modified PYBIND11_TYPE_CASTER code.

			bool load(handle src, bool convert) {
				// ... use `src` to extract and construct a Foo value inside the unique ptr, e.g.:
				if (!convert && !array_t<double>::check_(src))
					return false;

				auto buf = array_t<double, array::c_style | array::forcecast>::ensure(src);
				if (!buf)
					return false;

				auto dims = buf.ndim();
				if (dims != 2)
					return false;

				monty::shape_t<2> shape = monty::shape(buf.shape()[0], buf.shape()[1]);

				pybind11::buffer_info info = buf.request();
				auto ptr = static_cast<double *>(info.ptr);

				std::shared_ptr<monty::ndarray<double, 2>> ptr_nda = std::make_shared<monty::ndarray<double, 2>>(ptr, shape);
				//std::unique_ptr<std::shared_ptr<monty::ndarray<double, 2>>> ptr_ptr_nda = std::make_unique<std::shared_ptr<monty::ndarray<double, 2>>>(std::make_shared<monty::ndarray<double, 2>>(ptr, shape));
				value_ptr.reset(&ptr_nda);
				return true;
			}

			static handle cast(std::shared_ptr<monty::ndarray<double, 2>> src, return_value_policy, handle) {
				// ... use `f` to create and return an appropriate Python handle
				std::vector<size_t> shape(2);
				std::vector<size_t> strides(2);

				for (int i = 0; i<2; i++) {
					shape[i] = src->shape[i];
				}
				strides[0] = shape[1] * sizeof(double);
				strides[1] = sizeof(double);

				array a(std::move(shape), std::move(strides), src->raw());
				return none().release();
			}
		};
	}
}

void ftest(std::shared_ptr<monty::ndarray<double, 2>> p_a)
{
	auto s = p_a->shape;
}

[kurt-skewton]

I just did a quick test for the case of casting the array directly - no std::shared_ptr, and it seems to work fine, which is great, but the server code I am trying to comply with requires shared pointers to these arrays (don't ask me why). So I am looking for an issue with the std::shared_ptr wrap I guess.

I will try to debug carefully to diagnose where the problem occurs.

edit: As I debug into the code, I am finding that the reference count of the std::shared_ptr probably goes to zero as I leave the load function. (I put a break point in the destructor of the array being held on to and hit that break point.) I must be doing something idiotic. The statement here must be wrong:

std::shared_ptr<monty::ndarray<double, 2>> ptr_nda = std::make_shared<monty::ndarray<double, 2>>(ptr, shape);
value_ptr.reset(&ptr_nda);

[jagerman]

You might be able to adapt this to simply store the shared_ptr<Type> directly (instead of a unique_ptr<shared_ptr<Type>>. In fact, I wonder if that would work even with the default type caster macro (since a shared_ptr<T> is default constructible).

[kurt-skewton]

I weakly tried this before writing to you, if I remember well. The problem is that I don't have a default constructor for the underlying type. I thought at some point that the pybind11 mechanism might call the shared_ptr with nullptr, but it is actually looking for the default constructor of the underlying type.

But now I figured that I am just not handling std::shared_ptr and std::unique_ptr semantics well, so I did a bit of digging and the following lines now seem to work:

std::unique_ptr<std::shared_ptr<monty::ndarray<double, 2>>> ptr_ptr_nda = std::make_unique<std::shared_ptr<monty::ndarray<double, 2>>>(std::make_shared<monty::ndarray<double, 2>>(ptr, shape));
value_ptr.reset(ptr_ptr_nda.release());

Before, I guess I was resetting the std::unique_ptr with a naked pointer to the underlying std::shared_ptr, which is kind of nasty and the std:shared_ptr would ref count to 0 and be done. Now I am actually creating a std::unique_ptr, then am transferring ownership properly, releasing the old one and resetting the new one.

Thank you for all the help jagerman. It's greatly appreciated.