felipealmeida pushed a commit to branch master. http://git.enlightenment.org/core/efl.git/commit/?id=df0715a9aa27903dd549c2b4516ab2cba0274413
commit df0715a9aa27903dd549c2b4516ab2cba0274413 Author: Felipe Magno de Almeida <fel...@expertisesolutions.com.br> Date: Tue Feb 11 17:48:26 2020 +0000 C#: Add error checking for Eina.Success_Flag return type When a get and/or set from property is defined to return, explicitly, a Eina.Success_Flag, the mono generator will check the return value and generate an exception if the call fails. Resolves T8383. Reviewed-by: João Paulo Taylor Ienczak Zanette <joao....@expertisesolutions.com.br> Differential Revision: https://phab.enlightenment.org/D11281 --- .../eolian_mono/eolian/mono/function_definition.hh | 147 +++++++++++++-------- .../eolian_mono/eolian/mono/property_definition.hh | 7 +- src/lib/eina/eina_error.h | 9 ++ src/lib/eo/eina_types.eot | 4 + src/lib/eolian_cxx/grammar/eps.hpp | 8 +- src/tests/efl_mono/Eo.cs | 21 +++ src/tests/efl_mono/dummy_test_object.c | 58 ++++++++ src/tests/efl_mono/dummy_test_object.eo | 58 +++++++- 8 files changed, 250 insertions(+), 62 deletions(-) diff --git a/src/bin/eolian_mono/eolian/mono/function_definition.hh b/src/bin/eolian_mono/eolian/mono/function_definition.hh index 6aeaadcedb..5fc0e84fbf 100644 --- a/src/bin/eolian_mono/eolian/mono/function_definition.hh +++ b/src/bin/eolian_mono/eolian/mono/function_definition.hh @@ -39,6 +39,7 @@ #include "using_decl.hh" #include "generation_contexts.hh" #include "blacklist.hh" +#include "grammar/eps.hpp" namespace eolian_mono { @@ -369,6 +370,7 @@ struct property_wrapper_definition_generator { using efl::eolian::grammar::attribute_reorder; using efl::eolian::grammar::counter; + using efl::eolian::grammar::eps; using efl::eolian::grammar::attributes::parameter_direction; using efl::eolian::grammar::attributes::parameter_def; @@ -457,6 +459,7 @@ struct property_wrapper_definition_generator bool is_get_public = get_scope == "public "; std::string set_scope = property.setter.is_engaged() ? eolian_mono::function_scope_get(*property.setter) : ""; bool is_set_public = set_scope == "public "; + bool get_has_return_error = false, set_has_return_error = false; // No need to generate this wrapper as no accessor is public. if (is_interface && (!is_get_public && !is_set_public)) @@ -490,6 +493,12 @@ struct property_wrapper_definition_generator set_scope = ""; } + if (property.getter && property.getter->explicit_return_type.c_type == "Eina_Success_Flag") + get_has_return_error = true; + + if (property.setter && property.setter->explicit_return_type.c_type == "Eina_Success_Flag") + set_has_return_error = true; + if (parameters.size() == 1) { if (!as_generator( @@ -510,73 +519,95 @@ struct property_wrapper_definition_generator return false; } - if (property.getter.is_engaged() && is_interface) + if (property.getter) { - if (is_get_public) + auto managed_getter_name = name_helpers::managed_method_name(*property.getter); + if (is_interface) + { + if (is_get_public) + { + if (!as_generator(scope_tab(2) << scope_tab << set_scope << "get;\n" + ).generate(sink, attributes::unused, context)) + return false; + } + } + else if (get_params == 0) { - if (!as_generator(scope_tab(2) << scope_tab << set_scope << "get;\n" - ).generate(sink, attributes::unused, context)) + if (!as_generator + (scope_tab(2) << scope_tab << get_scope + << "get " << "{ return " + managed_getter_name + "(); }\n" + ).generate(sink, attributes::unused, context)) return false; } - } - else if (property.getter.is_engaged() && get_params == 0/*parameters.size() == 1 && property.getter.is_engaged()*/) - { - if (!as_generator - (scope_tab(2) << scope_tab << get_scope - << "get " << "{ return " + name_helpers::managed_method_name(*property.getter) + "(); }\n" - ).generate(sink, attributes::unused, context)) - return false; - } - else if (parameters.size() >= 1 && property.getter) - { - if (!as_generator - (scope_tab(2) << scope_tab << get_scope << "get " - << "{\n" - << *attribute_reorder<1, -1, 1> - (scope_tab(4) << type(true) << " _out_" - << argument(false) << " = default(" << type(true) << ");\n" - ) - << scope_tab(4) << name_helpers::managed_method_name(*property.getter) - << "(" << (("out _out_" << argument(false)) % ", ") << ");\n" - << scope_tab(4) << "return (" << (("_out_"<< argument(false)) % ", ") << ");\n" - << scope_tab(3) << "}" << "\n" - ).generate(sink, std::make_tuple(parameters, parameters, parameters), context)) - return false; - } - // else if (parameters.size() == 1) - // { - // if (!as_generator - // (scope_tab << scope_tab << get_scope << "get " - // << "{\n" - // << *attribute_reorder<1, -1, 1>(scope_tab(3) << type(true) << " _out_" << argument(false) << " = default(" << type(true) << ");\n") - // << scope_tab(3) << name_helpers::managed_method_name(*property.getter) - // << "(" << (("out _out_" << argument(false)) % ",") << ");\n" - // << scope_tab(3) << "return " << (("_out_"<< argument(false)) % ",") << ";\n" - // << scope_tab(2) << "}" << "\n" - // ).generate(sink, std::make_tuple(parameters, parameters, parameters), context)) - // return false; - // } - - if (property.setter.is_engaged() && is_interface) - { - if (is_set_public) + else if (parameters.size() >= 1) { - if (!as_generator(scope_tab(2) << scope_tab << set_scope << "set;\n" - ).generate(sink, attributes::unused, context)) + if (!as_generator + (scope_tab(2) << scope_tab << get_scope << "get " + << "{\n" + << *attribute_reorder<1, -1, 1> + (scope_tab(4) << type(true) << " _out_" + << argument(false) << " = default(" << type(true) << ");\n" + ) + << scope_tab(4) << (get_has_return_error ? "var s = " : "") + << name_helpers::managed_method_name(*property.getter) + << "(" << (("out _out_" << argument(false)) % ", ") << ");\n" + << ((eps(get_has_return_error) << scope_tab(4) + << "if (s == '\\0') throw new Efl.EflException(" + << "\"Call of native function for " << managed_getter_name << " returned an error.\"" + << ");\n") + | eps) + << scope_tab(4) << "return (" << (("_out_"<< argument(false)) % ", ") << ");\n" + << scope_tab(3) << "}" << "\n" + ).generate(sink, std::make_tuple(parameters, parameters, parameters), context)) return false; } } - else if (parameters.size() == 1 && property.setter.is_engaged()) - { - if (!as_generator(scope_tab(2) << scope_tab << set_scope << "set " << "{ " + name_helpers::managed_method_name(*property.setter) + "(" + dir_mod + "value); }\n" - ).generate(sink, attributes::unused, context)) - return false; - } - else if (parameters.size() > 1 && property.setter.is_engaged()) + + if (property.setter) { - if (!as_generator(scope_tab(2) << scope_tab << set_scope << "set " << ("{ " + name_helpers::managed_method_name(*property.setter) + "(" + dir_mod) << ((" value.Item" << counter(1)) % ", ") << "); }" << "\n" - ).generate(sink, parameters, context)) - return false; + auto managed_setter_name = name_helpers::managed_method_name(*property.setter); + if (is_interface) + { + if (is_set_public) + { + if (!as_generator(scope_tab(2) << scope_tab << set_scope << "set;\n" + ).generate(sink, attributes::unused, context)) + return false; + } + } + else if (parameters.size() == 1) + { + if (!as_generator(scope_tab(2) << scope_tab << set_scope << "set " + << "{ " + << ((eps (set_has_return_error) << "\n" << scope_tab(4) << "var s = ") | eps) + << managed_setter_name + << "(" << dir_mod << "value);" + << ((eps(set_has_return_error) << "\n" << scope_tab(4) + << "if (s == '\\0') throw new Efl.EflException(" + << "\"Call of native function for " << managed_setter_name << " returned an error.\"" + << ");\n" << scope_tab(3)) + | eps) + << " }\n" + ).generate(sink, attributes::unused, context)) + return false; + } + else if (parameters.size() > 1) + { + if (!as_generator(scope_tab(2) << scope_tab + << set_scope << "set " + << "{ " + << ((eps (set_has_return_error) << "\n" << scope_tab(4) << "var s = ") | eps) + << name_helpers::managed_method_name(*property.setter) + << "(" << dir_mod << ((" value.Item" << counter(1)) % ", ") << ");" + << ((eps(set_has_return_error) << "\n" << scope_tab(4) + << "if (s == '\\0') throw new Efl.EflException(" + << "\"Call of native function for " << managed_setter_name << " returned an error.\"" + << ");\n" << scope_tab(3)) + | eps) + << " }" << "\n" + ).generate(sink, parameters, context)) + return false; + } } if (!as_generator(scope_tab(2) << "}\n\n").generate(sink, attributes::unused, context)) diff --git a/src/bin/eolian_mono/eolian/mono/property_definition.hh b/src/bin/eolian_mono/eolian/mono/property_definition.hh index df94fc36e8..703b70269d 100644 --- a/src/bin/eolian_mono/eolian/mono/property_definition.hh +++ b/src/bin/eolian_mono/eolian/mono/property_definition.hh @@ -115,11 +115,14 @@ bool property_generate_wrapper_getter(attributes::property_def const& property, if (!property.getter->keys.empty()) return false; - if (property.getter->explicit_return_type != attributes::void_) + if (property.getter->explicit_return_type != attributes::void_ + && property.getter->explicit_return_type.c_type != "Eina_Success_Flag") + { return false; + } assert (!!property.getter.is_engaged()); - + bool is_interface = context_find_tag<class_context>(context).current_wrapper_kind == class_context::interface; if (is_interface) { diff --git a/src/lib/eina/eina_error.h b/src/lib/eina/eina_error.h index c21cb7bc7e..e2dfd122ae 100644 --- a/src/lib/eina/eina_error.h +++ b/src/lib/eina/eina_error.h @@ -88,6 +88,15 @@ */ typedef int Eina_Error; +/** + * @typedef Eina_Success_Flag + * @brief A flag indicating a function completed succesfully. + * + * Errors are reported with a EINA_FALSE value for Eina_Success_Flag + * return and success with a EINA_TRUE. + */ +typedef Eina_Bool Eina_Success_Flag; + /** * @def EINA_ERROR_NO_ERROR * @brief No error reported. diff --git a/src/lib/eo/eina_types.eot b/src/lib/eo/eina_types.eot index 04d728a8d3..bff83968c1 100644 --- a/src/lib/eo/eina_types.eot +++ b/src/lib/eo/eina_types.eot @@ -163,3 +163,7 @@ struct @extern @free(eina_promise_free) Eina.Promise; [[Eina promise type /* FIXME: This definitely shouldn't be here. */ type @beta Efl.Event_Cb: __undefined_type; [[Efl event callback type]] +type @extern Eina.Success_Flag: char; [[Eina error type + + @since 1.24 +]] diff --git a/src/lib/eolian_cxx/grammar/eps.hpp b/src/lib/eolian_cxx/grammar/eps.hpp index 6416bc918f..354c8d7cdb 100644 --- a/src/lib/eolian_cxx/grammar/eps.hpp +++ b/src/lib/eolian_cxx/grammar/eps.hpp @@ -22,11 +22,17 @@ namespace efl { namespace eolian { namespace grammar { struct eps_generator { + eps_generator () : r(true) {} + eps_generator (bool r) : r(r) {} + bool r; + template <typename OutputIterator, typename Attribute, typename Context> bool generate(OutputIterator, Attribute const&, Context const&) const { - return true; + return r; } + + eps_generator operator()(bool r) const { return {r}; } }; template <> diff --git a/src/tests/efl_mono/Eo.cs b/src/tests/efl_mono/Eo.cs index 8c0c9ab994..641ddc0748 100644 --- a/src/tests/efl_mono/Eo.cs +++ b/src/tests/efl_mono/Eo.cs @@ -468,6 +468,27 @@ class TestCsharpProperties Test.AssertEquals(ret, (1, 2)); obj.Dispose(); } + + public static void test_csharp_return_type_get_prop () + { + var obj = new Dummy.TestObject(); + obj.ReturnTypeGetProp = 5; + var i = obj.ReturnTypeGetProp; + } + + public static void test_csharp_return_type_set_prop () + { + var obj = new Dummy.TestObject(); + obj.ReturnTypeSetProp = 5; + var i = obj.ReturnTypeSetProp; + } + + public static void test_csharp_return_type_prop () + { + var obj = new Dummy.TestObject(); + obj.ReturnTypeProp = 5; + var i = obj.ReturnTypeProp; + } } class TestEoGrandChildrenFinalize diff --git a/src/tests/efl_mono/dummy_test_object.c b/src/tests/efl_mono/dummy_test_object.c index 687737a4aa..210f2ca51f 100644 --- a/src/tests/efl_mono/dummy_test_object.c +++ b/src/tests/efl_mono/dummy_test_object.c @@ -4819,5 +4819,63 @@ const char* _dummy_inherit_helper_receive_dummy_and_call_in_stringshare(Dummy_Te return dummy_inherit_iface_stringshare_test (x, eina_stringshare_add("hello world")); } +Eina_Success_Flag _dummy_test_object_return_type_get_prop_get(EINA_UNUSED const Eo *obj, Dummy_Test_Object_Data *pd EINA_UNUSED, int* i EINA_UNUSED) +{ + return EINA_TRUE; +} + +void _dummy_test_object_return_type_get_prop_set(EINA_UNUSED Eo *obj, Dummy_Test_Object_Data *pd EINA_UNUSED, int i EINA_UNUSED) +{ +} + +int _dummy_test_object_return_type_set_prop_get(EINA_UNUSED const Eo *obj, Dummy_Test_Object_Data *pd EINA_UNUSED) +{ + return 0; +} + +Eina_Success_Flag _dummy_test_object_return_type_set_prop_set(EINA_UNUSED Eo *obj, Dummy_Test_Object_Data *pd EINA_UNUSED, int i EINA_UNUSED) +{ + return EINA_TRUE; +} + +Eina_Success_Flag _dummy_test_object_return_type_prop_get(EINA_UNUSED const Eo *obj, Dummy_Test_Object_Data *pd EINA_UNUSED, int* i EINA_UNUSED) +{ + return EINA_TRUE; +} + +Eina_Success_Flag _dummy_test_object_return_type_prop_set(EINA_UNUSED Eo *obj, Dummy_Test_Object_Data *pd EINA_UNUSED, int i EINA_UNUSED) +{ + return EINA_TRUE; +} + +Eina_Success_Flag _dummy_test_object_multi_value_return_type_get_prop_get(EINA_UNUSED const Eo *obj, Dummy_Test_Object_Data *pd EINA_UNUSED, int* i EINA_UNUSED, int* j EINA_UNUSED) +{ + return EINA_TRUE; +} + +void _dummy_test_object_multi_value_return_type_get_prop_set(EINA_UNUSED Eo *obj, Dummy_Test_Object_Data *pd EINA_UNUSED, int i EINA_UNUSED, int j EINA_UNUSED) +{ +} + +void _dummy_test_object_multi_value_return_type_set_prop_get(EINA_UNUSED const Eo *obj, Dummy_Test_Object_Data *pd EINA_UNUSED, int* i EINA_UNUSED, int* j EINA_UNUSED) +{ +} + +Eina_Success_Flag _dummy_test_object_multi_value_return_type_set_prop_set(EINA_UNUSED Eo *obj, Dummy_Test_Object_Data *pd EINA_UNUSED, int i EINA_UNUSED, int j EINA_UNUSED) +{ + return EINA_TRUE; +} + +Eina_Success_Flag _dummy_test_object_multi_value_return_type_prop_get(EINA_UNUSED const Eo *obj, Dummy_Test_Object_Data *pd EINA_UNUSED, int* i EINA_UNUSED, int* j EINA_UNUSED) +{ + return EINA_TRUE; +} + +Eina_Success_Flag _dummy_test_object_multi_value_return_type_prop_set(EINA_UNUSED Eo *obj, Dummy_Test_Object_Data *pd EINA_UNUSED, int i EINA_UNUSED, int j EINA_UNUSED) +{ + return EINA_TRUE; +} + + #include "dummy_test_object.eo.c" diff --git a/src/tests/efl_mono/dummy_test_object.eo b/src/tests/efl_mono/dummy_test_object.eo index fa3b52cace..85740f8b06 100644 --- a/src/tests/efl_mono/dummy_test_object.eo +++ b/src/tests/efl_mono/dummy_test_object.eo @@ -1686,7 +1686,63 @@ class Dummy.Test_Object extends Efl.Object implements Dummy.Test_Iface { prop: Dummy.Test_Iface; } } - } + + @property return_type_get_prop { + get { return: Eina.Success_Flag; } + set {} + values { i: int; } + } + + @property return_type_set_prop { + set { return: Eina.Success_Flag; } + get {} + values { i: int; } + } + + @property return_type_prop { + get { + return: Eina.Success_Flag; + } + set { + return: Eina.Success_Flag; + } + values { i: int; } + } + + @property multi_value_return_type_get_prop { + get { return: Eina.Success_Flag; } + set {} + values + { + i: int; + j: int; + } + } + + @property multi_value_return_type_set_prop { + set { return: Eina.Success_Flag; } + get {} + values + { + i: int; + j: int; + } + } + + @property multi_value_return_type_prop { + get { + return: Eina.Success_Flag; + } + set { + return: Eina.Success_Flag; + } + values + { + i: int; + j: int; + } + } +} implements { Efl.Object.constructor; Efl.Object.destructor; --