From c1b76d3008ecb7c558ad4ad8de02f1e03ffddf68 Mon Sep 17 00:00:00 2001 From: Vitor Sousa Date: Mon, 5 Aug 2019 19:06:11 -0300 Subject: [PATCH] csharp: fix ownership of value types in arrays and lists Summary: `eolian_mono` now considers the implicit ownership of value types in arrays and lists when generating ownership flags. Also, update manual bindings for arrays and lists to no longer free elements in the `Dispose` method when the container has ownership of the elements but C# itself does not have ownership of the container; the elements will be freed by whoever owns the container. Modifying and removing elements will still free them though. Re-enabled unit tests that required ownership of value type elements. Reviewers: felipealmeida, q66, vitor.sousa Reviewed By: felipealmeida Subscribers: cedric, #reviewers, #committers Tags: #efl Differential Revision: https://phab.enlightenment.org/D9457 --- src/bin/eolian_mono/eolian/mono/parameter.hh | 15 +++++++++------ src/bindings/mono/eina_mono/eina_array.cs | 2 +- src/bindings/mono/eina_mono/eina_list.cs | 2 +- src/lib/eolian_cxx/grammar/klass_def.hpp | 2 ++ src/tests/efl_mono/Eina.cs | 12 ------------ src/tests/efl_mono/dummy_test_object.eo | 12 ------------ 6 files changed, 13 insertions(+), 32 deletions(-) diff --git a/src/bin/eolian_mono/eolian/mono/parameter.hh b/src/bin/eolian_mono/eolian/mono/parameter.hh index e1e8661bcb..954ac25c70 100644 --- a/src/bin/eolian_mono/eolian/mono/parameter.hh +++ b/src/bin/eolian_mono/eolian/mono/parameter.hh @@ -612,7 +612,7 @@ struct native_convert_in_variable_generator return as_generator( "var " << string << " = new " << type << "(" << escape_keyword(param.param_name) << ", " << (param.type.has_own ? "true" : "false") - << ", " << (complex->subtypes.front().has_own ? "true" : "false") + << ", " << (complex->subtypes.front().is_value_type || complex->subtypes.front().has_own ? "true" : "false") << ");\n" ).generate(sink, std::make_tuple(in_variable_name(param.param_name), param.type), context); } @@ -724,7 +724,8 @@ struct convert_in_variable_generator ) return true; - if (complex->subtypes.front().has_own && !as_generator( + if (!complex->subtypes.front().is_value_type && complex->subtypes.front().has_own + && !as_generator( escape_keyword(param.param_name) << ".OwnContent = false;\n" ).generate(sink, attributes::unused, context)) return false; @@ -963,7 +964,7 @@ struct convert_out_assign_generator return as_generator( string << " = new " << type << "(" << string << ", " << (param.type.has_own ? "true" : "false") - << ", " << (complex->subtypes.front().has_own ? "true" : "false") + << ", " << (complex->subtypes.front().is_value_type || complex->subtypes.front().has_own ? "true" : "false") << ");\n" ).generate(sink, std::make_tuple(escape_keyword(param.param_name), param.type, out_variable_name(param.param_name)), context); } @@ -1092,7 +1093,7 @@ struct convert_return_generator if (!complex) return false; if (!as_generator("return new " << type << "(_ret_var, " << std::string{ret_type.has_own ? "true" : "false"} - << ", " << (complex->subtypes.front().has_own ? "true" : "false") + << ", " << (complex->subtypes.front().is_value_type || complex->subtypes.front().has_own ? "true" : "false") << ");\n") .generate(sink, ret_type, context)) return false; @@ -1244,7 +1245,8 @@ struct native_convert_out_assign_generator ) return true; - if (complex->subtypes.front().has_own && !as_generator( + if (!complex->subtypes.front().is_value_type && complex->subtypes.front().has_own + && !as_generator( string << ".OwnContent = false;\n" ).generate(sink, outvar, context)) return false; @@ -1370,7 +1372,8 @@ struct native_convert_return_generator && ret_type.c_type != "Eina_Accessor *" && ret_type.c_type != "const Eina_Accessor *" ) { - if (complex->subtypes.front().has_own && !as_generator("_ret_var.OwnContent = false; ") + if (!complex->subtypes.front().is_value_type && complex->subtypes.front().has_own + && !as_generator("_ret_var.OwnContent = false; ") .generate(sink, attributes::unused, context)) return false; } diff --git a/src/bindings/mono/eina_mono/eina_array.cs b/src/bindings/mono/eina_mono/eina_array.cs index e75ca6ae7a..8c09557098 100644 --- a/src/bindings/mono/eina_mono/eina_array.cs +++ b/src/bindings/mono/eina_mono/eina_array.cs @@ -138,7 +138,7 @@ public class Array : IEnumerable, IDisposable return; } - if (OwnContent) + if (Own && OwnContent) { int len = (int)eina_array_count_custom_export_mono(h); for (int i = 0; i < len; ++i) diff --git a/src/bindings/mono/eina_mono/eina_list.cs b/src/bindings/mono/eina_mono/eina_list.cs index 4c25c25e62..9fe5e90d77 100644 --- a/src/bindings/mono/eina_mono/eina_list.cs +++ b/src/bindings/mono/eina_mono/eina_list.cs @@ -182,7 +182,7 @@ public class List : IEnumerable, IDisposable return; } - if (OwnContent) + if (Own && OwnContent) { for (IntPtr curr = h; curr != IntPtr.Zero; curr = InternalNext(curr)) { diff --git a/src/lib/eolian_cxx/grammar/klass_def.hpp b/src/lib/eolian_cxx/grammar/klass_def.hpp index 9157cfd5b1..4d6d4ea79f 100644 --- a/src/lib/eolian_cxx/grammar/klass_def.hpp +++ b/src/lib/eolian_cxx/grammar/klass_def.hpp @@ -376,6 +376,7 @@ struct type_def bool is_ptr; bool is_beta; std::string doc_summary; + bool is_value_type; type_def() = default; type_def(variant_type original_type, std::string c_type, bool has_own, bool is_ptr, bool is_beta, std::string doc_summary) @@ -428,6 +429,7 @@ type_def const void_ {attributes::regular_type_def{"void", {qualifier_info::is_n inline void type_def::set(Eolian_Type const* eolian_type, Eolian_Unit const* unit, Eolian_C_Type_Type ctype) { c_type = ::eolian_type_c_type_get(eolian_type, ctype); + is_value_type = ('*' != c_type.back()); // ::eina_stringshare_del(stringshare); // this crashes Eolian_Type const* stp = eolian_type_base_type_get(eolian_type); has_own = !!::eolian_type_is_owned(eolian_type); diff --git a/src/tests/efl_mono/Eina.cs b/src/tests/efl_mono/Eina.cs index 07dfd1bed2..0f6d9774bc 100644 --- a/src/tests/efl_mono/Eina.cs +++ b/src/tests/efl_mono/Eina.cs @@ -838,7 +838,6 @@ class TestEinaArray Test.Assert(arr.Handle == IntPtr.Zero); } - /* public static void test_eina_array_int_in_own() { var t = new Dummy.TestObject(); @@ -851,7 +850,6 @@ class TestEinaArray Test.Assert(arr.Handle == IntPtr.Zero); Test.Assert(t.CheckEinaArrayIntInOwn()); } - */ public static void test_eina_array_int_out() { @@ -866,7 +864,6 @@ class TestEinaArray Test.Assert(t.CheckEinaArrayIntOut()); } - /* public static void test_eina_array_int_out_own() { var t = new Dummy.TestObject(); @@ -878,7 +875,6 @@ class TestEinaArray arr.Dispose(); Test.Assert(arr.Handle == IntPtr.Zero); } - */ public static void test_eina_array_int_return() { @@ -892,7 +888,6 @@ class TestEinaArray Test.Assert(t.CheckEinaArrayIntReturn()); } - /* public static void test_eina_array_int_return_own() { var t = new Dummy.TestObject(); @@ -903,7 +898,6 @@ class TestEinaArray arr.Dispose(); Test.Assert(arr.Handle == IntPtr.Zero); } - */ // String // public static void test_eina_array_str_in() @@ -1902,7 +1896,6 @@ class TestEinaList Test.Assert(lst.Handle == IntPtr.Zero); } - /* public static void test_eina_list_int_in_own() { var t = new Dummy.TestObject(); @@ -1914,7 +1907,6 @@ class TestEinaList Test.Assert(lst.Handle == IntPtr.Zero); Test.Assert(t.CheckEinaListIntInOwn()); } - */ public static void test_eina_list_int_out() { @@ -1928,7 +1920,6 @@ class TestEinaList Test.Assert(t.CheckEinaListIntOut()); } - /* public static void test_eina_list_int_out_own() { var t = new Dummy.TestObject(); @@ -1940,7 +1931,6 @@ class TestEinaList lst.Dispose(); Test.Assert(lst.Handle == IntPtr.Zero); } - */ public static void test_eina_list_int_return() { @@ -1953,7 +1943,6 @@ class TestEinaList Test.Assert(t.CheckEinaListIntReturn()); } - /* public static void test_eina_list_int_return_own() { var t = new Dummy.TestObject(); @@ -1964,7 +1953,6 @@ class TestEinaList lst.Dispose(); Test.Assert(lst.Handle == IntPtr.Zero); } - */ // String // public static void test_eina_list_str_in() diff --git a/src/tests/efl_mono/dummy_test_object.eo b/src/tests/efl_mono/dummy_test_object.eo index 47ec4c6b27..e25f7b29bb 100644 --- a/src/tests/efl_mono/dummy_test_object.eo +++ b/src/tests/efl_mono/dummy_test_object.eo @@ -385,7 +385,6 @@ class Dummy.Test_Object extends Efl.Object implements Dummy.Test_Iface { return: bool; } - /* eina_array_int_in_own { params { @in arr: array @owned; // @@ -395,7 +394,6 @@ class Dummy.Test_Object extends Efl.Object implements Dummy.Test_Iface { check_eina_array_int_in_own { return: bool; } - */ eina_array_int_out { params { @@ -407,14 +405,12 @@ class Dummy.Test_Object extends Efl.Object implements Dummy.Test_Iface { return: bool; } - /* eina_array_int_out_own { params { @out arr: array @owned; // } return: bool; } - */ eina_array_int_return { return: array; @@ -423,11 +419,9 @@ class Dummy.Test_Object extends Efl.Object implements Dummy.Test_Iface { return: bool; } - /* eina_array_int_return_own { return: array @owned; // } - */ /* String */ eina_array_str_in { @@ -584,7 +578,6 @@ class Dummy.Test_Object extends Efl.Object implements Dummy.Test_Iface { return: bool; } - /* eina_list_int_in_own { params { @in lst: list @owned; // @@ -594,7 +587,6 @@ class Dummy.Test_Object extends Efl.Object implements Dummy.Test_Iface { check_eina_list_int_in_own { return: bool; } - */ eina_list_int_out { params { @@ -606,14 +598,12 @@ class Dummy.Test_Object extends Efl.Object implements Dummy.Test_Iface { return: bool; } - /* eina_list_int_out_own { params { @out lst: list @owned; // } return: bool; } - */ eina_list_int_return { return: list; @@ -622,11 +612,9 @@ class Dummy.Test_Object extends Efl.Object implements Dummy.Test_Iface { return: bool; } - /* eina_list_int_return_own { return: list @owned; // } - */ /* String */ eina_list_str_in {