Page MenuHomePhabricator

csharp: make inherited C# classes constructible from native C

Authored by vitor.sousa on Jun 4 2019, 3:42 PM.



With this commit it is now possible for a class that inherits from a C# binding
class to be instantiated from native C code. It only has to provide a
constructor that receives an Efl.Eo.EoWrapper.ConstructingHandle struct,
and which calls the base binding constructor passing it.
For example:

private Type(ConstructingHandle ch) : base(ch) {}.

Add some test files to validate the proper behavior of this feature.

Add some small fixes in generation contexts in order to properly
generate base constructors.

Depends on D9070

Test Plan

meson test and make check

Diff Detail

rEFL core/efl
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
vitor.sousa created this revision.Jun 4 2019, 3:42 PM
vitor.sousa requested review of this revision.Jun 4 2019, 3:42 PM
YOhoho added a subscriber: YOhoho.Jun 11 2019, 2:25 AM
YOhoho added inline comments.

It may be EflConstructor'D'elegate.


What do you think to add fallback of BaseType constructor? e.g.) (if constructor == null, constructor = managedType.BaseType.GetConstructor) if there is custom widget(inherited widget) without MyWidget(ConstructingHandle ch) constructor, It will occur unhandled exception because constructor is null.

woohyun added inline comments.Jun 11 2019, 6:31 PM

: base() is needed ? to pass the test case ?

YOhoho requested changes to this revision.Jun 11 2019, 11:43 PM

There is infinite loop when i create a class inherited Efl.Ui.Widget.
Please test the sample code with this patch{F3724530}.

When i get instance from native C code, its inherited value is false.

This revision now requires changes to proceed.Jun 11 2019, 11:43 PM

One more question,

When i tested this sample

without D9070, D9071, it is possible for a class that inherits from a C# binding class to be instantiated from native C code.
Which point should i test?

fixed inherited value

vitor.sousa added a comment.EditedJun 27 2019, 12:08 PM

Hi @YOhoho, thank you for the review.
Sorry about the delay.

The implementation was really missing the assignment of the inherited field, which lead to the infinite loop when trying to resolve overridden methods.

I updated the patch with the needed fixes you pointed out.

Now about some of your questions and suggestions.

About trying to default to a base class constructor when no constructor is found in the derived class; from what I investigated, when instantiating a class all constructors in the class hierarchy needs to be called in the order of the most derived to the most "top level" base, each passing control to next one in a way that they start and end in the reverse order of the calls (i.e. from the top level base to the most derived class).
So, we are obliged to call a constructor in the derived class.
On the other hand, this constructor just needs to have a specific signature that we can search and call, and we can settle in any signature that we like the most.
It can even be the default constructor! But in such case we would need to use something like TLS to pass the handle value to the object, which can be very detrimental to performance.

My reasoning in using the ConstructingHandle struct parameter was to make this feature the "most explicit" possible and avoid any side effects, since the user will have to actively implement a constructor with a very specific type.
But we can discuss on that.

Then on your second question about being possible to instantiate a class in native code without this patch.
I can say that this is the kind of side effect I was trying to avoid with the specific type constructor.
It is happening because the "native part" of the object is being constructed in C, and later, right at the function return, the objected is being "wrapped" by a new C# object.
This is problematic because of a series of reasons.
For example, the instantiation of the native and managed parts doesn't happen at the same moment. If for some reason the object is never returned to C#, we would still expect it to work properly, calling managed methods from C when they are needed, but without this patch that will never happen if the object is never manually returned to C# before being used.

The IntPtr constructor in your example is working as the "wrapping constructor" we use for generated classes.
We don't expect this constructor to work really well with inherited classes, maybe we should change it to use a specific IntPtr wrapper too, so users are less prone to use it by accident.
I am going to work on something in this regard.

vitor.sousa updated this revision to Diff 23026.EditedJun 27 2019, 1:59 PM

Add missing override keyword to suppress warning.

YOhoho accepted this revision.Jun 28 2019, 2:08 AM

It makes sense to me. thank you for detailed reply.

This revision is now accepted and ready to land.Jun 28 2019, 2:08 AM
This revision was automatically updated to reflect the committed changes.