csharp: refactor native_inherit into a nested class.

Summary:
Efl.Ui.Button_NativeInherit -> Efl.Ui.Button.NativeMethods

Will help using EFL# with completion tools.

* Added pragmas around the native function definitions to avoid
  warnings related to the name of native functions
* Updated some style fixes for native function wrappers.

Their preamble and epilogue styling will be dealt with in future diffs.

As a side effect, concrete classes had to be made public again as they
hold the function pointers to the native methods of their interfaces.
Thus a third party library class that implements IFoo should be able to
access these methods.

Fixes T7743

Depends on D8622

Reviewers: vitor.sousa, felipealmeida, segfaultxavi

Reviewed By: vitor.sousa, segfaultxavi

Subscribers: cedric, #reviewers, #committers

Tags: #efl

Maniphest Tasks: T7743

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

src/bin/eolian_mono/eolian/mono/function_definition.hh

@@ -34,12 +34,14 @@ struct native_function_definition_generator
     EINA_CXX_DOM_LOG_DBG(eolian_mono::domain) << "native_function_definition_generator: " << f.c_name << std::endl;
     if(blacklist::is_function_blacklisted(f, context))
       return true;
-    else
-      {
+
+    auto const& indent = current_indentation(context);
+
+    // Delegate for the C# method we will export to EO as a method implementation.
     if(!as_generator
-       ("\n\n" << scope_tab
-        << eolian_mono::marshall_annotation(true)
-        << " private delegate "
+       (
+        indent << eolian_mono::marshall_annotation(true) << "\n"
+        << indent << "private delegate "
         << eolian_mono::marshall_type(true)
         << " "
         << string
@@ -49,14 +51,15 @@ struct native_function_definition_generator
         (
          (marshall_annotation << " " << marshall_parameter)
         ) % ", ")
-        << ");\n")
+        << ");\n\n")
        .generate(sink, std::make_tuple(f.return_type, f.return_type, f.c_name, f.parameters), context))
       return false;
 
+    // API delegate is the wrapper for the Eo methods exported from C that we will use from C#.
     if(!as_generator
-       ("\n\n" << scope_tab
-        << eolian_mono::marshall_annotation(true)
-        << " public delegate "
+       (
+        indent << eolian_mono::marshall_annotation(true) << "\n"
+        << indent << "public delegate "
         << eolian_mono::marshall_type(true)
         << " "
         << string << "_api_delegate(" << (f.is_static ? "" : "System.IntPtr obj")
@@ -65,17 +68,18 @@ struct native_function_definition_generator
         (
          (marshall_annotation << " " << marshall_parameter)
         ) % ", ")
-        << ");\n")
+        << ");\n\n")
        .generate(sink, std::make_tuple(f.return_type, f.return_type, f.c_name, f.parameters), context))
       return false;
 
+    // Delegate holder (so it can't be collected).
     if(!as_generator
-       (scope_tab
-        << " public static Efl.Eo.FunctionWrapper<" << string << "_api_delegate> " << string << "_ptr = new Efl.Eo.FunctionWrapper<"
-        << string << "_api_delegate>(_Module, \"" << string << "\");\n")
+       (indent << "public static Efl.Eo.FunctionWrapper<" << string << "_api_delegate> " << string << "_ptr = new Efl.Eo.FunctionWrapper<"
+          << string << "_api_delegate>(Module, \"" << string << "\");\n\n")
        .generate(sink, std::make_tuple(f.c_name, f.c_name, f.c_name, f.c_name), context))
       return false;
 
+    // Actual method implementation to be called from C.
     std::string return_type;
     if(!as_generator(eolian_mono::type(true)).generate(std::back_inserter(return_type), f.return_type, context))
       return false;
