efl-mono: Fix call of virtual methods after new API

Summary:
After the new API, the virtual wrapper classes (*NativeInherit) just
declared the wrappers for the current class. But as they didn't have any
inheritance information, reimplementing methods from a parent Eo class
wouldn't work. (e.g. Efl.Ui.Button reimplementing Efl.Object
FinalizeAdd).

This commit changes these NativeInherit classes to mimic the inheritance
chain of their regular/abstract counterparts, reusing the virtual
wrapper implementations.

In order to access the correct Eo class created, the methods on it were
changed from static to instance methods. The instance will be held as a
class member of the regular/abstract API class to keep the delegates
alive and allow getting C Function pointers from them.

The class_initializer method was also split in two. The method
collecting the wrapper delegates was extracted in order to call the
parent ones.

Also avoid exception in cached strings queries as TryGetValue requires
non-null keys.

Test Plan: Run test suite.

Reviewers: vitor.sousa, felipealmeida

Reviewed By: vitor.sousa

Subscribers: cedric, #reviewers, #committers

Tags: #efl

Differential Revision: https://phab.enlightenment.org/D7460

9 files changed, 124 insertions, 19 deletions

diff --git a/src/bin/eolian_mono/eolian/mono/function_definition.hh b/src/bin/eolian_mono/eolian/mono/function_definition.hh
index 2821360a04..756136007c 100644
--- a/src/bin/eolian_mono/eolian/mono/function_definition.hh
+++ b/src/bin/eolian_mono/eolian/mono/function_definition.hh
@@ -75,7 +75,7 @@ struct native_function_definition_generator
 
     if(!as_generator
        (scope_tab
-        << " private static "
+        << " private "
         << eolian_mono::marshall_type(true) << " "
         << string
         << "(System.IntPtr obj, System.IntPtr pd"
@@ -98,7 +98,7 @@ struct native_function_definition_generator
         << eolian_mono::native_function_definition_epilogue(*klass)
         << scope_tab << scope_tab << "} else {\n"
         << scope_tab << scope_tab << scope_tab << (return_type != " void" ? "return " : "") << string
-        << "(Efl.Eo.Globals.efl_super(obj, " << klass_inherit_name << ".klass)" << *(", " << argument) << ");\n"
+        << "(Efl.Eo.Globals.efl_super(obj, " << "EoKlass)" << *(", " << argument) << ");\n"
         << scope_tab << scope_tab << "}\n"
         << scope_tab << "}\n"
        )
@@ -114,13 +114,10 @@ struct native_function_definition_generator
                  , context))
       return false;
 
-    if(!as_generator
-       (scope_tab << "private static  "
-        << string
-        << "_delegate "
-        << string << "_static_delegate = new " << string << "_delegate(" << name_helpers::klass_native_inherit_name(*klass) << "." << string << ");\n"
-       )
-       .generate(sink, std::make_tuple(f.c_name, f.c_name, f.c_name, escape_keyword(f.name)), context))
+    // This is the delegate that will be passed to Eo to be called from C.
+    if(!as_generator(
+            scope_tab << "private " << f.c_name << "_delegate " << f.c_name << "_static_delegate;\n"
+        ).generate(sink, attributes::unused, context))
       return false;
     return true;
       }
diff --git a/src/bin/eolian_mono/eolian/mono/function_helpers.hh b/src/bin/eolian_mono/eolian/mono/function_helpers.hh
index 06bf5d44ec..017ddad9d8 100644
--- a/src/bin/eolian_mono/eolian/mono/function_helpers.hh
+++ b/src/bin/eolian_mono/eolian/mono/function_helpers.hh
@@ -41,7 +41,7 @@ struct native_function_definition_preamble_generator
                 ).generate(sink, std::make_tuple(f.parameters, f.parameters, f.parameters, f.return_type), context))
           return false;
 
-      return true;
+      return as_generator("\n").generate(sink, attributes::unused, context);
   }
 };
 
