Regression when binding alias functions

[jagerman]

There are a couple of regressions in current master versus 2.1 with the following:

#include <iostream>
#include <pybind11/pybind11.h>

namespace py = pybind11;

struct Foo {
protected:
    int foo() { return 42; }
public:
//    virtual ~Foo() = default;
};

struct PyFoo : Foo {
public:
    int foo() { return Foo::foo(); }
};

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

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

    return m.ptr();
}

which was a nice trick to allow exposing protected base class methods to Python.

The first regression is that the above fails a static assertion because Foo isn't polymorphic. But I think that if using py::init_alias<>() for constructors, or for aliases that have just simply inherited constructors and no member variables, a non-polymorphic class is actually okay.

The second problem is that the above fails because of the method_adapter changes added in #855: the cast from the deduced PyFoo member function pointer isn't castable to the base Foo member function pointer type.

Admittedly this is a bit fragile; we could also solve them by providing a nicer way to binding protected methods, if someone can think of one.

[dean0x7d]

This seems like it was walking into undefined behavior in v2.1 (Foo, PyFoo are probably the same in most implementations but not technically by the standard). It's better that the static asserts are catching this now.

I'm not sure about a simple way to do this. It strikes me that protected/private members aren't really prime candidates for bindings.

[jagerman]

private definitely not. protected being uncallable is something of a nuissance. I think we might be able to work around it, though, if the dispatcher learns to distinguish between a Foo and PyFoo instance; the dispatcher could skip PyFoo methods when the alias isn't actually initialized.

Edit: it would still need a public alias method that forwards to the protected version, of course, but that seems a reasonable limitation.

[dean0x7d]

I think the following might be the easiest way to do this (and with the least amount of trickery):

class A {
protected:
    int foo() { return 42; }
};

class PublicA : public A {
public:
    using A::foo;
};

PYBIND11_MODULE(expose, m) {
    py::class_<A>(m, "A")
        .def(py::init<>())
        .def("foo", &PublicA::foo);
}

This works because std::is_same<&PublicA::foo, &A::foo>(), i.e. A::foo is not a new function which shadows the base, it's the exact same function which has just been assigned a different access modifier in PublicA.

The access mode change looks like a hack, but it should be fairly well defined in the language. This has the advantage of not messing around with alias instances at all. PublicA just grants us access to the protected functions.

[jagerman]

Huh, I didn't know that (I had thought using on a member imported with the parent's access, as happens with constructors).

[garth-wells]

I have a use case where A::foo is a virtual function that is overloaded from Python, but for which I still want access to be able to call the base class implementation A::foo. My solution at the moment is to wrap A::foo in the trampoline class and then do

py::class_<A, Trampoline>(m, "A")
.def("py_name", [](Trampoline& self)
     { /* call protected function exposed in Trampoline */ });

[dean0x7d]

This should do the trick:

class A {
public:
    virtual ~A() = default;

protected:
    virtual int foo() { return 42; }
};

class PublicA : public A {
public:
    using A::foo;
};

class Trampoline : public A {
public:
    int foo() override { PYBIND11_OVERLOAD(int, A, foo, ); }
};

PYBIND11_MODULE(cmake_example, m) {
    py::class_<A, Trampoline>(m, "A")
        .def(py::init<>())
        .def("foo", &PublicA::foo);
}

Again, PublicA is used only to bypass the protected access modifier. Apart from that, everything is the same as described in the docs for public virtual functions.