Add a life support system for type_caster temporaries (+ binary size reduction)

[dean0x7d]

The first commit here changes the handling of temporary objects created by type_caster::load(). It uses an RAII loader_life_support class, placed in dispatcher, which holds a list of patients only for the lifetime of the function call. This has two nice advantages:

1. Supercasters don't need to care about maintaining the life of their subcasters.
2. Switching type_caster_generic to this system reduced the binary size of the test module by 3-4% (!), depending on the compiler. The main reason seems to be the removal of object temp from type_caster_generic, which makes the struct trivially copyable.

The second commit fixes #916 (alternative to #917) by using the new system in the Eigen::Ref caster.

[wjakob]

This generally looks great. However, one issue that should be considered here is that local static variables like static PyObject *ptr = nullptr; aren't going to be mutually visible in larger projects that may involve many separate modules. This is really the purpose of the internals data structure, thus I would suggest to keep it there.

[jagerman]

I was thinking of something like this; 👍 for implementing it. Also good for catching the py::cast issue; I was pretty sure the following was unsafe (and not addressed by #917):

using namespace Eigen;
void some_func(Ref<MatrixXd> x) { /* do something with x */ }
some_func(py::cast<Ref<MatrixXd>>(myarray));

With this PR that now raises an exception, which is good (it would be nicer still to make that work, but that doesn't really seem feasible).

The one issue I see is that with the life support instance being outside the dispatch loop, which means there's a single lifeboat for all overloads across both passes, rather than just the one that finally makes it (e.g. because the overload fails on some later argument, but the earlier argument already added a lifeboat entry). I think it would suffice to make the guard localized to the function calls, i.e. to put the guard localized to the try { ... } that calls the function (the first pass just after // 6. Call the function and the second in the loop over second_pass).

As for lifetime, I'm a bit concerned that this approach has the potential to vastly extend the lifetime of temporaries: it seems as though no pybind11 function call is ever going to result in freeing the temporaries when there is some pybind11 function call somewhere up the stack. If all my pybind11 functions are short, that's not an issue, but if a pybind function call sits high up the stack (e.g. perhaps it calls a python function that does significant work) this is going to build up temporaries without freeing them for potentially a very long time.

An alternative approach would be to get rid of the nesting and counter, and have loader_life_support contain a static forward_list<PyObject *> stack: construction pops a new (nullptr) value on the stack, destruction pops it off, and adding a new item appends, vivifying a new list at the beginning of the stack when needed. (It might be simpler and perhaps more efficient to just use a forward_list<forward_list<py::object>>: construction does an stack.emplace_front(), destruction does stack.pop_front(), and adding does a stack.front().emplace_front(h).

[dean0x7d]

Very good points. I've made the appropriate changes. The patients are now stashed in internals and they should have the minimal required lifetime.

I've now added tests for py::cast with temporary values and this does actually work without error in the majority of cases because the temp objects are supported by the same life support frame as the enclosing function. So things like this work now:

py::class_<A>(m, "A");
py::class_<B>(m, "B").def(py::init<A>());
py::implicitly_convertible<A, B>();

// when inside a bound function...
m.def("foo", []() {
    auto obj = py::cast(A{});
    const auto &b = obj.cast<const B&>(); // reference to temporary
    // ... use b -- previously dangling reference, now works

    auto str_obj = ...; // some string which requires an encoding change
    auto s = str_obj.cast<std::string_view>();
    // ... use s -- also works now
});

Is there any reason to not allow this? I suppose there could be a danger of creating too many temp objects (which are only destroyed on return). This could be explicitly disabled with an additional guard in py::cast, but it might actually be nice to keep it.

[jagerman]

This seems like it's going to be creating a lot of lists here, the vast majority of which are never used (i.e. life support is the exception rather than the rule). Is there a reason to prefer always allocating here to allocating the list on demand when the first add_patient is called?

[dean0x7d]

Indeed, there's no need to allocate the list in the majority of cases. Fixed.

[jagerman]

Ah, I hadn't thought about the life support already being present; that's a nice feature.

[jagerman]

This looks good to me!