Page MenuHomePhabricator
Create Task
Maniphest T8165

C#: Hide unecessary API
Open, HighPublic
Actions

Description

  1. Hiding unnecessary APIs
    • Now : many interop related APIs has public visibility e.g. efl_ui_win_wm_available_rotations_set_ptr
    • Suggested : many interop related APIs has private visibility e.g. efl_ui_win_wm_available_rotations_set_ptr

Details

Differential Revisions
D10102: csharp: Hide internal stuff from editors.

Related Objects
Search...

lauromoura created this task.Aug 21 2019, 11:01 AM
lauromoura triaged this task as High priority.
lauromoura added a comment.Aug 21 2019, 11:54 AM

We could try hiding them but AFAIR they should be public so inherited classes can use them.

What if we use [EditorBrowsable(EditorBrowsableState.Never)] to hide them from the editor?

[1] https://docs.microsoft.com/en-us/dotnet/api/system.componentmodel.editorbrowsableattribute?view=netframework-4.8

segfaultxavi added a comment.Aug 26 2019, 6:52 AM

If a method is meant to be used only by derived classes, shouldn't it be protected? What am I missing?

I don't like hiding it from the editor since it won't show up for writers of derived classes.

lauromoura added a comment.Aug 26 2019, 2:48 PM
In T8165#140891, @segfaultxavi wrote:

If a method is meant to be used only by derived classes, shouldn't it be protected? What am I missing?

I don't like hiding it from the editor since it won't show up for writers of derived classes.

I actually meant "inherited generated classes". No regular C# user (even overriding methods) will be using them.

zzervb added a subscriber: zzervb.Aug 27 2019, 1:12 AM

You'd better use internal instead if you want to hide them.

segfaultxavi added a comment.Aug 27 2019, 1:23 AM

Would that be an option, @lauromoura? Are all users of these methods inside the same assembly?

YOhoho added a subscriber: YOhoho.Sep 16 2019, 5:04 AM

If "inherited generated classes" are eo custom classes that inherited efl classes, internal will occur compiler error.
e.g.

mcs -d:EFL_BETA -target:library -out:src/tests/efl_mono/efl_mono_test.dll -r:src/bindings/mono/efl_mono.dll src/tests/efl_mono/dummy_child.eo.cs src/tests/efl_mono/dummy_numberwrapper.eo.cs src/tests/efl_mono/dummy_test_object.eo.cs src/tests/efl_mono/dummy_test_iface.eo.cs src/tests/efl_mono/dummy_inherit_helper.eo.cs src/tests/efl_mono/dummy_inherit_iface.eo.cs src/tests/efl_mono/dummy_part_holder.eo.cs src/tests/efl_mono/dummy_event_manager.eo.cs src/tests/efl_mono/dummy_constructible_object.eo.cs
src/tests/efl_mono/dummy_part_holder.eo.cs(83,80): error CS0122: `Efl.PartConcrete.NativeMethods.efl_part_get_ptr' is inaccessible due to its protection level

With this patch

diff --git a/src/bin/eolian_mono/eolian/mono/function_definition.hh b/src/bin/eolian_mono/eolian/mono/function_definition.hh
index a0f28df..149272a 100644
--- a/src/bin/eolian_mono/eolian/mono/function_definition.hh
+++ b/src/bin/eolian_mono/eolian/mono/function_definition.hh
@@ -61,7 +61,7 @@ struct native_function_definition_generator
     if(!as_generator
        (
         indent << eolian_mono::marshall_annotation(true) << "\n"
-        << indent << "public delegate "
+        << indent << "internal delegate "
         << eolian_mono::marshall_type(true)
         << " "
         << string << "_api_delegate(" << (f.is_static ? "" : "System.IntPtr obj")
@@ -76,7 +76,7 @@ struct native_function_definition_generator

     // Delegate holder (so it can't be collected).
     if(!as_generator
-       (indent << "public static Efl.Eo.FunctionWrapper<" << string << "_api_delegate> " << string << "_ptr = new Efl.Eo.FunctionWrapper<"
+       (indent << "internal 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;
diff --git a/src/bin/eolian_mono/eolian/mono/function_pointer.hh b/src/bin/eolian_mono/eolian/mono/function_pointer.hh
index 721368f..f4378d1 100644
--- a/src/bin/eolian_mono/eolian/mono/function_pointer.hh
+++ b/src/bin/eolian_mono/eolian/mono/function_pointer.hh
@@ -51,7 +51,7 @@ struct function_pointer {
           return false;
       // "Internal" delegate, 1-to-1 with the Unamaged function type
       if (!as_generator(marshall_annotation(true)
-                  << "public delegate " << marshall_type(true) << " " << string // public?
+                  << "internal delegate " << marshall_type(true) << " " << string
                   << "Internal(IntPtr data" << *grammar::attribute_reorder<-1, -1>((", " << marshall_annotation << " " << marshall_parameter)) << ");\n")
               .generate(sink, std::make_tuple(f.return_type, f.return_type, f_name, f.parameters), funcptr_ctx))
lauromoura added a comment.Sep 20 2019, 8:29 AM
In T8165#142364, @YOhoho wrote:

If "inherited generated classes" are eo custom classes that inherited efl classes, internal will occur compiler error.

Yeah, I'm referring to eolian-generated classes that go in other assemblies, like the test suite.

Generated classes sometimes may refer to these methods, like when implementing an interface, where it accesses the SomeInterfaceConcrete.NativeMethods.some_method_ptr. That is why they can't be always protected/internal and must be kept public (although already in a "reserved" namespace/class, like that NativeMethods).