From 8e951504f584f124ba88471fc46f1e7b6d2d3639 Mon Sep 17 00:00:00 2001 From: Yeongjong Lee Date: Tue, 17 Dec 2019 11:34:41 -0300 Subject: [PATCH] csharp : add move tag info to EinaAccessor, EinaIterator converter Summary: Included commits in devs/lauromoura/remove_eina_mono-rebased ``` commit ed6679db1901c710cc6ddb50e7001cfd20caa77a Author: Lauro Moura Date: Mon Dec 2 13:58:04 2019 -0300 csharp: add move information to EnumerableToAccessor Still need to fix the converted accessor ownership, maybe by creating a custom accessor class that released the pinned memory when is freed. ``` ref T8486 Depends On D10878 Co-authored-by: Lauro Moura Test Plan: meson build -Dbindings=mono,cxx -Dmono-beta=true Reviewers: YOhoho Reviewed By: YOhoho Subscribers: cedric, #reviewers, #committers Tags: #efl Maniphest Tasks: T8486 Differential Revision: https://phab.enlightenment.org/D10879 --- src/bin/eolian_mono/eolian/mono/parameter.hh | 29 ++------- .../eolian/mono/struct_definition.hh | 2 +- src/bindings/mono/eo_mono/iwrapper.cs | 59 +++++++++++++++---- src/tests/efl_mono/Eina.cs | 11 ++-- src/tests/efl_mono/Eo.cs | 40 ++++++++++--- 5 files changed, 90 insertions(+), 51 deletions(-) diff --git a/src/bin/eolian_mono/eolian/mono/parameter.hh b/src/bin/eolian_mono/eolian/mono/parameter.hh index a5a70f2bb0..210e5f22d5 100644 --- a/src/bin/eolian_mono/eolian/mono/parameter.hh +++ b/src/bin/eolian_mono/eolian/mono/parameter.hh @@ -759,7 +759,7 @@ struct convert_in_variable_generator return false; auto var_name = in_variable_name(param.param_name); if (!as_generator( - "var " << string << " = " << "Efl.Eo.Globals.IEnumerableToIterator(" << escape_keyword(param.param_name) << ");\n" + "var " << string << " = Efl.Eo.Globals.IEnumerableToIterator(" << escape_keyword(param.param_name) << ", " << (param.type.has_own ? "true" : "false")<< ");\n" ).generate(sink, var_name, context)) return false; } @@ -770,7 +770,7 @@ struct convert_in_variable_generator return false; auto var_name = in_variable_name(param.param_name); if (!as_generator( - "var " << string << " = " << "Efl.Eo.Globals.IEnumerableToAccessor(" << escape_keyword(param.param_name) << ");\n" + "var " << string << " = Efl.Eo.Globals.IEnumerableToAccessor(" << escape_keyword(param.param_name) << ", " << (param.type.has_own ? "true" : "false")<< ");\n" ).generate(sink, var_name, context)) return false; } @@ -1291,12 +1291,6 @@ struct native_convert_out_assign_generator ).generate(sink, outvar, context)) return false; - // Iterators and Accessors can't own their content. - if (param.type.c_type == "Eina_Iterator *" || param.type.c_type == "const Eina_Iterator *" - || param.type.c_type == "Eina_Accessor *" || param.type.c_type == "const Eina_Accessor *" - ) - return true; - if ((param.type.has_own && (complex->subtypes.front().is_value_type && complex->subtypes.front().has_own)) && !as_generator( string << ".OwnContent = false;\n" @@ -1314,7 +1308,7 @@ struct native_convert_out_assign_generator return false; auto outvar = out_variable_name(param.param_name); if (!as_generator( - string << " = Efl.Eo.Globals.IEnumerableToAccessor(" << string << ");\n" + string << " = Efl.Eo.Globals.IEnumerableToAccessor(" << string << ", " << (param.type.has_own ? "true" : "false")<< ");\n" ).generate(sink, std::make_tuple(escape_keyword(param.param_name), outvar), context)) return false; } @@ -1329,7 +1323,7 @@ struct native_convert_out_assign_generator return false; auto outvar = out_variable_name(param.param_name); if (!as_generator( - string << " = Efl.Eo.Globals.IEnumerableToIterator(" << string << ");\n" + string << " = Efl.Eo.Globals.IEnumerableToIterator(" << string << ", " << (param.type.has_own ? "true" : "false")<< ");\n" ).generate(sink, std::make_tuple(escape_keyword(param.param_name), outvar), context)) return false; } @@ -1453,28 +1447,17 @@ struct native_convert_return_generator .generate(sink, attributes::unused, context)) return false; - // Iterators and Accessors can't own their content. - if (ret_type.c_type != "Eina_Iterator *" && ret_type.c_type != "const Eina_Iterator *" - && ret_type.c_type != "Eina_Accessor *" && ret_type.c_type != "const Eina_Accessor *" - ) - { - if ((ret_type.has_own && (complex->subtypes.front().is_value_type || complex->subtypes.front().has_own)) - && !as_generator("_ret_var.OwnContent = false; ") - .generate(sink, attributes::unused, context)) - return false; - } - return as_generator("return _ret_var.Handle;") .generate(sink, attributes::unused, context); } else if (ret_type.c_type == "Eina_Accessor *" || ret_type.c_type == "const Eina_Accessor *") { - return as_generator("return Efl.Eo.Globals.IEnumerableToAccessor(_ret_var);") + return as_generator(lit("return Efl.Eo.Globals.IEnumerableToAccessor(_ret_var, ") << (ret_type.has_own ? "true" : "false") << ");") .generate(sink, attributes::unused, context); } else if (ret_type.c_type == "Eina_Iterator *" || ret_type.c_type == "const Eina_Iterator *") { - return as_generator("return Efl.Eo.Globals.IEnumerableToIterator(_ret_var);") + return as_generator(lit("return Efl.Eo.Globals.IEnumerableToIterator(_ret_var, ") << (ret_type.has_own ? "true" : "false") << ");") .generate(sink, attributes::unused, context); } else if (ret_type.c_type != "void") diff --git a/src/bin/eolian_mono/eolian/mono/struct_definition.hh b/src/bin/eolian_mono/eolian/mono/struct_definition.hh index 1267a8d38c..7a3019ec45 100644 --- a/src/bin/eolian_mono/eolian/mono/struct_definition.hh +++ b/src/bin/eolian_mono/eolian/mono/struct_definition.hh @@ -72,7 +72,7 @@ struct to_internal_field_convert_generator else if ((complex && (complex->outer.base_type == "iterator"))) { if (!as_generator( - indent << scope_tab << scope_tab << "_internal_struct." << string << " = Efl.Eo.Globals.IEnumerableToIterator(_external_struct." << string << ");\n") + indent << scope_tab << scope_tab << "_internal_struct." << string << " = Efl.Eo.Globals.IEnumerableToIterator(_external_struct." << string << ", " << (field.type.has_own ? "true" : "false") << ");\n") .generate(sink, std::make_tuple(field_name, field_name), context)) return false; } diff --git a/src/bindings/mono/eo_mono/iwrapper.cs b/src/bindings/mono/eo_mono/iwrapper.cs index 32ed323113..8ba8c964bd 100644 --- a/src/bindings/mono/eo_mono/iwrapper.cs +++ b/src/bindings/mono/eo_mono/iwrapper.cs @@ -774,21 +774,37 @@ public static class Globals } } - internal static IntPtr IEnumerableToAccessor(IEnumerable enumerable) + internal static IntPtr IEnumerableToAccessor(IEnumerable enumerable, bool isMoved) { if (enumerable == null) + { throw new ArgumentException("enumerable is null", nameof(enumerable)); - IntPtr[] intPtrs = new IntPtr[enumerable.Count()]; + } - int i = 0; + // If we are a wrapper around an existing Eina.Accessor, we can just forward + // it and avoid unnecessary copying in non-owning transfers. + var wrappedAccessor = enumerable as Eina.Accessor; + + if (wrappedAccessor != null && !isMoved) + { + return wrappedAccessor.Handle; + } + + var list = new List(); foreach (T data in enumerable) { - intPtrs[i] = Eina.TraitFunctions.ManagedToNativeAlloc(data); - i++; + list.Add(Eina.TraitFunctions.ManagedToNativeAlloc(data)); } - IntPtr[] dataArray = intPtrs.ToArray(); + IntPtr[] dataArray = list.ToArray(); GCHandle pinnedArray = GCHandle.Alloc(dataArray, GCHandleType.Pinned); //FIXME: Need to free. - return Eina.AccessorNativeFunctions.eina_carray_length_accessor_new(pinnedArray.AddrOfPinnedObject(), (uint)(IntPtr.Size), (uint)dataArray.Length); + IntPtr ret = Eina.AccessorNativeFunctions.eina_carray_length_accessor_new(pinnedArray.AddrOfPinnedObject(), (uint)(IntPtr.Size), (uint)dataArray.Length); + + if (!isMoved) + { + // FIXME Need to free ret and unpin pinnedArray in the future. + } + + return ret; } internal static IEnumerable IteratorToIEnumerable(IntPtr iterator) @@ -802,22 +818,38 @@ public static class Globals } } - internal static IntPtr IEnumerableToIterator(IEnumerable enumerable) + internal static IntPtr IEnumerableToIterator(IEnumerable enumerable, bool isMoved) { if (enumerable == null) + { throw new ArgumentException("enumerable is null", nameof(enumerable)); + } + + // If we are a wrapper around an existing Eina.Iterator, we can just forward + // it and avoid unnecessary copying in non-owning transfers. + var wrappedIterator = enumerable as Eina.Iterator; + + if (wrappedIterator != null && !isMoved) + { + return wrappedIterator.Handle; + } var list = new List(); - //IntPtr[] intPtrs = new IntPtr[enumerable.Count()]; - foreach (T data in enumerable) { list.Add(Eina.TraitFunctions.ManagedToNativeAlloc(data)); } IntPtr[] dataArray = list.ToArray(); - GCHandle pinnedArray = GCHandle.Alloc(dataArray, GCHandleType.Pinned); //FIXME: Need to free. - return Eina.IteratorNativeFunctions.eina_carray_length_iterator_new(pinnedArray.AddrOfPinnedObject(), (uint)(IntPtr.Size), (uint)dataArray.Length); + GCHandle pinnedArray = GCHandle.Alloc(dataArray, GCHandleType.Pinned); + IntPtr ret = Eina.IteratorNativeFunctions.eina_carray_length_iterator_new(pinnedArray.AddrOfPinnedObject(), (uint)(IntPtr.Size), (uint)dataArray.Length); + + if (!isMoved) + { + // FIXME Need to free ret and unpin pinnedArray in the future. + } + + return ret; } internal static IEnumerable ListToIEnumerable(IntPtr list) @@ -841,8 +873,9 @@ public static class Globals IntPtr list = IntPtr.Zero; foreach (T data in enumerable) { - list = Eina.ListNativeFunctions.eina_list_append(list, Eina.TraitFunctions.ManagedToNativeAlloc(data)); //FIXME: need to free + list = Eina.ListNativeFunctions.eina_list_append(list, Eina.TraitFunctions.ManagedToNativeAlloc(data)); } + // FIXME need to free `list` if the returned list is not @moved return list; } diff --git a/src/tests/efl_mono/Eina.cs b/src/tests/efl_mono/Eina.cs index e3c9159a33..46d89e4c55 100644 --- a/src/tests/efl_mono/Eina.cs +++ b/src/tests/efl_mono/Eina.cs @@ -4100,10 +4100,10 @@ class TestEinaIterator Test.Assert(arr.Own); Test.Assert(arr.OwnContent); - // Will take ownership of the Iterator + // Will copy the Iterator, owning the copy. Test.Assert(t.EinaIteratorIntInOwn(itr)); - Test.Assert(!itr.Own); + Test.Assert(itr.Own); Test.Assert(arr.Own); // Content must continue to be owned by the array Test.Assert(arr.OwnContent); @@ -4224,7 +4224,7 @@ class TestEinaIterator Test.Assert(t.EinaIteratorStrInOwn(itr)); - Test.Assert(!itr.Own); + Test.Assert(itr.Own); Test.Assert(arr.Own); Test.Assert(arr.OwnContent); @@ -4344,7 +4344,8 @@ class TestEinaIterator Test.Assert(t.EinaIteratorStrshareInOwn(itr)); - Test.Assert(!itr.Own); + // Moving collections currently copy them, should not reflect on managed objects. + Test.Assert(itr.Own); Test.Assert(arr.Own); Test.Assert(arr.OwnContent); @@ -4464,7 +4465,7 @@ class TestEinaIterator Test.Assert(t.EinaIteratorObjInOwn(itr)); - Test.Assert(!itr.Own); + Test.Assert(itr.Own); Test.Assert(arr.Own); Test.Assert(arr.OwnContent); diff --git a/src/tests/efl_mono/Eo.cs b/src/tests/efl_mono/Eo.cs index e946f6cf37..586a18cba3 100644 --- a/src/tests/efl_mono/Eo.cs +++ b/src/tests/efl_mono/Eo.cs @@ -257,25 +257,47 @@ class TestVariables class TestEoAccessors { - public static void basic_eo_accessors() + private static void do_eo_accessors(IEnumerable accessor) { var obj = new Dummy.TestObject(); - Eina.List lst = new Eina.List(); - lst.Append(4); - lst.Append(3); - lst.Append(2); - lst.Append(5); - IEnumerable acc = obj.CloneAccessor(lst.GetAccessor()); - var zipped = acc.Zip(lst, (first, second) => new Tuple(first, second)); + IEnumerable acc = obj.CloneAccessor(accessor); + + var zipped = acc.Zip(accessor, (first, second) => new Tuple(first, second)); foreach (Tuple pair in zipped) { Test.AssertEquals(pair.Item1, pair.Item2); } - lst.Dispose(); obj.Dispose(); } + + public static void eina_eo_accessors() + { + Eina.List lst = new Eina.List(); + lst.Append(4); + lst.Append(3); + lst.Append(2); + lst.Append(5); + + // FIXME: Replace the first accessor with the list once Eina.List implements Eina.IList + do_eo_accessors(lst.GetAccessor()); + + lst.Dispose(); + } + + public static void managed_eo_accessors() + { + var obj = new Dummy.TestObject(); + + List lst = new List(); + lst.Add(-1); + lst.Add(1); + lst.Add(4); + lst.Add(42); + + do_eo_accessors(lst); + } } class TestEoFinalize