Page MenuHomePhabricator

Evaluate work needed to fix EO classes inheriting from more than one regular class or abstract
Closed, ResolvedPublic

Description

Parent task contains a comment with the list of offending classes: https://phab.enlightenment.org/T7240#124206
There's 8 of them (while waiting for T7365 to produce a possibly more exact list).

The output of this task should be a comment with the list of things that need to be done to make all EO classes inherit from only one regular class (or abstract). Ideally, with time estimates.

segfaultxavi triaged this task as Normal priority.

Multiple class inheritance from abstract and class is removed by D6982 and D6984.

Hello @Jaehyun_Cho : We discussed in Korea that abstract classes should _NOT_ inherit from regular classes for our changes in C# to work correctly, is that still possible?

Regards,

Dear @felipealmeida

As you know, while we discussed in Korea together, we haven't discussed how to resolve abstract inheriting from regular class because we did not know that this case exists.

As I commented in T7240, I am not sure if it is right way to forbid inheritance from regular class to abstract class because it is totally fine in OOP perspective.

Moreover, if we convert abstract to interface in C# bindings, then we cannot call abstract class' method implementation.

As far as I can see, the problem here is that some mixin inherits from abstract class.

How about we convert mixin into another form?

@woohyun and I are considering a way to convert mixin into another form. i.e. removing inheritance of mixin and converting mixin to regular class and using it as composite object.

I will share details as soon as possible.

Regards,

Hello @Jaehyun_Cho,

So, one problem is that if a mixin is allowed to inherit from regular classes, then it can't be generated as an interface.

Maybe, we could generate a mixin in C# as being an interface if it only inherits from abstract classes and/or interfaces (directly or indirectly), and generate it as a abstract class otherwise. However, we must make sure that classes that inherit from mixins which inherit form regular classes do _not_ inherit from another regular class too, or we would not obey the non-multiple-inheritance principle, right?

Would this make sense?

@felipealmeida

