Page MenuHomePhabricator

eo: Fix efl_isa for class checking of recursively inherited types
Needs RevisionPublic

Authored by felipealmeida on Thu, Jan 31, 11:23 PM.

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9312
Build 7923: arc lint + arc unit
felipealmeida created this revision.Thu, Jan 31, 11:23 PM
felipealmeida requested review of this revision.Thu, Jan 31, 11:23 PM
SanghyeonLee requested changes to this revision.Fri, Feb 1, 12:09 AM

looks something wrong in the patch.

src/lib/eo/eo.c
1779

i think _eo_class_id_get is more right way to get Efl_Class

1783

it passed always true.

This revision now requires changes to proceed.Fri, Feb 1, 12:09 AM
bu5hm4n requested changes to this revision.Fri, Feb 1, 12:10 AM
bu5hm4n added a subscriber: bu5hm4n.

Mhmmm MRO should contain all recursive classes.

SanghyeonLee added a comment.EditedFri, Feb 1, 12:19 AM

Mhmmm MRO should contain all recursive classes.

yeah.. but it failed on interface type.. and we thought it because not search every recursive cases... but I don't know why... in the code.. it should work as it is.

YOhoho added a subscriber: YOhoho.EditedFri, Feb 1, 12:38 AM

What do you think to check extensions after checking mro?

SanghyeonLee added a comment.EditedFri, Feb 1, 12:46 AM

The issue case is easily found in example of efl_ui_list_view_example_1.c

I've change some code, so
it is now looking for Efl.Ui.View Interface from the Efl.Ui.List_Default_Item,
which is implemented in Efl.Ui.Layout.

I create new function to tracking the log efl_isa_class, and here it is.

ERR<25820>:eo lib/eo/eo.c:1763 efl_isa_class() LSH :: 'Efl.Ui.View' is the class looking for

ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Ui.List_Default_Item(0x55dbc1dae8e0)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Ui.List_Item(0x55dbc1dab4c0)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Ui.Item(0x55dbc1da8430)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Ui.Layout(0x55dbc1b01370)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.File(0x55dbc19cd430)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Ui.Widget(0x55dbc19afb50)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Access.Object(0x55dbc1a48270)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Access.Component(0x55dbc1a47750)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Ui.Focus.Object(0x55dbc1a4f730)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Ui.Selection(0x55dbc1a4bc10)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Ui.Dnd(0x55dbc19c0d10)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Canvas.Group(0x55dbc19c0960)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Canvas.Object(0x55dbc1a8c300)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Gfx.Color(0x55dbc19a6460)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Gfx.Map(0x55dbc19b1a10)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Loop_Consumer(0x55dbc18335a0)'
ERR<25820>:eo lib/eo/eo.c:1774 efl_isa_class() LSH :: find 'Efl.Object(0x55dbc181de20)'

it looks 'MRO' doesn't really have all recursive implements,
but is is not exactly same as their extends trees.. after efl_ui_layout, it goes efl_file which is implemented by efl_ui_layout not extended.
I don't know what exactly we can get from this mro...

Can you paste you efl_ui_layout.eo.c ? I just checked it and it seems that efl.ui.view is part of efl.ui.layout's MRO here.

However, if MRO does not contain this file for you for some reason then we should probebly fix the mro generation :)

Parts of my output with "EINA_LOG_DOMAINS="eo:10" elementary_test"

