From 73df0d47ff914211ee0a0b5fe46a381f1cd30f10 Mon Sep 17 00:00:00 2001 From: Vitor Sousa Date: Fri, 28 Jun 2019 18:20:01 -0300 Subject: [PATCH] csharp: encapsulate some internal code of EoWrapper Summary: Encapsulate some parts of EoWrapper making them less accessible to lib users. This can avoid unnecessary and risky usage of code that is only intended for internal usage. `inherited` field was made private and renamed to `generated`. Now its value can only be obtained through the `IsGeneratedBindingClass` property. `handle` field was made private. `eventLock` was renamed to `eflBindingEventLock` `ConstructingHandle` property set was made private. Constructors that are used to create new EFL# managed objects by wrapping a preexisting eo handle now receive a specific struct wrapping the handle pointer. This can avoid faulty interactions with the Reflection engine used only for generated classes that implement this constructor. Test Plan: meson test Reviewers: lauromoura, felipealmeida, YOhoho Subscribers: cedric, #reviewers, #committers Tags: #efl Differential Revision: https://phab.enlightenment.org/D9212 --- src/bin/eolian_mono/eolian/mono/events.hh | 4 +-- .../eolian/mono/function_definition.hh | 5 ++-- src/bin/eolian_mono/eolian/mono/klass.hh | 6 ++-- src/bindings/mono/eo_mono/EoWrapper.cs | 28 +++++++++++-------- src/bindings/mono/eo_mono/iwrapper.cs | 20 +++++++++++-- src/tests/efl_mono/EoConstruction.cs | 9 +++--- 6 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/bin/eolian_mono/eolian/mono/events.hh b/src/bin/eolian_mono/eolian/mono/events.hh index 9b4b8c362f..c1a45033f9 100644 --- a/src/bin/eolian_mono/eolian/mono/events.hh +++ b/src/bin/eolian_mono/eolian/mono/events.hh @@ -435,7 +435,7 @@ struct event_definition_generator scope_tab << "{\n" << scope_tab << scope_tab << "add\n" << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << "lock (eventLock)\n" + << scope_tab << scope_tab << scope_tab << "lock (eflBindingEventLock)\n" << scope_tab << scope_tab << scope_tab << "{\n" << scope_tab << scope_tab << scope_tab << scope_tab << "Efl.EventCb callerCb = (IntPtr data, ref Efl.Event.NativeStruct evt) =>\n" << scope_tab << scope_tab << scope_tab << scope_tab << "{\n" @@ -460,7 +460,7 @@ struct event_definition_generator << scope_tab << scope_tab << "}\n\n" << scope_tab << scope_tab << "remove\n" << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << "lock (eventLock)\n" + << scope_tab << scope_tab << scope_tab << "lock (eflBindingEventLock)\n" << scope_tab << scope_tab << scope_tab << "{\n" << scope_tab << scope_tab << scope_tab << scope_tab << "string key = \"_" << upper_c_name << "\";\n" << scope_tab << scope_tab << scope_tab << scope_tab << "RemoveNativeEventHandler(" << library_name << ", key, value);\n" diff --git a/src/bin/eolian_mono/eolian/mono/function_definition.hh b/src/bin/eolian_mono/eolian/mono/function_definition.hh index f2dda1e7ca..8c43008c53 100644 --- a/src/bin/eolian_mono/eolian/mono/function_definition.hh +++ b/src/bin/eolian_mono/eolian/mono/function_definition.hh @@ -178,9 +178,10 @@ struct function_definition_generator std::string self = "this.NativeHandle"; - // inherited is set in the constructor, true if this instance is from a pure C# class (not generated). + // IsGeneratedBindingClass is set in the constructor, true if this + // instance is from a pure C# class (not generated). if (do_super && !f.is_static) - self = "(inherited ? Efl.Eo.Globals.efl_super(" + self + ", this.NativeClass) : " + self + ")"; + self = "(IsGeneratedBindingClass ? " + self + " : Efl.Eo.Globals.efl_super(" + self + ", this.NativeClass))"; else if (f.is_static) self = ""; diff --git a/src/bin/eolian_mono/eolian/mono/klass.hh b/src/bin/eolian_mono/eolian/mono/klass.hh index 8b018f6413..8f9932664b 100644 --- a/src/bin/eolian_mono/eolian/mono/klass.hh +++ b/src/bin/eolian_mono/eolian/mono/klass.hh @@ -214,7 +214,7 @@ struct klass << scope_tab << scope_tab << name_helpers::klass_get_name(cls) << "();\n" << scope_tab << "/// Initializes a new instance of the class.\n" << scope_tab << "/// Internal usage: This is used when interacting with C code and should not be used directly.\n" - << scope_tab << "private " << concrete_name << "(System.IntPtr raw) : base(raw)\n" + << scope_tab << "private " << concrete_name << "(Efl.Eo.Globals.WrappingHandle wh) : base(wh)\n" << scope_tab << "{\n" << scope_tab << "}\n\n" ) @@ -511,7 +511,7 @@ struct klass << scope_tab << "/// Initializes a new instance of the class.\n" << scope_tab << "/// Internal usage: Constructs an instance from a native pointer. This is used when interacting with C code and should not be used directly.\n" << scope_tab << "/// The native pointer to be wrapped.\n" - << scope_tab << "protected " << inherit_name << "(System.IntPtr raw) : base(raw)\n" + << scope_tab << "protected " << inherit_name << "(Efl.Eo.Globals.WrappingHandle wh) : base(wh)\n" << scope_tab << "{\n" << scope_tab << "}\n\n" ).generate(sink, std::make_tuple(constructors, constructors, constructors), context)) @@ -526,7 +526,7 @@ struct klass scope_tab << "[Efl.Eo.PrivateNativeClass]\n" << scope_tab << "private class " << inherit_name << "Realized : " << inherit_name << "\n" << scope_tab << "{\n" - << scope_tab << scope_tab << "private " << inherit_name << "Realized(IntPtr ptr) : base(ptr)\n" + << scope_tab << scope_tab << "private " << inherit_name << "Realized(Efl.Eo.Globals.WrappingHandle wh) : base(wh)\n" << scope_tab << scope_tab << "{\n" << scope_tab << scope_tab << "}\n" << scope_tab << "}\n" diff --git a/src/bindings/mono/eo_mono/EoWrapper.cs b/src/bindings/mono/eo_mono/EoWrapper.cs index 7e512acf56..5b14b48695 100644 --- a/src/bindings/mono/eo_mono/EoWrapper.cs +++ b/src/bindings/mono/eo_mono/EoWrapper.cs @@ -12,9 +12,9 @@ namespace Eo public abstract class EoWrapper : IWrapper, IDisposable { - protected readonly object eventLock = new object(); - protected bool inherited = false; - protected System.IntPtr handle = IntPtr.Zero; + protected readonly object eflBindingEventLock = new object(); + private bool generated = true; + private System.IntPtr handle = IntPtr.Zero; private static Efl.EventCb ownershipUniqueDelegate = new Efl.EventCb(OwnershipUniqueCallback); private static Efl.EventCb ownershipSharedDelegate = new Efl.EventCb(OwnershipSharedCallback); @@ -32,7 +32,7 @@ public abstract class EoWrapper : IWrapper, IDisposable /// Tag struct storing the native handle of the object being constructed. protected EoWrapper(ConstructingHandle ch) { - inherited = true; + generated = false; handle = Efl.Eo.Globals.efl_constructor(Efl.Eo.Globals.efl_super(ch.NativeHandle, Efl.Eo.Globals.efl_class_get(ch.NativeHandle))); if (handle == IntPtr.Zero) { @@ -47,11 +47,12 @@ public abstract class EoWrapper : IWrapper, IDisposable } /// Initializes a new instance of the class. - /// Internal usage: Constructs an instance from a native pointer. This is used when interacting with C code and should not be used directly. + /// Internal usage: Constructs an instance from a native pointer. This is used when interacting with C code and should not be used directly. + /// Do not implement this constructor. /// The native pointer to be wrapped. - protected EoWrapper(System.IntPtr raw) + protected EoWrapper(Efl.Eo.Globals.WrappingHandle wh) { - handle = raw; + handle = wh.NativeHandle; AddWrapperSupervisor(); } @@ -67,9 +68,9 @@ public abstract class EoWrapper : IWrapper, IDisposable [CallerFilePath] string file = null, [CallerLineNumber] int line = 0) { - inherited = ((object)this).GetType() != managedType; + generated = ((object)this).GetType() == managedType; IntPtr actual_klass = baseKlass; - if (inherited) + if (!generated) { actual_klass = Efl.Eo.ClassRegister.GetInheritKlassOrRegister(baseKlass, ((object)this).GetType()); } @@ -82,7 +83,7 @@ public abstract class EoWrapper : IWrapper, IDisposable parent_ptr = parent.NativeHandle; } - if (!inherited) + if (generated) { handle = Efl.Eo.Globals._efl_add_internal_start(file, line, actual_klass, parent_ptr, 1, 0); } @@ -123,6 +124,11 @@ public abstract class EoWrapper : IWrapper, IDisposable get; } + protected bool IsGeneratedBindingClass + { + get { return generated; } + } + /// Releases the underlying native instance. protected virtual void Dispose(bool disposing) { @@ -292,7 +298,7 @@ public abstract class EoWrapper : IWrapper, IDisposable NativeHandle = h; } - public IntPtr NativeHandle { get; set; } + public IntPtr NativeHandle { get; private set; } } public abstract class NativeMethods : Efl.Eo.NativeClass diff --git a/src/bindings/mono/eo_mono/iwrapper.cs b/src/bindings/mono/eo_mono/iwrapper.cs index caf28a700f..7c2eeb452b 100644 --- a/src/bindings/mono/eo_mono/iwrapper.cs +++ b/src/bindings/mono/eo_mono/iwrapper.cs @@ -624,14 +624,15 @@ public class Globals try { var flags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic; - constructor = managedType.GetConstructor(flags, null, new Type[1] { typeof(System.IntPtr) }, null); + constructor = managedType.GetConstructor(flags, null, new Type[1] { typeof(WrappingHandle) }, null); } catch (InvalidOperationException) { throw new InvalidOperationException($"Can't get constructor for type {managedType}"); } - var ret = (Efl.Eo.IWrapper) constructor.Invoke(new object[1] { handle }); + WrappingHandle wh = new WrappingHandle(handle); + var ret = (Efl.Eo.IWrapper) constructor.Invoke(new object[1] { wh }); if (ret == null) { @@ -715,6 +716,21 @@ public class Globals Monitor.Exit(Efl.All.InitLock); } + /// + /// Internal struct used by the binding to pass the native handle pointer + /// to the managed object wrapping constructor. + /// Internal usage only: do not use this class in inherited classes. + /// + public struct WrappingHandle + { + public WrappingHandle(IntPtr h) + { + NativeHandle = h; + } + + public IntPtr NativeHandle { get; private set; } + } + } // Globals public static class Config diff --git a/src/tests/efl_mono/EoConstruction.cs b/src/tests/efl_mono/EoConstruction.cs index 4708befc5b..bfcf40b0f1 100644 --- a/src/tests/efl_mono/EoConstruction.cs +++ b/src/tests/efl_mono/EoConstruction.cs @@ -27,9 +27,10 @@ class InheritedConstructibleObject : Dummy.ConstructibleObject public int DefaultConstrutorCallCount { get; set; } = 0; public int SpecialConstrutorCallCount { get; set; } = 0; - public bool InheritedFlag + /// Pointer to the native class description. + public bool IsInheritedClass { - get { return inherited; } + get { return !this.IsGeneratedBindingClass; } } public override int MultiplyIntegerValue(int v) @@ -56,7 +57,7 @@ class TestEoConstruction public static void TestInheritedEoDirectConstruction() { var obj = new InheritedConstructibleObject(); - Test.AssertEquals(obj.InheritedFlag, true); + Test.AssertEquals(obj.IsInheritedClass, true); Test.AssertEquals(obj.NativeConstructionCount, 1); Test.AssertEquals(obj.DefaultConstructionCount, 1); Test.AssertEquals(obj.SpecialConstructionCount, 0); @@ -75,7 +76,7 @@ class TestEoConstruction Test.AssertEquals(obj.MultiplyIntegerValue(21), 42); var obj2 = (InheritedConstructibleObject) obj.ConstructTypeAndStore(typeof(InheritedConstructibleObject)); - Test.AssertEquals(obj2.InheritedFlag, true); + Test.AssertEquals(obj2.IsInheritedClass, true); Test.AssertEquals(obj2.NativeConstructionCount, 1); Test.AssertEquals(obj2.DefaultConstructionCount, 0); Test.AssertEquals(obj2.SpecialConstructionCount, 1);