@@ -92,34 +96,37 @@ struct native_function_definition_generator
       self = "";
 
     if(!as_generator
-       (scope_tab
-        << " private static "
+       (indent << "private static "
         << eolian_mono::marshall_type(true) << " "
         << string
         << "(System.IntPtr obj, System.IntPtr pd"
         << *(", " << marshall_parameter)
         << ")\n"
-        << scope_tab << "{\n"
-        /****/
-        << scope_tab << scope_tab << "Eina.Log.Debug(\"function " << string << " was called\");\n"
-        /****/
-        << scope_tab << scope_tab << "Efl.Eo.IWrapper wrapper = Efl.Eo.Globals.PrivateDataGet(pd);\n"
-        << scope_tab << scope_tab << "if(wrapper != null) {\n"
-        << scope_tab << scope_tab << scope_tab << eolian_mono::native_function_definition_preamble()
-        << scope_tab << scope_tab << scope_tab << "try {\n"
-        << scope_tab << scope_tab << scope_tab << scope_tab << (return_type != "void" ? "_ret_var = " : "")
+        << indent << "{\n"
+        << indent << scope_tab << "Eina.Log.Debug(\"function " << string << " was called\");\n"
+        << indent << scope_tab << "Efl.Eo.IWrapper wrapper = Efl.Eo.Globals.PrivateDataGet(pd);\n"
+        << indent << scope_tab << "if (wrapper != null)\n"
+        << indent << scope_tab << "{\n"
+        << eolian_mono::native_function_definition_preamble()
+        << indent << scope_tab << scope_tab << "try\n"
+        << indent << scope_tab << scope_tab << "{\n"
+        << indent << scope_tab << scope_tab << scope_tab << (return_type != "void" ? "_ret_var = " : "")
         << (f.is_static ? "" : "((") << klass_cast_name << (f.is_static ? "." : ")wrapper).") << string
         << "(" << (native_argument_invocation % ", ") << ");\n"
-        << scope_tab << scope_tab << scope_tab << "} catch (Exception e) {\n"
-        << scope_tab << scope_tab << scope_tab << scope_tab << "Eina.Log.Warning($\"Callback error: {e.ToString()}\");\n"
-        << scope_tab << scope_tab << scope_tab << scope_tab << "Eina.Error.Set(Eina.Error.UNHANDLED_EXCEPTION);\n"
-        << scope_tab << scope_tab << scope_tab << "}\n"
-        << eolian_mono::native_function_definition_epilogue(*klass)
-        << scope_tab << scope_tab << "} else {\n"
-        << scope_tab << scope_tab << scope_tab << (return_type != "void" ? "return " : "") << string
+        << indent << scope_tab << scope_tab << "}\n"
+        << indent << scope_tab << scope_tab << "catch (Exception e)\n"
+        << indent << scope_tab << scope_tab << "{\n"
+        << indent << scope_tab << scope_tab << scope_tab << "Eina.Log.Warning($\"Callback error: {e.ToString()}\");\n"
+        << indent << scope_tab << scope_tab << scope_tab << "Eina.Error.Set(Eina.Error.UNHANDLED_EXCEPTION);\n"
+        << indent << scope_tab << scope_tab << "}\n\n"
+        << eolian_mono::native_function_definition_epilogue(*klass) << "\n"
+        << indent << scope_tab << "}\n"
+        << indent << scope_tab << "else\n"
+        << indent << scope_tab << "{\n"
+        << indent << scope_tab << scope_tab << (return_type != "void" ? "return " : "") << string
         << "_ptr.Value.Delegate(" << self << ((!f.is_static && f.parameters.size() > 0) ? ", " : "") << (argument % ", ") << ");\n"
-        << scope_tab << scope_tab << "}\n"
-        << scope_tab << "}\n"
+        << indent << scope_tab << "}\n"
+        << indent << "}\n\n"
        )
        .generate(sink, std::make_tuple(f.return_type, escape_keyword(f.name), f.parameters
                                        , /***/f.c_name/***/
@@ -139,11 +146,11 @@ struct native_function_definition_generator
 
     // This is the delegate that will be passed to Eo to be called from C.
     if(!as_generator(
-            scope_tab << "private static " << f.c_name << "_delegate " << f.c_name << "_static_delegate;\n"
+            indent << "private static " << f.c_name << "_delegate " << f.c_name << "_static_delegate;\n\n"
         ).generate(sink, attributes::unused, context))
       return false;
+
     return true;
-      }
   }
 };
   

src/bin/eolian_mono/eolian/mono/function_registration.hh

@@ -30,35 +30,34 @@ struct function_registration_generator
   bool generate(OutputIterator sink, attributes::function_def const& f, Context const& context) const
   {
     EINA_CXX_DOM_LOG_DBG(eolian_mono::domain) << "function_registration_generator: " << f.name << std::endl;
+    auto const& indent = current_indentation(context);
+
     if(blacklist::is_function_blacklisted(f, context) || f.is_static) // Static methods aren't overrideable
       return true;
-    else
-      {
-    // auto index = index_generator();
 
     if(!as_generator(
-               scope_tab << scope_tab << "if (" << f.c_name << "_static_delegate == null)\n"
-            << scope_tab << scope_tab << scope_tab << f.c_name << "_static_delegate = new " << f.c_name << "_delegate(" <<
-                escape_keyword(f.name) << ");\n"
+               indent << "if (" << f.c_name << "_static_delegate == null)\n"
+               << indent << "{\n"
+               << indent << scope_tab << f.c_name << "_static_delegate = new " << f.c_name << "_delegate(" << escape_keyword(f.name) << ");\n"
+               << indent << "}\n\n"
         ).generate(sink, attributes::unused, context))
       return false;
 
-    if(!as_generator
-       (scope_tab << scope_tab
-        << "if (methods.FirstOrDefault(m => m.Name == \"" << string << "\") != null)\n"
-        << scope_tab << scope_tab << scope_tab
-        << "descs.Add(new Efl_Op_Description() {"
+    if(!as_generator(
+        indent << "if (methods.FirstOrDefault(m => m.Name == \"" << string << "\") != null)\n"
+        << indent << "{\n"
+        << indent << scope_tab << "descs.Add(new Efl_Op_Description() {"
 #ifdef _WIN32
         << "api_func = Marshal.StringToHGlobalAnsi(\"" << string << "\")"
 #else
-        << "api_func = Efl.Eo.FunctionInterop.LoadFunctionPointer(_Module.Module, \"" << string << "\")"
+        << "api_func = Efl.Eo.FunctionInterop.LoadFunctionPointer(Module.Module, \"" << string << "\")"
 #endif
-        << ", func = Marshal.GetFunctionPointerForDelegate(" << string << "_static_delegate)});\n"
+        << ", func = Marshal.GetFunctionPointerForDelegate(" << string << "_static_delegate) });\n"
+        << indent << "}\n\n"
        )
        .generate(sink, std::make_tuple(name_helpers::managed_method_name(f), f.c_name, f.c_name), context))
       return false;
     return true;
-      }
   }
 };
   

src/bin/eolian_mono/eolian/mono/klass.hh

@@ -88,6 +88,8 @@ struct klass
    {
      EINA_CXX_DOM_LOG_DBG(eolian_mono::domain) << "klass_generator: " << cls.eolian_name << std::endl;
 
+     auto const& indent = current_indentation(context);
+
      if (blacklist::is_class_blacklisted(cls, context))
        {
           EINA_CXX_DOM_LOG_DBG(eolian_mono::domain) << "class " << cls.eolian_name << " is blacklisted. Skipping." << std::endl;
@@ -131,7 +133,7 @@ struct klass
          return false;
 
        // Mark the interface with the proper native Efl_Class* getter
-       if(!as_generator(lit("[") << name_helpers::klass_native_inherit_name(cls) << "]\n")
+       if(!as_generator(lit("[") << name_helpers::klass_full_native_inherit_name(cls) << "]\n")
           .generate(sink, attributes::unused, iface_cxt))
          return false;
 
@@ -209,10 +211,12 @@ struct klass
          auto concrete_name = name_helpers::klass_concrete_name(cls);
          auto interface_name = name_helpers::klass_interface_name(cls);
 
+         // We can't make these internal yet as they have methods that are used by
+         // other classes that implement the interface.
          if(!as_generator
             (
              documentation
-             << "sealed internal class " << concrete_name << " : " << "\n"
+             << "sealed public class " << concrete_name << " : " << "\n"
              << (klass_full_concrete_or_interface_name % ",") << "\n"
              << (inherit_classes.size() > 0 ? ", " : "" ) << interface_name << "\n"
              << scope_tab << *(", " << name_helpers::klass_full_concrete_or_interface_name) << "\n"
@@ -281,6 +285,8 @@ struct klass
            ).generate(sink, attributes::unused, concrete_cxt))
            return false;
 
+         if(!generate_native_inherit_class(sink, cls, change_indentation(indent.inc(), context)))
+           return true;
 
          if(!as_generator("}\n").generate(sink, attributes::unused, concrete_cxt)) return false;
        }
@@ -296,7 +302,7 @@ struct klass
         if(!as_generator
             (
              documentation
-             << "[" << name_helpers::klass_native_inherit_name(cls) << "]\n"
+             << "[" << name_helpers::klass_full_native_inherit_name(cls) << "]\n"
              << "public " << class_type << " " << name_helpers::klass_concrete_name(cls) << " : "
              << (klass_full_concrete_or_interface_name % ",") // classes
              << (inherit_classes.empty() ? "" : ",")
@@ -358,11 +364,24 @@ struct klass
            ).generate(sink, attributes::unused, inherit_cxt))
            return false;
 
+         if(!generate_native_inherit_class(sink, cls, change_indentation(indent.inc(), context)))
+           return true;
+
          if(!as_generator("}\n").generate(sink, attributes::unused, inherit_cxt)) return false;
        }
 
-     // Native Inherit class
-     //if(class_type == "class")
+
+     if(!name_helpers::close_namespaces(sink, cls.namespaces, context))
+       return false;
+
+     return true;
+   }
+
+   // NativeInherit class. Contains function pointers for the native Eo methods and delegates that are registered in Eo as virtual method implementations
+   // These delegates are called from C to C#, checking whether the C# subclass reimplemented it.
+   template <typename OutputIterator, typename Context>
+   bool generate_native_inherit_class(OutputIterator sink, attributes::klass_def const& cls, Context const& context) const
+   {
        {
          auto inative_cxt = context_add_tag(class_context{class_context::inherit_native,
                                             name_helpers::klass_full_concrete_or_interface_name(cls)},
@@ -370,6 +389,9 @@ struct klass
          auto native_inherit_name = name_helpers::klass_native_inherit_name(cls);
          auto inherit_name = name_helpers::klass_inherit_name(cls);
          auto implementable_methods = helpers::get_all_implementable_methods(cls);
+         bool root = !helpers::has_regular_ancestor(cls);
+         auto const& indent = current_indentation(inative_cxt);
+
          std::string base_name;
          if(!root)
            {
@@ -379,12 +401,28 @@ struct klass
 
          if(!as_generator
             (
-             "public class " << native_inherit_name << " " << (root ? " : Efl.Eo.NativeClass" : (": " + base_name)) <<"{\n"
-             << scope_tab << "public " << (root ? "" : "new ") << " static Efl.Eo.NativeModule _Module = new Efl.Eo.NativeModule("
-             << context_find_tag<library_context>(context).actual_library_name(cls.filename) << ");\n"
-             << scope_tab << "public override System.Collections.Generic.List<Efl_Op_Description> GetEoOps(System.Type type)\n"
-             << scope_tab << "{\n"
-             << scope_tab << scope_tab << "var descs = new System.Collections.Generic.List<Efl_Op_Description>();\n"
+             indent << lit("/// <summary>Wrapper for native methods and virtual method delegates.\n")
+             << indent << "/// For internal use by generated code only.</summary>\n"
+             << indent << "public " << (root ? "" : "new " ) << "class " << native_inherit_name << " " << (root ? " : Efl.Eo.NativeClass" : (": " + base_name)) <<"\n"
+             << indent << "{\n"
+            ).generate(sink, attributes::unused, inative_cxt))
+           return false;
+
+         if(implementable_methods.size() >= 1)
+           {
+              if(!as_generator(
+                    indent << scope_tab << "private static Efl.Eo.NativeModule Module = new Efl.Eo.NativeModule("
+                    << indent << context_find_tag<library_context>(context).actual_library_name(cls.filename) << ");\n"
+                 ).generate(sink, attributes::unused, inative_cxt))
+                return false;
+           }
+
+         if(!as_generator(
+             indent << scope_tab << "/// <summary>Gets the list of Eo operations to override.</summary>\n"
+             << indent << scope_tab << "/// <returns>The list of Eo operations to be overload.</returns>\n"
+             << indent << scope_tab << "public override System.Collections.Generic.List<Efl_Op_Description> GetEoOps(System.Type type)\n"
+             << indent << scope_tab << "{\n"
+             << indent << scope_tab << scope_tab << "var descs = new System.Collections.Generic.List<Efl_Op_Description>();\n"
             )
             .generate(sink, attributes::unused, inative_cxt))
            return false;
@@ -394,53 +432,46 @@ struct klass
          // only non-registrable methods like class functions, leading to unused `methods` variable.
          std::string tmp_registration;
          if(!as_generator(*(function_registration(cls)))
-            .generate(std::back_inserter(tmp_registration), implementable_methods, inative_cxt))
+            .generate(std::back_inserter(tmp_registration), implementable_methods, change_indentation(indent.inc(2), inative_cxt)))
            return false;
 
          if (tmp_registration.find("methods") != std::string::npos)
            if (!as_generator(
-                    scope_tab << scope_tab << "var methods = Efl.Eo.Globals.GetUserMethods(type);\n"
+                    indent << scope_tab << scope_tab << "var methods = Efl.Eo.Globals.GetUserMethods(type);\n\n"
                     << tmp_registration
                 ).generate(sink,  attributes::unused, inative_cxt))
              return false;
 
          if(!root)
-           if(!as_generator(scope_tab << scope_tab << "descs.AddRange(base.GetEoOps(type));\n").generate(sink, attributes::unused, inative_cxt))
+           if(!as_generator(indent << scope_tab << scope_tab << "descs.AddRange(base.GetEoOps(type));\n").generate(sink, attributes::unused, inative_cxt))
              return false;
 
          if(!as_generator(
-                scope_tab << scope_tab << "return descs;\n"
-                << scope_tab << "}\n"
+                indent << scope_tab << scope_tab << "return descs;\n"
+                << indent << scope_tab << "}\n"
             ).generate(sink, attributes::unused, inative_cxt))
            return false;
 
          // Attribute getter of the native 'Efl_Class *' handle (for proper inheritance from additional explicit interfaces)
          if(!as_generator(
-                 scope_tab << "public override IntPtr GetEflClass()\n"
-              << scope_tab << "{\n"
-              << scope_tab << scope_tab << "return " << name_helpers::klass_get_full_name(cls) << "();\n"
-              << scope_tab << "}\n\n"
-           ).generate(sink, attributes::unused, inative_cxt))
-           return false;
-
-         if(!as_generator(
-              scope_tab << "public static " << (root ? "" : "new ") << " IntPtr GetEflClassStatic()\n"
-              << scope_tab << "{\n"
-              << scope_tab << scope_tab << "return " << name_helpers::klass_get_full_name(cls) << "();\n"
-              << scope_tab << "}\n\n"
+              indent << scope_tab << "/// <summary>Returns the Eo class for the native methods of this class.</summary>\n"
+              << indent << scope_tab << "/// <returns>The native class pointer.</returns>\n"
+              << indent << scope_tab << "public override IntPtr GetEflClass()\n"
+              << indent << scope_tab << "{\n"
+              << indent << scope_tab << scope_tab << "return " << name_helpers::klass_get_full_name(cls) << "();\n"
+              << indent << scope_tab << "}\n\n"
            ).generate(sink, attributes::unused, inative_cxt))
            return false;
 
          // Native method definitions
-         if(!as_generator(*(native_function_definition(cls)))
-            .generate(sink, implementable_methods, inative_cxt)) return false;
+         if(!as_generator(
+                indent << scope_tab << "#pragma warning disable CA1707, SA1300, SA1600\n\n"
+                <<  *(native_function_definition(cls))
+                << indent << scope_tab << "#pragma warning restore CA1707, SA1300, SA1600\n\n")
+            .generate(sink, implementable_methods, change_indentation(indent.inc(), inative_cxt))) return false;
 
          if(!as_generator("}\n").generate(sink, attributes::unused, inative_cxt)) return false;
        }
-
-     if(!name_helpers::close_namespaces(sink, cls.namespaces, context))
-       return false;
-
      return true;
    }
 
@@ -469,7 +500,7 @@ struct klass
                 << scope_tab << scope_tab << "{\n"
                 << scope_tab << scope_tab << scope_tab << "if (((object)this).GetType() == typeof(" << inherit_name << "))\n"
                 << scope_tab << scope_tab << scope_tab << "{\n"
-                << scope_tab << scope_tab << scope_tab << scope_tab << "return " << native_inherit_full_name << ".GetEflClassStatic();\n"
+                << scope_tab << scope_tab << scope_tab << scope_tab << "return GetEflClassStatic();\n"
                 << scope_tab << scope_tab << scope_tab << "}\n"
                 << scope_tab << scope_tab << scope_tab << "else\n"
                 << scope_tab << scope_tab << scope_tab << "{\n"
@@ -513,7 +544,6 @@ struct klass
    {
      bool root = !helpers::has_regular_ancestor(cls);
      auto inherit_name = name_helpers::klass_concrete_name(cls);
-     auto native_inherit_name = name_helpers::klass_native_inherit_name(cls);
 
      if(!as_generator(
              scope_tab << "[System.Runtime.InteropServices.DllImport(" << context_find_tag<library_context>(context).actual_library_name(cls.filename)

src/bin/eolian_mono/eolian/mono/name_helpers.hh

@@ -398,29 +398,15 @@ inline std::string klass_inherit_name(T const& klass)
 }
 
 template<typename T>
-inline std::string klass_native_inherit_name(T const& klass)
+inline std::string klass_native_inherit_name(EINA_UNUSED T const& klass)
 {
-  switch(klass.type)
-  {
-  case attributes::class_type::abstract_:
-  case attributes::class_type::regular:
-    return klass_concrete_name(klass) + "NativeInherit";
-  default:
-    return klass_interface_name(klass) + "NativeInherit";
-  }
+  return "NativeMethods";
 }
 
 template<typename T>
 inline std::string klass_full_native_inherit_name(T const& klass)
 {
-  switch(klass.type)
-  {
-  case attributes::class_type::abstract_:
-  case attributes::class_type::regular:
-    return klass_full_concrete_name(klass) + "NativeInherit";
-  default:
-    return klass_full_interface_name(klass) + "NativeInherit";
-  }
+  return klass_full_concrete_name(klass) + "." + klass_native_inherit_name(klass);
 }
 
 template<typename T>