DBG<14812>:eo ../src/lib/eo/eo.c:1698 efl_class_new() Finished building class 'Efl.Ui.View'
DBG<14812>:eo ../src/lib/eo/eo.c:1518 efl_class_new() Started building extensions list for class 'Efl.Ui.Layout'
DBG<14812>:eo ../src/lib/eo/eo.c:1550 efl_class_new() Finished building extensions list for class 'Efl.Ui.Layout'
DBG<14812>:eo ../src/lib/eo/eo.c:1555 efl_class_new() Started building MRO list for class 'Efl.Ui.Layout'
DBG<14812>:eo ../src/lib/eo/eo.c:1566 efl_class_new() Finished building MRO list for class 'Efl.Ui.Layout'
DBG<14812>:eo ../src/lib/eo/eo.c:1574 efl_class_new() Started building Mixins list for class 'Efl.Ui.Layout'
DBG<14812>:eo ../src/lib/eo/eo.c:1588 efl_class_new() Finished building Mixins list for class 'Efl.Ui.Layout'
DBG<14812>:eo ../src/lib/eo/eo.c:1625 efl_class_new() Added 'Efl.Container' extension
DBG<14812>:eo ../src/lib/eo/eo.c:1625 efl_class_new() Added 'Efl.File' extension
DBG<14812>:eo ../src/lib/eo/eo.c:1625 efl_class_new() Added 'Efl.Ui.View' extension
DBG<14812>:eo ../src/lib/eo/eo.c:1625 efl_class_new() Added 'Efl.Ui.Model.Connect' extension
DBG<14812>:eo ../src/lib/eo/eo.c:1625 efl_class_new() Added 'Efl.Ui.Factory' extension
DBG<14812>:eo ../src/lib/eo/eo.c:1625 efl_class_new() Added 'Efl.Layout.Calc' extension
DBG<14812>:eo ../src/lib/eo/eo.c:1625 efl_class_new() Added 'Efl.Layout.Signal' extension
DBG<14812>:eo ../src/lib/eo/eo.c:1625 efl_class_new() Added 'Efl.Layout.Group' extension
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.Ui.Layout' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.File' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.Ui.Widget' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.Access.Object' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.Access.Component' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.Ui.Focus.Object' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.Ui.Selection' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.Ui.Dnd' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.Canvas.Group' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.Canvas.Object' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.Gfx.Color' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.Gfx.Map' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.Loop_Consumer' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1638 efl_class_new() Added 'Efl.Object' to MRO
DBG<14812>:eo ../src/lib/eo/eo.c:1659 efl_class_new() Added 'Efl.File' to Data Offset info
DBG<14812>:eo ../src/lib/eo/eo.c:1659 efl_class_new() Added 'Efl.Access.Object' to Data Offset info
DBG<14812>:eo ../src/lib/eo/eo.c:1659 efl_class_new() Added 'Efl.Ui.Focus.Object' to Data Offset info
DBG<14812>:eo ../src/lib/eo/eo.c:1659 efl_class_new() Added 'Efl.Gfx.Map' to Data Offset info

Do you have a old efl_ui_layout.eo.c arround somewhere in your tree ?

I think I resolve the problem by recursive calls of efl_isa for mro

Can you paste you efl_ui_layout.eo.c ? I just checked it and it seems that efl.ui.view is part of efl.ui.layout's MRO here.

However, if MRO does not contain this file for you for some reason then we should probebly fix the mro generation :)

yes it is.. but when i try to search the Efl.Ui.View Interface from the Efl.Ui.List_Default_Item which is the descendant of Efl.Ui.Layout, I cannot find it.

kls_itr = lookinto->mro;
if (!kls_itr) return EINA_FALSE;

// skip the self klass check.
kls_itr++;

while (*kls_itr)
  {
     if (efl_isa((const Eo *)_eo_class_id_get(*kls_itr), klass_id))
       return EINA_TRUE;
     kls_itr++;
  }

kls_itr = lookinto->extensions;
if (!kls_itr) return EINA_FALSE;

while (*kls_itr)
  {
     if (efl_isa((const Eo *)_eo_class_id_get(*kls_itr), klass_id))
       return EINA_TRUE;
     kls_itr++;
  }

I've checked upon code will fix the bug but it checking too many duplicated class in the loop.

Do you have a old efl_ui_layout.eo.c arround somewhere in your tree ?

I've cleaned it all for meson test every day... sadly :(

regardless of my test case,
SIMPLE_CLASS test in interface_main also not passed with current mro traverse.

SanghyeonLee edited reviewers, added: q66; removed: QA_Igor.Fri, Feb 1, 1:50 AM

It appears that we are recursivly skipping interfaces, but not on the direct implementer. I will look later next week into it.

It appears that we are recursivly skipping interfaces, but not on the direct implementer. I will look later next week into it.

so you mean we need to fix mro itself not efl_isa right?
I think that is right way to solve this problem..

@SanghyeonLee please check the attached patch :)

SanghyeonLee accepted this revision.Fri, Feb 1, 2:23 AM

I tested and it fix the problem well :) thank you.

SanghyeonLee requested changes to this revision.Fri, Feb 1, 2:23 AM

oh its wrong patch.

What do you mean ? Does my patch work or not ? :)