Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

classbinding for classes with multiple inheritance #62

Closed
padjon opened this issue Aug 26, 2017 · 5 comments
Closed

classbinding for classes with multiple inheritance #62

padjon opened this issue Aug 26, 2017 · 5 comments

Comments

@padjon
Copy link

padjon commented Aug 26, 2017

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 added a commit that referenced this issue Aug 27, 2017
Cast pointers to class member (variable, function, property getter/setter)
to the real class type in addition to 4974a8b

Added constructors and copy-constructors for `property_` template in order
to use them in `class_::set()` for casting to the real class type.

Added tests for binding classes with multiple inheritance.
pmed added a commit that referenced this issue Aug 27, 2017
Cast pointers to class member (variable, function, property getter/setter)
to the real class type in addition to 4974a8b

Added constructors and copy-constructors for `property_` template in order
to use them in `class_::set()` for casting to the real class type.

Added tests for binding classes with multiple inheritance.
@pmed
Copy link
Owner

pmed commented Aug 27, 2017

Hi @padjon,

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

@padjon
Copy link
Author

padjon commented Aug 27, 2017

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
Copy link
Owner

pmed commented Sep 6, 2017

Hi @padjon

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

@padjon
Copy link
Author

padjon commented Sep 6, 2017

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 added a commit that referenced this issue Sep 7, 2017
Supply actual registry type to `object_registry::find_object()` on unwrapping
in order to cast the result object.
@pmed
Copy link
Owner

pmed commented Sep 7, 2017

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.

@pmed pmed closed this as completed Feb 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants