Enable enable_shared_from_this support for casts

[jagerman]

Currently we support enable_shared_from_this only if we end up taking over an existing reference or pointer; this extends the support by looking for std::enable_shared_shared_from_this inheritance for any type of cast except for an explicit return_value_policy::copy.

This lets pybind avoid a copy when doing something like:

    m.def("f", []() {
        auto b = std::make_shared<B>();
        foo(*b);
    }

where foo is some function that takes the B by lvalue reference, but ends up requiring casting it to Python. (In the motivating example for this PR the specific case was a trampoline overload).

The .so impact is minimal (+8192 bytes including the added test code) as the compiler should be able to optimize away the check entirely for non-shared-from-this classes.

Cc: @florianwechsung for input.

[florianwechsung]

Is it possible to do the check if B implements enable_shared_from_this at runtime?
I'm asking for the following scenario:

 
// These classes are defined in the C++ library I want to wrap.
// I would like to avoid changes as these have to be discussed with upstream.
class Bbase {
...
};
class A {
    public:
        A() {}
        virtual void plus1(Bbase& b) = 0;
};

This is my wrapping code:

class B : public Bbase, public std::enable_shared_from_this<B> {
};
  
class PyA: public A{
    public:
 
        virtual void plus1(Bbase & b)
        {            
            PYBIND11_OVERLOAD_PURE(void, A, plus1, b);
        }
};

PYBIND11_MODULE(example, m) {
    m.doc() =  "pybind11 example plugin";
 
    pybind11::class_<A, std::shared_ptr<A>, PyA>(m, "A")
        .def(pybind11::init<>());
 
    pybind11::class_<Bbase, std::shared_ptr<Bbase>>(m, "Bbase");
        
    pybind11::class_<B, std::shared_ptr<B>, Bbase>(m, "B")
        .def(pybind11::init<>());
 
    m.def("test1", [](A& instance){
            auto b = std::make_shared<B>();
            instance.plus1(*b);
            });

This will introduce the unwanted copy construction, as Bbase does not inherit from std::enable_shared_from_this.

Could the check be done at runtime instead?

[jagerman]

I don't see how a runtime check is possible practically (at least without a ridiculous amount of overhead)--we have a structure:

Bbase     std::enable_s_f_t<B>
     \   /
       B

with no way to determine, from a Bbase &, that the instance is actually some enable_shared_from_this<T> for some T. But even if we could (for example, by checking all of Bbase's known descendant classes), we'd still be talking about a huge run-time penalty incurred on every cast for a feature that is only very rarely used.

The current approach works by deducing T, but such deduction only works when we are working with B, not when we work from Bbase.

The only way I could practically see to do this would be to have some internals data that stores a list of subclass-via-shared_from_this lambdas that get assigned when a registered subclass for a type inheriting from shared_from_this is added. But even that would be fairly heavy, and probably not all that easy to implement.

[florianwechsung]

Ah I see, yeah that makes sense and is much more involved. I'll add std::enable_shared_from_this to the class in the main C++ source then. Thanks a lot!

[florianwechsung]

It appears that this breaks the ability to use a nullptr as default for B.

#include <pybind11/pybind11.h>
#include <memory>
#include <iostream>
namespace py = pybind11;

class B : public std::enable_shared_from_this<B> {
    public:
        B() {std::cout << "Bconstructor " << std::endl;}
        B(B const & src) { 
            std::cout << "B copy constructor " << std::endl;
        }
        ~B() {
            std::cout << "B destructor " << std::endl;
        }

};

class A {
    public:
        A() {}
        virtual void plus(B& b) = 0;
        void with_default(std::shared_ptr<B> b = nullptr) {
        }
};

class PyA: public A{
    public:

        virtual void plus(B & b)
        {
            std::cout << "In plus2 (c++)" << std::endl;
            PYBIND11_OVERLOAD_PURE(void, A, plus2, b);
        }
};





PYBIND11_MODULE(example, m) {
    m.doc() =  "pybind11 example plugin";

    pybind11::class_<B, std::shared_ptr<B>>(m, "B")
        .def(py::init<>());

    pybind11::class_<A, std::shared_ptr<A>, PyA>(m, "A")
        .def(py::init<>())
        .def("with_default", &A::with_default, py::arg("b")=(B*) nullptr);

    m.def("test", [](A& instance){
            auto b = std::make_shared<B>();
            instance.plus(*b);
            });
}

This gives a segfault when I import example in python.

[jagerman]

I think c60a114 should fix that.

[florianwechsung]

Yep it does! Thanks

[florianwechsung]

Hi @jagerman and @wjakob, can we get this PR merged?
The merge conflict seems to be fairly trivial. Maybe @jagerman can rebase onto master or alternatively I'm happy to do this myself and open a new PR. Or is there any other reason why this hasn't been merged yet?