diff --git a/src/bin/eolian_mono/eolian/mono/function_registration.hh b/src/bin/eolian_mono/eolian/mono/function_registration.hh
index e8a41047f9..0b8da4a05a 100644
--- a/src/bin/eolian_mono/eolian/mono/function_registration.hh
+++ b/src/bin/eolian_mono/eolian/mono/function_registration.hh
@@ -35,14 +35,20 @@ struct function_registration_generator
     else
       {
     auto index = index_generator();
-    
+
+    if(!as_generator(
+            scope_tab << scope_tab << f.c_name << "_static_delegate = new " << f.c_name << "_delegate(" <<
+                escape_keyword(f.name) << ");\n"
+        ).generate(sink, attributes::unused, context))
+      return false;
+
     if(!as_generator
 #ifdef _WIN32
        (scope_tab << scope_tab << "descs[" << index << "].api_func = Marshal.StringToHGlobalAnsi(\"" << string << "\");\n"
 #else
        (scope_tab << scope_tab << "descs[" << index << "].api_func = Efl.Eo.Globals.dlsym(Efl.Eo.Globals.RTLD_DEFAULT, \"" << string << "\");\n"
 #endif
-        << scope_tab << scope_tab << "descs[" << index << "].func = Marshal.GetFunctionPointerForDelegate(" << name_helpers::klass_native_inherit_name(*klass) << "." << string << "_static_delegate);\n"
+        << scope_tab << scope_tab << "descs[" << index << "].func = Marshal.GetFunctionPointerForDelegate(" << string << "_static_delegate);\n"
        )
        .generate(sink, std::make_tuple(f.c_name, f.c_name), context))
       return false;
diff --git a/src/bin/eolian_mono/eolian/mono/klass.hh b/src/bin/eolian_mono/eolian/mono/klass.hh
index f10c3a38d4..90133291a8 100644
--- a/src/bin/eolian_mono/eolian/mono/klass.hh
+++ b/src/bin/eolian_mono/eolian/mono/klass.hh
@@ -357,10 +357,19 @@ struct klass
        {
          auto inative_cxt = context_add_tag(class_context{class_context::inherit_native}, context);
          auto native_inherit_name = name_helpers::klass_native_inherit_name(cls);
+         auto inherit_name = name_helpers::klass_inherit_name(cls);
+         std::string base_name;
+         if(!root)
+           {
+              attributes::klass_def parent_klass(get_klass(*cls.parent, cls.unit), cls.unit);
+              base_name = name_helpers::klass_full_native_inherit_name(parent_klass);
+           }
+
          if(!as_generator
             (
-             "internal " << class_type << " " << native_inherit_name << " {\n"
-             << scope_tab << "public static byte class_initializer(IntPtr klass)\n"
+             "public " << class_type << " " << native_inherit_name << " " << (root ? "" : (": " + base_name)) <<"{\n"
+             << scope_tab << (root ? "protected IntPtr EoKlass { get; set; }\n" : "\n")
+             << scope_tab << "public " << (root ? "" : "new ") << "Efl_Op_Description[] GetEoOps()\n"
              << scope_tab << "{\n"
              << scope_tab << scope_tab << "Efl_Op_Description[] descs = new Efl_Op_Description[" << grammar::int_ << "];\n"
             )
@@ -371,20 +380,35 @@ struct klass
          if(!as_generator(*(function_registration(index_generator, cls)))
             .generate(sink, helpers::get_all_implementable_methods(cls), inative_cxt)) return false;
 
+         if(!root)
+           if(!as_generator(scope_tab << scope_tab << "descs = descs.Concat(base.GetEoOps()).ToArray();\n").generate(sink, attributes::unused, inative_cxt))
+             return false;
+
+         if(!as_generator(
+                scope_tab << scope_tab << "return descs;\n"
+                << scope_tab << "}\n"
+            ).generate(sink, attributes::unused, inative_cxt))
+           return false;
+
          if(!as_generator
-            (   scope_tab << scope_tab << "IntPtr descs_ptr = Marshal.AllocHGlobal(Marshal.SizeOf(descs[0])*" << function_count << ");\n"
+            (scope_tab << "public " << (root ? "" : "new " ) << "byte class_initializer(IntPtr klass)\n"
+             << scope_tab << "{\n"
+             << scope_tab << scope_tab << "var descs = GetEoOps();\n"
+             << scope_tab << scope_tab << "var count = descs.Length;\n"
+             << scope_tab << scope_tab << "IntPtr descs_ptr = Marshal.AllocHGlobal(Marshal.SizeOf(descs[0])*count);\n"
              << scope_tab << scope_tab << "IntPtr ptr = descs_ptr;\n"
-             << scope_tab << scope_tab << "for(int i = 0; i != " << function_count << "; ++i)\n"
+             << scope_tab << scope_tab << "for(int i = 0; i != count; ++i)\n"
              << scope_tab << scope_tab << "{\n"
              << scope_tab << scope_tab << scope_tab << "Marshal.StructureToPtr(descs[i], ptr, false);\n"
              << scope_tab << scope_tab << scope_tab << "ptr = IntPtr.Add(ptr, Marshal.SizeOf(descs[0]));\n"
              << scope_tab << scope_tab << "}\n"
              << scope_tab << scope_tab << "Efl_Object_Ops ops;\n"
              << scope_tab << scope_tab << "ops.descs = descs_ptr;\n"
-             << scope_tab << scope_tab << "ops.count = (UIntPtr)" << function_count << ";\n"
+             << scope_tab << scope_tab << "ops.count = (UIntPtr)count;\n"
              << scope_tab << scope_tab << "IntPtr ops_ptr = Marshal.AllocHGlobal(Marshal.SizeOf(ops));\n"
              << scope_tab << scope_tab << "Marshal.StructureToPtr(ops, ops_ptr, false);\n"
              << scope_tab << scope_tab << "Efl.Eo.Globals.efl_class_functions_set(klass, ops_ptr, IntPtr.Zero);\n"
+             << scope_tab << scope_tab << "EoKlass = klass;\n"
             ).generate(sink, attributes::unused, inative_cxt)) return false;
 
 
@@ -413,6 +437,7 @@ struct klass
      bool is_inherit = is_inherit_context(context);
 
      std::string class_getter = "return Efl.Eo.Globals.efl_class_get(handle);";
+     std::string native_inherit_full_name = name_helpers::klass_full_native_inherit_name(cls);
 
      // The klass field is static but there is no problem if multiple C# classes inherit from this generated one
      // as it is just a simple wrapper, forwarding the Eo calls either to the user API (where C#'s virtual method
@@ -421,6 +446,7 @@ struct klass
        {
           if(!as_generator(
              scope_tab << "public " << (root ? "" : "new ") <<  "static System.IntPtr klass = System.IntPtr.Zero;\n"
+             << scope_tab << "public " << (root ? "" : "new ") << "static " << native_inherit_full_name << " nativeInherit = new " << native_inherit_full_name << "();\n"
              ).generate(sink, attributes::unused, context))
             return false;
           class_getter = "return klass;";
@@ -493,7 +519,7 @@ struct klass
                      scope_tab << "///<summary>Creates a new instance.</summary>\n"
                      << scope_tab << "///<param name=\"parent\">Parent instance.</param>\n"
                      << scope_tab << "///<param name=\"init_cb\">Delegate to call constructing methods that should be run inside the constructor.</param>\n"
-                     << scope_tab << "public " << inherit_name << "(Efl.Object parent = null, ConstructingMethod init_cb=null) : base(" << native_inherit_name << ".class_initializer, \"" << inherit_name << "\", " << name_helpers::klass_get_name(cls) <<  "(), typeof(" << inherit_name << "), parent, ref klass)\n"
+                     << scope_tab << "public " << inherit_name << "(Efl.Object parent = null, ConstructingMethod init_cb=null) : base(nativeInherit.class_initializer, \"" << inherit_name << "\", " << name_helpers::klass_get_name(cls) <<  "(), typeof(" << inherit_name << "), parent, ref klass)\n"
                      << scope_tab << "{\n"
                      << scope_tab << scope_tab << "if (init_cb != null) {\n"
                      << scope_tab << scope_tab << scope_tab << "init_cb(this);\n"
@@ -518,7 +544,7 @@ struct klass
              scope_tab << "///<summary>Creates a new instance.</summary>\n"
              << scope_tab << "///<param name=\"parent\">Parent instance.</param>\n"
              << scope_tab << "///<param name=\"init_cb\">Delegate to call constructing methods that should be run inside the constructor.</param>\n"
-             << scope_tab << "public " << inherit_name << "(Efl.Object parent = null, ConstructingMethod init_cb=null) : this(" << native_inherit_name << ".class_initializer, \"" << inherit_name << "\", " << name_helpers::klass_get_name(cls) << "(), typeof(" << inherit_name << "), parent, ref klass)\n"
+             << scope_tab << "public " << inherit_name << "(Efl.Object parent = null, ConstructingMethod init_cb=null) : this(nativeInherit.class_initializer, \"" << inherit_name << "\", " << name_helpers::klass_get_name(cls) << "(), typeof(" << inherit_name << "), parent, ref klass)\n"
              << scope_tab << "{\n"
              << scope_tab << scope_tab << "if (init_cb != null) {\n"
              << scope_tab << scope_tab << scope_tab << "init_cb(this);\n"
diff --git a/src/bin/eolian_mono/eolian/mono/name_helpers.hh b/src/bin/eolian_mono/eolian/mono/name_helpers.hh
index 06291f412c..cfda7229f6 100644
--- a/src/bin/eolian_mono/eolian/mono/name_helpers.hh
+++ b/src/bin/eolian_mono/eolian/mono/name_helpers.hh
@@ -354,6 +354,12 @@ inline std::string klass_native_inherit_name(T const& klass)
 }
 
 template<typename T>
+inline std::string klass_full_native_inherit_name(T const& klass)
+{
+  return klass_full_concrete_name(klass) + "NativeInherit";
+}
+
+template<typename T>
 inline std::string klass_get_name(T const& clsname)
 {
   return utils::to_lowercase(join_namespaces(clsname.namespaces, '_') + clsname.eolian_name + "_class_get");
diff --git a/src/bin/eolian_mono/eolian_mono.cc b/src/bin/eolian_mono/eolian_mono.cc
index 328b983052..2e0e8918cb 100644
--- a/src/bin/eolian_mono/eolian_mono.cc
+++ b/src/bin/eolian_mono/eolian_mono.cc
@@ -131,6 +131,7 @@ run(options_type const& opts)
    if (!as_generator("using System;\n"
                      "using System.Runtime.InteropServices;\n"
                      "using System.Collections.Generic;\n"
+                     "using System.Linq;\n"
                      "using System.ComponentModel;\n")
      .generate(iterator, efl::eolian::grammar::attributes::unused, efl::eolian::grammar::context_null()))
      {
diff --git a/src/bindings/mono/eo_mono/iwrapper.cs b/src/bindings/mono/eo_mono/iwrapper.cs
index 11e9fa8091..35ee606540 100644
--- a/src/bindings/mono/eo_mono/iwrapper.cs
+++ b/src/bindings/mono/eo_mono/iwrapper.cs
@@ -141,6 +141,10 @@ public class Globals {
     public static IntPtr cached_string_to_intptr(Dictionary<String, IntPtr> dict, String str)
     {
         IntPtr ptr = IntPtr.Zero;
+
+        if (str == null)
+            return ptr;
+
         if (!dict.TryGetValue(str, out ptr))
         {
             ptr = Eina.StringConversion.ManagedStringToNativeUtf8Alloc(str);
@@ -153,6 +157,10 @@ public class Globals {
     public static IntPtr cached_stringshare_to_intptr(Dictionary<String, IntPtr> dict, String str)
     {
         IntPtr ptr = IntPtr.Zero;
+
+        if (str == null)
+            return ptr;
+
         if (!dict.TryGetValue(str, out ptr))
         {
             ptr = Eina.Stringshare.eina_stringshare_add(str);
diff --git a/src/tests/efl_mono/Eo.cs b/src/tests/efl_mono/Eo.cs
index 7d5df72457..5567f1ef33 100644
--- a/src/tests/efl_mono/Eo.cs
+++ b/src/tests/efl_mono/Eo.cs
@@ -345,4 +345,39 @@ class TestCsharpProperties
     }
 }
 
+class TestEoGrandChildrenFinalize
+{
+    public sealed class Child : Dummy.TestObject
+    {
+        public int receivedValue = 0;
+        public override Efl.Object FinalizeAdd()
+        {
+            receivedValue = 42;
+            return this;
+        }
+    }
+
+    public static void test_grand_children_finalize()
+    {
+        Child obj = new Child();
+        Test.AssertEquals(42, obj.receivedValue);
+    }
+
+    public sealed class GrandChild : Dummy.Child
+    {
+        public int receivedValue = 0;
+        public override Efl.Object FinalizeAdd()
+        {
+            receivedValue = -42;
+            return this;
+        }
+    }
+
+    public static void test_grand_grand_children_finalize()
+    {
+        GrandChild obj = new GrandChild();
+        Test.AssertEquals(-42, obj.receivedValue);
+    }
+}
+
 }
diff --git a/src/tests/efl_mono/libefl_mono_native_test.c b/src/tests/efl_mono/libefl_mono_native_test.c
index 52876e6859..841c6a92c6 100644
--- a/src/tests/efl_mono/libefl_mono_native_test.c
+++ b/src/tests/efl_mono/libefl_mono_native_test.c
@@ -36,6 +36,7 @@
 
 #include "dummy_numberwrapper.eo.h"
 #include "dummy_test_object.eo.h"
+#include "dummy_child.eo.h"
 #include "dummy_test_iface.eo.h"
 #include "dummy_another_iface.eo.h"
 
@@ -66,6 +67,10 @@ typedef struct Dummy_Numberwrapper_Data
    int number;
 } Dummy_Numberwrapper_Data;
 
+typedef struct Dummy_Child_Data
+{
+} Dummy_Child_Data;
+
 
 static
 void *_new_int(int v)
@@ -3898,8 +3903,29 @@ int _dummy_test_object_dummy_test_iface_iface_prop_get(EINA_UNUSED const Eo *obj
     return pd->iface_prop;
 }
 
+/// Dummy.Child
+static Efl_Object *
+_dummy_child_efl_object_constructor(Eo *obj, EINA_UNUSED Dummy_Child_Data *pd)
+{
+    efl_constructor(efl_super(obj, DUMMY_CHILD_CLASS));
+    return obj;
+}
+
+EOLIAN static void
+_dummy_child_class_constructor(Efl_Class *klass)
+{
+    (void)klass;
+}
+
+EOLIAN static void
+_dummy_child_class_destructor(Efl_Class *klass)
+{
+    (void)klass;
+}
+
 #include "dummy_test_object.eo.c"
 #include "dummy_numberwrapper.eo.c"
+#include "dummy_child.eo.c"
 #include "dummy_test_iface.eo.c"
 #include "dummy_another_iface.eo.c"