pmed / v8pp

Bind C++ functions and classes into V8 JavaScript engine
http://pmed.github.io/v8pp/
Other
898 stars 120 forks source link

classbinding for classes with multiple inheritance #62

Closed padjon closed 6 years ago

padjon commented 7 years ago

If an object inherits from multiple classes, and the defined method mapping is not part of the first declared inheritance, this leads to accessing the wrong address.

for example: class CInheriting: public Class1, public Class2 {};

v8pp::class_ CInheriting_class(isolate); CInheriting_class .set("Class1Method", &Class1Method) << Works .set("Class2Method", &Class2Method) << Fails

Tried in VS2017

pmed commented 7 years ago

Hi @padjon,

Thank you for the bug reporting! I hope I have fixed it, please pull the master branch to check the fix.

padjon commented 7 years ago

Hi @pmed, thank you so much for the fast bugfix and this project in general. It works now with .set

I noticed, it still doesn't work if you use .inherit instead of set.

pmed commented 7 years ago

Hi @padjon

Could you please elaborate this `v8pp::class_::inherit()' issue? Because I can't reproduce it.

padjon commented 7 years ago

Hi @pmed, sure change your test code of test_class.cpp to this:

    v8pp::class_<B, use_shared_ptr> B_class(isolate);
    B_class.set("xB", &B::x)
        .set("zB", &B::z)
        .set("g", &B::g);

    v8pp::class_<C, use_shared_ptr> C_class(isolate);
    C_class
        .inherit<B>()
        .template ctor<>()
            .set("xA", &A::x)
        .set("xC", &C::x)

        .set("zA", &A::z)
        .set("zC", &C::z)

        .set("f", &A::f)    
        .set("h", &C::h)

        .set("rF", v8pp::property(&C::f))
        .set("rG", v8pp::property(&C::g))
        .set("rH", v8pp::property(&C::h))

        .set("F", v8pp::property(&C::f, &C::set_f))
        .set("G", v8pp::property(&C::g, &C::set_g))
        .set("H", v8pp::property(&C::h, &C::set_h))
        ;

Will work, with struct C : B, A but not with struct C : A, B

pmed commented 7 years ago

Hi @padjon

Thank you so much for the report and test code, it helped a lot!

I fixed another bug with casting object pointers from derived to base classes. It seems that multiple inheritance is quite rarely used.

Please pull the master branch to check this fix.