std::reference_wrapper: (some) non-generic type support; failure on None

[jagerman]

This reimplements the std::reference_wrapper<T> caster to be a shell around the underlying T caster (rather than assuming T is a generic type), which lets it work for things like std::reference_wrapper<int> or anything else custom type caster with a lvalue cast operator.

Fixes #848: this also makes it properly fail when None is provided, just as an ordinary lvalue reference argument would similarly fail.

This also adds a static assert to test that T has an appropriate type caster. It triggers for casters like std::pair, which have return-by-value cast operators. (In theory this could be supported by storing a local temporary for such types, but that's beyond the scope of this PR). This shouldn't break anything in terms of backwards compatibility: currently reference_wrapper only actually works for generic types, which always have an lvalue cast operator.

[jagerman]

Struck by random CI failures on both appveyor and travis on this one. (Restarted the travis, and the appveyor is just a single #792 failure).

[dean0x7d]

This existed before this PR, but I'm just noticing: casting as a pointer here means that rvp::automatic will behave like rvp::take_ownership which goes completely against std::reference_wrapper. Would there be anything wrong with just hard coding rvp::reference here? I don't think any other policy could possibly be valid.

[jagerman]

Sounds reasonable to me.

[jagerman]

So I agree that rvp::automatic and take_ownership seem wrong and should be masked, but copy could be valid. Perhaps it would be better to just set it to rvp::reference if it's set to automatic or take_ownership, and otherwise leave it alone?

[dean0x7d]

When is copy valid?

[jagerman]

copy is the default currently if you py::cast(x) a reference_wrapper<T> x. Essentially both copy and reference currently work the way I'd expect:

#include <pybind11/pybind11.h>

namespace py = pybind11;

struct Foo { int value = 2; };

std::vector<std::reference_wrapper<Foo>> foos() {
    static Foo foo1, foo2;
    std::vector<std::reference_wrapper<Foo>> r;
    r.emplace_back(foo1);
    r.emplace_back(foo2);
    return r;
}

PYBIND11_PLUGIN(copy) {
    py::module m("copy");

    py::class_<Foo>(m, "Foo").def(py::init<>()).def_readwrite("value", &Foo::value);

    m.def("z1", []() {
        py::list l;
        for (auto &f : foos()) l.append(py::cast(f, py::return_value_policy::copy));
        return l;
    });
    m.def("z2", []() {
        py::list l;
        for (auto &f : foos()) l.append(py::cast(f, py::return_value_policy::reference));
        return l;
    });

    return m.ptr();
}

>>> from copy import z1, z2, Foo
>>> a = z1()
>>> a2 = z1()
>>> b = z2()
>>> b2 = z2()
>>> a[0] is a2[0]
False
>>> b[0] is b2[0]
True

(The ::copy there is redundant: it'll be the default for an lvalue reference wrapper already.)

[jagerman]

Though now that I think about it, I'm not sure why we pass a pointer here at all, instead of just passing an lvalue reference (i.e. changing &src.get() -> src.get()).

[dean0x7d]

Doesn't &src.get() -> src.get() make the following a copy?

T x = ...; // T is a user-defined type
py::function f = ...;
f(std::ref(x)); // <-- rvp::automatic_reference: the `cast(const&)` overload does a copy 

[jagerman]

That doesn't actually copy (because of the rvalue), but if you change the reference wrapper argument to an lvalue it is indeed a problem.

I'll change back to the pointer cast, with a take_ownership mask.

[dean0x7d]

Looks like there's a conflict now. But otherwise, looks good to me.