def_property and virtual inheritance

[cmdawson]

The below code is an adaption of #959. The base virtual base class has a member variable and the usual c++ accessor, and for now only the base A and the two classes B0_ and B1 which derive immediately from it are wrapped.

struct A {
    std::vector<int> a_;
    std::vector<int>& a(void) { return a_; }
};
struct B0_ : public virtual A {};
struct B1 : public virtual A {};
struct C : public B0_, public B1
{ C() { std::cout << "in constructor" << std::endl; } };

PYBIND11_MAKE_OPAQUE(std::vector<int>);

PYBIND11_MODULE(cpp, m)
{
  py::bind_vector<std::vector<int>>(m, "IntVector");

  py::class_<A>(m, "A")
    .def_property("a", &A::a, &A::a);
  py::class_<B0_, A>(m, "B0")
    .def(py::init<>());
  py::class_<B1, A>(m, "B1");
  // py::class_<C, B0_, B1>(m, "C");
}

Running the following in python

import cpp
G = cpp.B0()
n = len(G.a)

raises the following exception

Traceback (most recent call last):
  File "./test.py", line 6, in <module>
    print(len(G.a))
OverflowError: cannot fit 'int' into an index-sized integer

However, after uncommenting the line py::class_<C, B0_, B1>(m, "C"); to expose the final class in the diamond pattern the overflow exception is no longer raised and everything behaves as expected.

I'm using pybind11 master e7761e3 and have tested this on OSX with LLVM version 9.0.0 (clang-900.0.39.2), and also on linux with gcc version 7.3.1 20180303.

[jagerman]

You'll need to add a py::multiple_inheritance() to your py::class_<B0_, A> and py::class_<B1_, A> constructors to tell pybind that simple inheritance assumptions won't work here:

    py::class_<B0_, A>(m, "B0", py::multiple_inheritance())

[cmdawson]

That's awesome, many thanks. Sorry it never occurred to me to try that since B0_ only has the single base. Makes sense in hindsight, is it worth me adding a sentence to the docs in that section?

[bstaletic]

I'd support this addition to the docs, because this didn't seem obvious even though I've read about the py::multiple_iheritance().

[jagerman]

In theory it ought to be possible to detect this sort of virtual inheritance and apply it automatically, using something along the lines of the second answer here: https://stackoverflow.com/questions/2893791/c-can-virtual-inheritance-be-detected-at-compile-time

[cmdawson]

Do you mean using some adaption of boost::is_virtual_base_of and using it to conditionally call traverse_offset_bases ?

[jagerman]

Yes (except that we don't depend on boost, so we need our own implementation of is_virtual_base_of in detail/common.h).

[Social-Mean]

Yes (except that we don't depend on boost, so we need our own implementation of is_virtual_base_of in detail/common.h).

I cannot find is_virtual_base_of in detail/common.h. And this bug has not been fixed up to now.

Can anyone fix it?