Compilation error when using public virtual parent class

[kliegeois]

In one of the projects I am working on, there are errors while compiling the Pybind11 files generated by binder such as pybind11/pybind11.h:1487:12: error: pointer to member conversion via virtual base .

There is a work around:

https://github.com/kliegeois/binder/commit/a1cece2e2b12a8056910eb4b9d793b783c0f59c1

that solves the problem and consists in skipping a public parent class if it has the virtual keyword.

@lyskov what are you thought about this?

[lyskov]

This could be a Binder issue (ie it bind something that not actually bindable). I could look it up but to do so i will need a minimal example to replicate the issue. Thanks,

[kliegeois]

I will try to write a minimal example for this, thanks!

[kliegeois]

I think that #169 is closely related to this issue.

[kliegeois]

I am currently working on a minimal example.

[kliegeois]

I finally have a minimal example:

#ifndef TEST_STRUCT_H
#define TEST_STRUCT_H

#include <string>
#include <vector>
#include <memory>

namespace testers {

template <int I>
class Animal {
public:
    virtual ~Animal() = default;
    virtual void Eat() const = 0;
    bool isAnimal() { return true;}
    virtual bool isDog() const;
};

template <int I>
bool Animal<I>::isDog() const {
    return false;
}

template <int I>
class Mammal : public virtual Animal<I> {
public:
    virtual void Breathe() const = 0;
};

template <int I>
class Dog: public virtual Mammal<I> {
public:
    void Eat() const override { printf("Eat\n");}
    void Breathe() const override { printf("Breathe\n");} 
    bool isDog() const override;
};

template <int I>
bool
Dog<I>::isDog() const {
    return true;
}

template <int I>
bool isThisAnimalADog(const std::shared_ptr<Animal<I>> &a) { return a->isDog();}

inline void initiate(Dog<1> p) {};
template bool isThisAnimalADog<1>(const std::shared_ptr<Animal<1>> &a);

} // namespace testers

#endif

This input file generates the following pybind11 output:

    { // testers::Mammal file:test_struct_2.hpp line:27
        pybind11::class_<testers::Mammal<1>, Teuchos::RCP<testers::Mammal<1>>, PyCallBack_testers_Mammal_1_t, testers::Animal<1>> cl(M("testers"), "Mammal_1_t", "");
        cl.def( pybind11::init( [](){ return new PyCallBack_testers_Mammal_1_t(); } ) );
        cl.def(pybind11::init<PyCallBack_testers_Mammal_1_t const &>());
        cl.def("Breathe", (void (testers::Mammal<1>::*)() const) &testers::Mammal<1>::Breathe, "C++: testers::Mammal<1>::Breathe() const --> void");
        cl.def("assign", (class testers::Mammal<1> & (testers::Mammal<1>::*)(const class testers::Mammal<1> &)) &testers::Mammal<1>::operator=, "C++: testers::Mammal<1>::operator=(const class testers::Mammal<1> &) --> class testers::Mammal<1> &", pybind11::return_value_policy::automatic, pybind11::arg(""));
        cl.def("Eat", (void (testers::Animal<1>::*)() const) &testers::Animal<1>::Eat, "C++: testers::Animal<1>::Eat() const --> void");
        cl.def("isAnimal", (bool (testers::Animal<1>::*)()) &testers::Animal<1>::isAnimal, "C++: testers::Animal<1>::isAnimal() --> bool");
        cl.def("isDog", (bool (testers::Animal<1>::*)() const) &testers::Animal<1>::isDog, "C++: testers::Animal<1>::isDog() const --> bool");
        cl.def("assign", (class testers::Animal<1> & (testers::Animal<1>::*)(const class testers::Animal<1> &)) &testers::Animal<1>::operator=, "C++: testers::Animal<1>::operator=(const class testers::Animal<1> &) --> class testers::Animal<1> &", pybind11::return_value_policy::automatic, pybind11::arg(""));
    }

(Note the fact that binder is generating 4 functions associated to Animal (those functions generate the errors)).

If we use the work around https://github.com/kliegeois/binder/commit/a1cece2e2b12a8056910eb4b9d793b783c0f59c1 , the output is the following:

    { // testers::Mammal file:test_struct_2.hpp line:27
        pybind11::class_<testers::Mammal<1>, Teuchos::RCP<testers::Mammal<1>>, PyCallBack_testers_Mammal_1_t, testers::Animal<1>> cl(M("testers"), "Mammal_1_t", "");
        cl.def( pybind11::init( [](){ return new PyCallBack_testers_Mammal_1_t(); } ) );
        cl.def(pybind11::init<PyCallBack_testers_Mammal_1_t const &>());
        cl.def("Breathe", (void (testers::Mammal<1>::*)() const) &testers::Mammal<1>::Breathe, "C++: testers::Mammal<1>::Breathe() const --> void");
        cl.def("assign", (class testers::Mammal<1> & (testers::Mammal<1>::*)(const class testers::Mammal<1> &)) &testers::Mammal<1>::operator=, "C++: testers::Mammal<1>::operator=(const class testers::Mammal<1> &) --> class testers::Mammal<1> &", pybind11::return_value_policy::automatic, pybind11::arg(""));
    }

and can be compiled without errors (and then used as expected).

The fact that there are templates in this example is required to reproduce this behavior.

[lyskov]

Thank you @kliegeois , - i am looking this up!

Re templates just to double check that i understood you correctly: - are you saying that making classes concrete (ie removing templates) from the example above immediately fix the issue? Thanks,

[kliegeois]

Thanks @lyskov ! Yes, if we make the classes from the example concrete, the issue is gone. It seems that the three known cases where the issue occurred (the example, the project I am working on, and https://github.com/RosettaCommons/binder/issues/169 ) have templates.

[lyskov]

Interesting issue! It is not clear to me why compiler refuse to allow type conversion when base class is template... - maybe it have not instantiated v-table for it? (it should though since we explicitly instantiated Dog<1> and so it have to instantiate it chain of destructors and all v-table's). Please let me know if you have other thoughts on this.

From practical point of view: i think your workaround is probably the best way to address this so i will push it into master soon. - thank you for putting this together and providing solution @kliegeois !

[lyskov]

fixed in 92d183a1ed0ff830b1ace245c49956751dca8d2f

[kliegeois]

No problem @lyskov, I am happy to help and that you approved my workaround!

I have no other thoughts about this. Thanks for your investigation and for the push.