You idea (converting "mixin" in eo to "interface" in C# binding) is a solution.
But as you said, it requires we need to remove inheritance from abstract class, regular class to mixin in eo. And I think this is quite difficult.

@felipealmeida @segfaultxavi @lauromoura @q66

Here, @woohyun and I propose that using object composition instead of mixin inheritance.

Mixin is converted to class in C# bindings and child class contains mixin instance and calls the mixin instance methods.
The following is only available in C# bindings.

  • Remove inheritance from mixin to its child regular class and abstract class.
  • Add all methods of mixin to its child regular class and abstract class.
  • Convert mixin to class.
  • The child regular class and abstract class constructs an instance of the mixin (it is converted to class in C# bindings).
  • When mixin's methods (these methods are added to child class in C# bindings) are called, the mixin (it is converted to clas in C# bindings) method is called with mixin instance.

The example is as follows.

//eo
mixin M (...)
{
   methods {
      foo {
      }
   }
}

class C (M, ...)
{
}
//C# bindings
class M : ...
{
   foo( )
   {
   }
}

class C : ...
{
   C( )
   {
      m = new M( );
   }
   foo( )
   {
      m.foo( );
   }
}

What do you think about this?
The only problem here is override function case. (child class overrides parent mixin's method.) If so, child class method is preserved and called. (we can see the method is re-implemented or not in eo)

Please share your opinion.

Couldn't we just not generate mixin classes at all and just include the methods in the class definition?

This comment was removed by Jaehyun_Cho.

@felipealmeida

Yes, that is one of options.

If we do your suggestion, then we need to resolve how to call mixin's function in c source code without mixin class.

I think what @felipealmeida is suggesting is to remove the mixin classes completely from the C# bindings. Nothing would change in C land.

Unless you are talking about a mixin created entirely in C#, which needs to be called from C. If this case is a problem, then I think I missed something important. My understanding is that the C# bindings are meant to create C# applications, or C# widgets to be used by C# applications.

Do you think that C# should be used to create EFL infrastructure (like a mixin) which could then be usable by any other EFL app regardless of its language?

@segfaultxavi

Yes I understood same way about @felipealmeida comment.

What I do not know is how to call C function foo() of mixin M when C# binding function foo() of class C is called if class C inherits from mixin M.

Can we just call C function foo() of mixin M with eo object of class C instance in C# bindings?

e.g.
class C : ...
{

foo()
{
   c_function_foo_of_mixin_M(eo_obj_of_class_C);
}

}

I do not understand why do you want to call C functions from C#. That is the bindings job and Felipe will take care of it, right? Your C# app or your C# widgets should only use C# methods. What am I missing?

@segfaultxavi

I am discussing how to resolve multi class inheritance with mixin in C# bindings.
Because now C# bindings work is blocked by how to treat mixin in C# bindings..
So I am suggesting what form would be good for mixin in C# bindings.

Sure, sure, I understand that.

Can you please tell me why you want to call C methods from C#?

@Jaehyun Wouldn't it be the same as calling the parent method? Remember that the parent class still has implementations for all methods you would override.

Just call with base in C# and that will go to the parent class which will call the mixin's implementation.

Anything else I'm missing?

Well, maybe I'm missing something. How a C# user would inherit from a mixin directly. If the user inherits from a class that inherits from a mixin it works OK, because the class has the method. But then it couldn't inherit from a mixin and get called from C or call in C.

Sorry, but I still don't understand why a C# user would want to call to/from C, or inherit from a mixin.

The way I see it (please correct me if I am wrong), from the C# user's perspective, EFL is a library that you can use to build graphical apps.
The C# user does not know that EFL is internally coded in C. The C# user should only need to interact with C#.
The C# user does not know that EFL internally uses EO, and have things called classes, interfaces, mixins... The C# user only sees some C# classes (and maybe interfaces), which can be inherited from to modify their behavior. I do not think we need to offer to the C# user all the options that EO has, because it is complicated and it will feel unnatural to C# anyway.

It is the job of the C# bindings to convert between the above C# user's perspective and the internal EO structure.

Do we agree on the above, @Jaehyun_Cho @felipealmeida ?

@segfaultxavi

Let me clear with an actual example. Otherwise, I think we will miscommunicate.

Let's assume the EFL C# bindings generator translates eo class to C# class and eo abstract class to C# abstract class.
Since there is no mixin in C# language, eo mixin should be translated among class or interface or nothing (remove the mixin in C#) by EFL C# bindings generator.

The following mixin Efl.Access.Object is a problem case.
This mixin inherits from abstract class Efl.Object and abstract class Efl.Ui.Widget inherits from class Efl.Canvas.Group and mixin Efl.Access.Object.
Based on our assumption, eo abstract class is translated to C# abstract class so this mixin may be translated to class.
As a result, this is multi class inheritance in C# and not allowed.

The above discussion is to avoid this multi class inheritance.
And one of our options is that EFL C# bindings generator removes mixin Efl.Access.Object in C# and adds @property state_set { get; } to abstract class Efl.Ui.Widget in C#.

//efl_access_object.eo
mixin Efl.Access.Object (Efl.Interface, Efl.Object)
{
   ...
   @property state_set {
      get {
      }
   }
}

//efl_ui_widget.eo
abstract Efl.Ui.Widget (Efl.Canvas.Group, Efl.Access.Object, ...)
{
   ...
   implements {
      Efl.Access.Object.state_set { get; }
   }
}

The reason why I am talking about calling C function in C# is because C# class function calls the actual C function by EFL C# bindings.
So I am asking the form of GetStateSet() generated by EFL C# bindings generator if calling efl_access_object_state_set_get() is ok.
(I am not talking about user's perspective.)

//efl_ui_widget.eo.cs generated by EFL C# bindings generator
sealed public class Widget : IWidget
{
   ...
   public efl.access.State_Set GetStateSet() {
      var _ret_var = efl_access_object_state_set_get(this.raw_handle); //actual C function is called inside C# class function
      return _ret_var;
   }
}

@felipealmeida @lauromoura

I think we don't need to consider the case that user inherits from mixin.
Because we are considering that EFL C# bindings generator removes mixin. (user cannot inherit from mixin because it does not exist.)
e.g. mixin Efl.Access.Object is removed in C# but its @property state_set { get; } is added to abstract class Efl.Ui.Widget in C#.

I want to ask if there is problem in this case.
If we remove mixin Efl.Access.Object in C#, then calling efl_access_object_state_set_get() in GetStateSet() of abstract class Efl.Ui.Widget is ok?
Regarding calling C function triggers calling its C# function, there is no C# function for efl_access_object_state_set_get().

@Jaehyun_Cho OK, now I understand. I thought you were talking about the user's perspective but you have been talking about the C# bindings implementation all the time. Thanks for clearing up the misunderstanding.

I'm not sure removing Mixin from inheritance tree helps in anyway.

Let's say we create class A (everything in Eolian) and it inherits from Mixin M and regular class B.

Now, Mixin M inherits from regular class C too. If we remove Mixin, we still need to inherit
form both B and C in A, which is still a multi-inheritance problem.

Regards,

About hiding them from the tree, these are the methods either returning (->) or receiving as a parameter (between ()) a Mixin type.

  • Efl.Ui.Focus.Manager::move -> Efl.Ui.Focus.Object
  • Efl.Ui.Focus.Manager::request_move -> Efl.Ui.Focus.Object
  • Efl.Ui.Focus.Manager::request_move (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager::request_subchild -> Efl.Ui.Focus.Object
  • Efl.Ui.Focus.Manager::request_subchild (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager::fetch (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager::setup_on_first_touch (entry:Efl.Ui.Focus.Object)
  • Efl.Access.Object::event_emit (accessible:Efl.Access.Object)
  • Efl.Access.Object::relationship_append (relation_object:Efl.Access.Object)
  • Efl.Access.Object::relationship_remove (relation_object:Efl.Access.Object)
  • Efl.Ui.Focus.Parent_Provider::find_logical_parent -> Efl.Ui.Focus.Object
  • Efl.Ui.Focus.Parent_Provider::find_logical_parent (widget:Efl.Ui.Focus.Object)
  • Efl.Ui.Widget_Focus_Manager::focus_manager_create (root:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Util::focus (focus_elem:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::register (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::register (parent:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::register_logical (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::register_logical (parent:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::update_redirect (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::update_parent (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::update_parent (parent:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::update_children (parent:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::update_order (parent:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::unregister (child:Efl.Ui.Focus.Object)

Even if we somehow hide the mixin methods in the class, we'd need an interface representing the Mixin in order to make these methods correctly callable.

I just put together a small lua script that produces a diagram of the inheritance tree of any class: P236

The output looks like this: https://pasteboard.co/HFY3e8X.png

I hope it helps a bit. It already shows that Efl.Object is being inherited 3 times.

I'm not sure removing Mixin from inheritance tree helps in anyway.

Let's say we create class A (everything in Eolian) and it inherits from Mixin M and regular class B.

Now, Mixin M inherits from regular class C too. If we remove Mixin, we still need to inherit
form both B and C in A, which is still a multi-inheritance problem.

Regards,

Indeed, the diamond problem is already in our hierarchy, no matter how we implement it in C# (look at the picture linked above).

I think mixins have been abused inadvertently: There are 40 EFL mixins (I am not counting legacy), of which, at least:

  • 6 inherit from Efl.Object
  • 3 inherit from Efl.Ui.Widget (OMG! look at the diagram for efl_ui_view_list.eo)

All these have to be fixed. Remember that this task is about "Evaluate work needed to fix EO classes inheriting from more than one regular class or abstract", so the parent task can continue ("Not allow multi class inheritance in eo").

About hiding them from the tree, these are the methods either returning (->) or receiving as a parameter (between ()) a Mixin type.

  • Efl.Ui.Focus.Manager::move -> Efl.Ui.Focus.Object
  • Efl.Ui.Focus.Manager::request_move -> Efl.Ui.Focus.Object
  • Efl.Ui.Focus.Manager::request_move (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager::request_subchild -> Efl.Ui.Focus.Object
  • Efl.Ui.Focus.Manager::request_subchild (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager::fetch (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager::setup_on_first_touch (entry:Efl.Ui.Focus.Object)
  • Efl.Access.Object::event_emit (accessible:Efl.Access.Object)
  • Efl.Access.Object::relationship_append (relation_object:Efl.Access.Object)
  • Efl.Access.Object::relationship_remove (relation_object:Efl.Access.Object)
  • Efl.Ui.Focus.Parent_Provider::find_logical_parent -> Efl.Ui.Focus.Object
  • Efl.Ui.Focus.Parent_Provider::find_logical_parent (widget:Efl.Ui.Focus.Object)
  • Efl.Ui.Widget_Focus_Manager::focus_manager_create (root:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Util::focus (focus_elem:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::register (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::register (parent:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::register_logical (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::register_logical (parent:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::update_redirect (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::update_parent (child:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::update_parent (parent:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::update_children (parent:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::update_order (parent:Efl.Ui.Focus.Object)
  • Efl.Ui.Focus.Manager_Calc::unregister (child:Efl.Ui.Focus.Object)

    Even if we somehow hide the mixin methods in the class, we'd need an interface representing the Mixin in order to make these methods correctly callable.

@bu5hm4n

Hello @bu5hm4n :)

As far as I know, you and @woohyun discussed about how to deal with classes and mixins related to Focus.

Could you share how to resolve classes and mixins related to Focus?

I actaully don't know right now what this is abount resolving what exactly ?

bu5hm4n added a comment.EditedOct 3 2018, 12:19 AM

I'm not sure removing Mixin from inheritance tree helps in anyway.

Let's say we create class A (everything in Eolian) and it inherits from Mixin M and regular class B.

Now, Mixin M inherits from regular class C too. If we remove Mixin, we still need to inherit
form both B and C in A, which is still a multi-inheritance problem.

Regards,

Indeed, the diamond problem is already in our hierarchy, no matter how we implement it in C# (look at the picture linked above).

I think mixins have been abused inadvertently: There are 40 EFL mixins (I am not counting legacy), of which, at least:

  • 6 inherit from Efl.Object
  • 3 inherit from Efl.Ui.Widget (OMG! look at the diagram for efl_ui_view_list.eo)

    All these have to be fixed. Remember that this task is about "Evaluate work needed to fix EO classes inheriting from more than one regular class or abstract", so the parent task can continue ("Not allow multi class inheritance in eo").

You need to differ between two reasons to inherit from a regular class in a mixin:

  • you have the reason for extending the provided API due to inheriting from this mixin (which should be done with an interface which is then implemented), if you do this with a regular class, then things will go REALLY bad, since the private data etc. is not allocated in the mixins side.
  • You want to inherit from a class, in order to work with its functions in the implements {} section. The inheritee of the mixin however should also inherit from this particular class. To ensure constructors are called, private data exists etc.

(Any other case should be banned from eolian IMO)

Applying this means: You can ignore Efl.Object here, its in the inheritance of every object anyway (reason #2) . Same case for Efl.Ui.Widget, in all 3 cases the mixin will only work if the inheriting class will have Efl.Ui.Widget in its inherit classes.

So in my opinion it would be sane - to just drop if a regular class is in the inherit list of a mixin but not in the inherit list of the normal class (at the end, when the inheritance reached a level where the *thing* can be instanciated.
This resolves (as far as I can see) the problems with the focus inheritance - right ? @Jaehyun_Cho @woohyun ?

EDIT: Regular classes in mixins inheritance hierarchy should maybe be treated more as a "this class should be inherited from higher in the hierarchy" - let this be via normal inheritance or composition.

segfaultxavi added a comment.EditedOct 3 2018, 4:26 AM

I have been talking with @bu5hm4n for a long time this morning and I learnt a lot about how is EO supposed to work.

I wanted to share this knowledge to make sure we all understand the same thing. Please review the following list and tell me if there is anything you think is incorrect.

I am calling Primary Ancestor the first item in the inheritance list and Secondary Ancestors the rest of items in the inheritance list.

  1. Regular classes
    1. Regular classes inherit from their primary ancestor.
    2. Can implement methods from any interface listed in the inheritance list. These methods appear in the implements section.
    3. Can include mixins. The mixins methods are exported by the class. The mixin implementation and internal data becomes part of the class (but private).
    4. If a regular class appears as a secondary ancestor then only its API is used, meaning that it is treated as an interface. Maybe Eolian could generate a warning about this.
  2. Mixins
    1. Mixins inherit from their primary ancestor, which must be another mixin or an interface.
    2. Can implement methods from any interface listed in the inheritance list and these methods appear in the implements section (just like regular classes).
    3. If a regular class (let's call it C1) appears as a secondary ancestor then it gets interesting. The mixin can implement any of the methods of C1 (listing them in the implements section) but the class private data will not be included in the mixin. This means that any other class C2 including this mixin MUST inherit from C1 to provide the private data. All our current classes do this, but it is not enforced by eolian, so it is prone to errors.
    4. Call resolution: When a method is defined in a class and then overridden multiple times in other classes (for example: C1.method(), then mixin M inherits from C1 and overrides method(), and then class C2 inherits from C1 and overrides method() and also includes M). Always one of the implementations of method() will be called. If this implementation calls efl_super() then the call is chained up and ALL implementations might end up being called.

Before proceeding with the C# bindings work, is this how you understand EO inheritance to work?

lauromoura added a comment.EditedOct 3 2018, 9:42 PM

I've rebased the current new API work on top of yesterday (Oct 3rd) master (still in devs/lauromoura/csharp-new-classes) and it compiled without requiring any changes to the Eo files. All tests pass and a quick test showed at least elm examples are working. The generator new code requires some cleanup before pushing, though.

The Mixin's are being generated as interfaces and any regular class that is a parent of a Mixin do not appear in the mixin inheritance tree due to C# limitation of regulars not allowed as interface bases. As Xavi said above, this seems to work incidentally as these regulars end up being in the tree by hand (maybe even through other inheritance paths).

PS: I've pushed in the branch some commits already in differential for upstream as they fix make examples.
PS2: There are a bunch of new/override warnings that need to be fixed too.

@segfaultxavi @bu5hm4n
Thank you for your explanation. It helps my understanding about Eolian clearly.

@segfaultxavi

  1. Mixins
    1. Mixins inherit from their primary ancestor, which must be another mixin or an interface.

Do you mean abstract class as an interface? (i.e. mixin can inherit from abstract class as primary ancestor)

@segfaultxavi @bu5hm4n
Thank you for your explanation. It helps my understanding about Eolian clearly.

@segfaultxavi

  1. Mixins
    1. Mixins inherit from their primary ancestor, which must be another mixin or an interface.

Do you mean abstract class as an interface? (i.e. mixin can inherit from abstract class as primary ancestor)

mixins cannot inherit from abstracts as primary ancestor. The only types that are possible as primary ancestor are Interfaces or Mixins.

@segfaultxavi @bu5hm4n
Thank you for your explanation. It helps my understanding about Eolian clearly.

@segfaultxavi

  1. Mixins
    1. Mixins inherit from their primary ancestor, which must be another mixin or an interface.

Do you mean abstract class as an interface? (i.e. mixin can inherit from abstract class as primary ancestor)

mixins cannot inherit from abstracts as primary ancestor. The only types that are possible as primary ancestor are Interfaces or Mixins.

@bu5hm4n
Thank you for answering :)

@lauromoura @felipealmeida @bu5hm4n

In the latest devs/lauromoura/csharp-new-classes branch, all the duplicated re-implemented methods are removed. (parent's methods are used instead)
e.g. Efl.Ui.Widget.FocusStateApply() is not re-implemented in Efl.Ui.Layout.Object because efl_ui_layout_object.eo does not implement focus_state_apply.

However, it reveals mixin's hierarchy problem because eo mixin is converted to EFL# interface so eo mixin's abstract class parent is removed in EFL# interface's parent list.
e.g. mixin Efl.Ui.Focus.Layer does not inherit from Efl.Ui.Widget in EFL#. So Efl.Ui.Focus.Layer's re-implemented FocusStateApply() cannot be called although efl_ui_focus_layer.eo implements focus_state_apply.

I think we need to think how to resolve this mixin case here again for language bindings.

bu5hm4n added a comment.EditedOct 22 2018, 1:15 AM

I am not understanding something. why does FocusStateApply() need to be called ?

Let's take the popup case. The popup class should when elm_focus_state_apply is called with efl_super select the mixin as next level - and call the function there. What exactly is not working there ? The function then is not FocusStateApply. However it is efl_ui_widget_focus_state_apply.

@bu5hm4n

Let's think about C# application developer wants to create a class inheriting Efl.Ui.Focus.Layer to apply Efl.Ui.Focus.Layer's focus_state_apply.

However, there is no eo file for C# application developer's class so Efl.Ui.Focus.Layer's focus_state_apply cannot be called.

In short, mixin Efl.Ui.Focus.Layer's functionality cannot be used by C# application developers.

Well, but the focus_state_apply function is defined in efl_ui_widget, so in the end a c# user would call something like base.FocusStateApply(...) right ?
and this call _should_ (at least IMO) then result in a call to eo which figures out the next instance to call the method on ...

@bu5hm4n
I am not sure if user-defined C# class works for Efl.Ui.Focus.Layer's focus_state_apply. (at least it is not crystal clear that it works based on C# class hierarchy..)

But although if the above works, C# application developer has no clue why they should inherit from such mixin because there is no re-implemented method in the mixin in C#.

Those mixins would become black box and it would be difficult to understand and explain why those mixins are used and how those mixins work for C# application developers.

segfaultxavi added a comment.EditedOct 22 2018, 3:17 AM

Hi @Jaehyun_Cho,

Again, I am confused. I thought that C# applications should only instantiate efl# classes and use them. I didn't think that C# applications should be allowed to inherit from efl# classes and extend them.
In other words, I thought that if you needed a new widget, you had to create it in C, using EO and C files, and then you could use it from C#. I thought you should not be allowed to create new widgets from C#.
This would make the C# bindings much easier, because all efl# classes could be sealed and we should not worry about inheriting from them.

So, do you think we should allow inheriting from efl# classes? Why? This question is important and the source of a lot of misunderstanding.

@segfaultxavi
Of course we need to support inheritance from efl# classes to user-defined class without defining EO and C file.

The purpose of C# bindings is to provide better usability to users.

If we want users to make EO and C file for user-defined C# class, then nobody would do that...

Without providing customization method (e.g. user-defined class with inheritance from efl# classes), no commercialized application would be developed by EFL C#...

OK, OK, I just wanted to make that clear.

It is a very different thing to provide bindings to use a library or to extend a library. Provide bindings that allow extending EFL is much more difficult, as we are seeing.

Are we sure that we need to extend EFL from C#? I mean, EFL is very flexible and customizable: you have themes, widget parts, etc. Can you give some examples of the type of new widgets users would like to create from C# which cannot be already created by customizing an existing widget?

@segfaultxavi
Yes, it is essential to extend EFL from C#. This is not only for C# application developers but also for C# framework developers like us.

C# framework developers would prefer to inherit from efl# classes and add/override methods instead of adding EO and C file.

Here is an example for C# framework developers.
Let's think about a widget which is very similar to Efl.Ui.Button but it should load a specific EDJ group instead of "efl/button".
It would be much easier to inherit from Efl.Ui.Button and simply override a method to load the specific theme.

OK, after talking with @felipealmeida and @lauromoura today I understand that the C# bindings have been designed to allow extending EFL through C#.

However, the example you give puzzles me. The edj group is selected with the Style API, right? You can call SetStyle("anchor") and it will select the "anchor" group.
Can you give me a different example that requires subclassing?

YOhoho added a subscriber: YOhoho.Oct 22 2018, 3:35 PM

@Jaehyun_Cho can you provide an example that requires subclassing which is NOT about changing the widget style? It would help us understand your requirements.

@Jaehyun_Cho @woohyun while there is still this inheritance conundrum, this problem seems to happen in the code on master too right?

I'm thinking about pushing these modifications (camel.case namespaces, removed '_', more code reuse in regular/abstract classes) so we can deal with the mixin in separate commit (And Xavi can start updating the examples/tutorials - sorry, Xavi :) )

Hello @segfaultxavi . It is not restricted to Widgets. People may want to inherit Thread objects, or some other kind of Objects. I don't think we can run away from inheritance in a OO-language/world.

@lauromoura

I agree with you! Please push modifications which are not related to dealing with mixin :)

@segfaultxavi

For now, I cannot think of any case not related to Widget.. (I think it would be rare that app developers modify non Widget class by using inheritance)

I am thinking about adding new Widget class, which provides new methods and properties, by using only EFL C# without adding EO.
e.g. class Efl.Ui.Popup_Notify may be added by inheriting class Efl.Ui.Popup. The class Efl.Ui.Popup_Notify has additional methods to class Efl.Ui.Popup. And the class Efl.Ui.Popup_Notify can be added by adding CS file without adding EO.

My point is that Tizen Framework developers may also want to add new class only in C# by inheriting from efl# class without adding EO to provide the new class to C# App developers.

OK, thank you all for helping me understand a bit better this problem. I apologize for dragging this discussion for so long.

I'll wait for the C# bindings team to figure out how to solve the remaining issues, and please let me know if there's anything we can do from the EFL interfaces side of things!

@lauromoura

I agree with you! Please push modifications which are not related to dealing with mixin :)

Just to be clear, I'd like to push the branch as is (after squashing), with the "mixins as interfaces" approach. The problem to split it is that this change was one of the first ones, together with the class inheritance refactor, and all others built on top of that.

@segfaultxavi

I also agree with @lauromoura that we don't need to wait for the works of @lauromoura.
Eo class inheritance refactoring should be done independently with his C# binding works.

So, let's get back to the topic how to refactor Eo class hierarchy which can support well OOP language bindings.
In my opinion, Focus, Access, and other MIXINs can be checked first.

q66 moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Nov 14 2018, 5:21 AM
bu5hm4n closed this task as Resolved.Jan 11 2019, 3:48 AM
bu5hm4n claimed this task.

@bu5hm4n
Regarding resolving this task with 3ddd577fb0c67b1867c6664af42a89652134c73a, I want to discuss about it.
I understand that the patch forbid a regular class to inherit from a new regular class from mixin.

However, if mixin inherits from a regular class such as Efl.Object and it overrides constructor/destructor, then the mixin is assumed as a regular class itself in some languages. (e.g. C#)
Consequently, this mixin is converted to a regular class if the mixin's constructor and destructor remain in the language binding code. (for the purpose of overriding)
As a result, multi class inheritance exists in those language binding code.

How about not allowing regular class inheritance in mixin?
Then, mixin implements interface only. But for the original purpose, mixin contains its own implementation code inside unlike interface.

Good morning, I am not entirely sure what you mean. What the patch does: It only allows mixins to inherit from other mixins or interfaces. However, it allows the mixins to *require* a regular / abstract class. The functions that are in required classes can also be overridden, which is (based on the fact how eo is build) possible without the eo class of the mixin directly inheriting from the class. When then later the mixin is used, the function is overridden anyways.
In other words, the regular classes that would have been in the inheritance list of a mixin are now moved to the first regular/abstract user of the mixin. The class needs to inherit from them in the first place, which means, the two subtrees of the class hirachy are unified at this place.

I cannot really comment on why the C# bindings decided to do that like this. However, mixins cannot be used in C# as far as I understood it, thus they are converted to interfaces.
When C# inherits from such a interface the class_get function of the mixin-interface (which gets passed to efl_class_new, to mirror the new class into eo), could just return the mixin_get function, which then will make the mixins work like if you use C.

So summed up:

  • Patch does restrict mixins to only inherit from mixins and inheritance
  • This means a mixin can just be converted to an interface, from the bindings POV. The fact that there is actaully a implementation for certain functions does not need to be handled by the bindings, since eo will call the functions of the mixins.

To your last paragraph: as i said above, in fact, mixins cannot inherit from regulars / abstracts.

C# binding is not suppose to be doing that. It, AFAIR, just ignores regular classes in mixins.

Right, the new syntax with *required* is enforcing this. Additinally, the regular classes are not passed to eo anymore, to enforce this on all levels.

However, I am now a bit confused, what exactly is then the problem ?

@bu5hm4n
I mean that your patch is good. There is no problem on that.
But I was not sure if it was sufficient to close this task.
(But now I think that this task is to evaluate the main task so it is ok this task is closed)

My point was as follows.

  1. mixin becomes interface in C#. Otherwise, if mixin becomes class in C#, then multi class inheritance violation happens based on current .eo files. (e.g. class inherits from both class and mixin)
  2. If mixin becomes interface in C#, then regular class inherited by mixin is removed.
  3. Because of 2, in C# we lose override functions in C#. (e.g. Efl.Object.Constructor)

To keep override functions of regular class inherited by mixin in C#, mixin should become a regular class. So this is why I thought that this task was not fully closed.
But like I said above, now I think it is ok this task is closed. Maybe we can discuss how to handle mixin case later (after resolving other important tasks first~ :) )

Ah okay - I understand now clearly what you mean :), i will answer to it in the parent task of this task later today. Thank you! :)

Shouldn't the user that will be inheriting from the mixin also inherit from the same regular class? It is not tested, but we could add a runtime test. This needs documentation, however.

Shouldn't the user that will be inheriting from the mixin also inherit from the same regular class? It is not tested, but we could add a runtime test. This needs documentation, however.

Yes. That is already enforced in eolian with the requires syntax.

In eolian, but it is not in Eo, so the C# class will not be enforced by this.