Page MenuHomePhabricator

csharp: cleanup concrete class
ClosedPublic

Authored by YOhoho on Sep 10 2019, 4:48 AM.

Details

Summary

Concrete class is only used to call static member of NativeMethod. they don't
need any inheritance and implementation of c functions.

Depends on D9893

Test Plan

ninja test

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
YOhoho created this revision.Sep 10 2019, 4:48 AM
YOhoho requested review of this revision.Sep 10 2019, 4:48 AM
lauromoura requested changes to this revision.Sep 10 2019, 7:38 PM
lauromoura added a subscriber: segfaultxavi.

I'm not sure we can remove all these functions from the Concrete classes yet.

For example, an hypothetical scenario:

  • A given method returns an interface instance, like, let's say, Efl.IModel Efl.Ui.View.GetModel()
  • The C# binding will happily try to use introspection to get the actual Efl_Class* of this instance through Globals.CreateWrapperFor
    • This makes sure we instantiate the actual generated C# class for that given Eo instance if possible.
  • But what if this is a non-generated class completely hidden from the binding? Like some kind of "private native model class"?

In this case we would rely on the Concrete class to be able to provide the C# user an instance that implements this interface. It would just wrap the Eo pointer like a regular generated class.

I'm afraid we did not have such test case in our test (thus this patch worked) (or better comments explaining the reasoning for keeping these classes around).

PS: Except for the rebase, looks good and tests passes.

src/bin/eolian_mono/eolian/mono/klass.hh
196–197

There is a small conflict in this line after D9725 was merged. (It changed from sealed public to public sealed).

This revision now requires changes to proceed.Sep 10 2019, 7:38 PM
YOhoho updated this revision to Diff 28150.Tue, Jan 14, 12:41 AM

Add unit test.

I'm not sure we can remove all these functions from the Concrete classes yet.

For example, an hypothetical scenario:

  • A given method returns an interface instance, like, let's say, Efl.IModel Efl.Ui.View.GetModel()
  • The C# binding will happily try to use introspection to get the actual Efl_Class* of this instance through Globals.CreateWrapperFor
    • This makes sure we instantiate the actual generated C# class for that given Eo instance if possible.
  • But what if this is a non-generated class completely hidden from the binding? Like some kind of "private native model class"?

I would like to make sure that API user can't make non-generated class that only have generated interfaces without EoWrapper like that,

public class MyModel : Efl.IModel
{
}

Because API user can't implement NativeHandle and NativeClass in Efl.Eo.IWrapper that all generated interfaces inherit from.

There are 2 instantiation cases of non-generated class inherits from EoWrapper(all non-generated EFL class have to inherits from EoWrapper by what i said above).

  1. from C# (unit test test_iface_value_property())
    • WrapperSupervisor is added in protected EoWrapper(IntPtr baseKlass, Efl.Object parent, string file = null, int line = 0).
  2. from C (unit test test_iface_value_from_c())
    • WrapperSupervisor is added in protected EoWrapper(ConstructingHandle ch).

So, when Globals.CreateWrapperFor is called to get the actual Efl_Class * of this instance, Globals.GetWrapperSupervisor(handle) always return proper ws.Target.

@felipealmeida

Can you find any problem on this patch ?
If no , I hope to accept this asap :)

woohyun accepted this revision.Wed, Jan 22, 2:35 PM
This revision was not accepted when it landed; it landed in state Needs Review.Wed, Jan 22, 2:35 PM
Closed by commit rEFL97098dcc50b6: csharp: cleanup concrete class (authored by Yeongjong Lee <yj34.lee@samsung.com>, committed by woohyun). · Explain Why
This revision was automatically updated to reflect the committed changes.