From fd3e76365692fe2ec070681d3a59cfef0d77d4ec Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 6 Aug 2019 02:35:25 +0200 Subject: [PATCH] Streamline and fix returning objects by value Remove the code related to "_result_ref" which was confusing and plain wrong, as it generated something that compiled but crashed during run-time due to the use of a pointer to an already destroyed stack object. Instead, correct the "out" typemap to create a new copy of the object, which mostly works fine on its own, except that it depends on using SwigValueWrapper if necessary, so add the call to cplus_value_type() does this. This also required removing the code resolving typedefs in the "type" attribute because it confused the base class logic and still needs an explicit cast to the actual return type due to the use of (and probable bug in) get_wrapper_func_return_type(). These changes mean that "cppouttype" typemap is not used any longer, so remove it too. A couple more tests pass now. --- Examples/test-suite/c/Makefile.in | 4 --- Lib/c/c.swg | 19 +--------- Lib/c/std_string.i | 12 +++++-- Source/Modules/c.cxx | 58 ++++++++----------------------- 4 files changed, 25 insertions(+), 68 deletions(-) diff --git a/Examples/test-suite/c/Makefile.in b/Examples/test-suite/c/Makefile.in index d8a557807..8ec13cb04 100644 --- a/Examples/test-suite/c/Makefile.in +++ b/Examples/test-suite/c/Makefile.in @@ -48,7 +48,6 @@ FAILING_CPP_TESTS := \ arrays_dimensionless \ arrays_global \ arrays_global_twodim \ - char_strings \ constant_pointers \ director_smartptr \ director_string \ @@ -57,7 +56,6 @@ FAILING_CPP_TESTS := \ extend_default \ extern_c \ extern_template_method \ - features \ funcptr_cpp \ global_scope_types \ grouping \ @@ -67,7 +65,6 @@ FAILING_CPP_TESTS := \ li_boost_shared_ptr_bits \ li_boost_shared_ptr_director \ li_boost_shared_ptr_template \ - li_carrays_cpp \ li_std_combinations \ li_std_deque \ li_std_map \ @@ -83,7 +80,6 @@ FAILING_CPP_TESTS := \ nested_class \ smart_pointer_template_defaults_overload \ stl_no_default_constructor \ - struct_initialization_cpp \ template_basic \ template_default \ template_enum \ diff --git a/Lib/c/c.swg b/Lib/c/c.swg index 96a463cba..20e03548e 100644 --- a/Lib/c/c.swg +++ b/Lib/c/c.swg @@ -204,7 +204,7 @@ same_action_all_primitive_types(out, "$result = $1;", "$result = *$1;") *($&1_ltype) &$result = $1; } -%typemap(out) SWIGTYPE "$result = (SwigObj*) &$1;" +%typemap(out) SWIGTYPE "$result = (SwigObj*)new $1_ltype($1);" %typemap(out) SWIGTYPE *, SWIGTYPE & "$result = (SwigObj*) $1;" @@ -231,23 +231,6 @@ same_action_all_primitive_types(out, "$result = $1;", "$result = *$1;") $result = ($1_ltype) 0; } -// typemaps for 'cppresult' -same_macro_all_primitive_types_but_void(same_type,cppouttype); -same_macro_all_primitive_types_but_void(cref_as_ptr,cppouttype); - -%typemap(cppouttype, retobj="1") SWIGTYPE "$1_ltype *" -%typemap(cppouttype) SWIGTYPE * "$1_ltype" -%typemap(cppouttype) const SWIGTYPE * "const $1_ltype" -%typemap(cppouttype) SWIGTYPE & "$1_ltype" -%typemap(cppouttype) SWIGTYPE [ANY] "$1_ltype" -%typemap(cppouttype) SWIGTYPE * [ANY] "/*SWIGTYPE *[ANY] */ $1_ltype" -%typemap(cppouttype, retobj="1") enum SWIGTYPE "int" -%typemap(cppouttype) enum SWIGTYPE &, enum SWIGTYPE * "int *" -%typemap(cppouttype) SWIGTYPE (CLASS::*) "$1_ltype" - -%typemap(cppouttype, fragment="stdbool_inc") bool, const bool, const bool & "bool" -%typemap(cppouttype, fragment="stdbool_inc") bool *, const bool *, bool & "bool *" - #ifdef SWIG_CPPMODE %insert("runtime") %{ diff --git a/Lib/c/std_string.i b/Lib/c/std_string.i index d7ea01e39..546d34a41 100644 --- a/Lib/c/std_string.i +++ b/Lib/c/std_string.i @@ -13,8 +13,6 @@ class string; %typemap(ctype) string * "char *" %typemap(ctype) string & "char *" %typemap(ctype) const string & "char *" -%typemap(cppouttype, retobj="1") string "std::string*" -%typemap(cppouttype) const string &, string *, string & "std::string*" %typemap(in) string { if ($input) { @@ -40,7 +38,15 @@ class string; delete $1; } -%typemap(out) string, const string &, string *, string & { +%typemap(out) string { + const char *str = cppresult.c_str(); + size_t len = strlen(str); + $result = (char *) malloc(len + 1); + memcpy($result, str, len); + $result[len] = '\0'; +} + +%typemap(out) const string &, string *, string & { const char *str = cppresult->c_str(); size_t len = strlen(str); $result = (char *) malloc(len + 1); diff --git a/Source/Modules/c.cxx b/Source/Modules/c.cxx index e5fe50222..a30d3ccfb 100644 --- a/Source/Modules/c.cxx +++ b/Source/Modules/c.cxx @@ -915,7 +915,6 @@ public: current_output = output_wrapper_def; // C++ function wrapper - String *storage = Getattr(n, "storage"); SwigType *type = Getattr(n, "type"); SwigType *otype = Copy(type); SwigType *return_type = get_wrapper_func_return_type(n); @@ -923,7 +922,6 @@ public: ParmList *parms = Getattr(n, "parms"); Parm *p; bool is_void_return = (SwigType_type(type) == T_VOID); - bool return_object = false; // create new function wrapper object Wrapper *wrapper = NewWrapper(); @@ -932,12 +930,12 @@ public: // add variable for holding result of original function 'cppresult' if (!is_void_return) { - if (String *tm = Swig_typemap_lookup("cppouttype", n, "", 0)) { - Wrapper_add_local(wrapper, "cppresult", SwigType_str(tm, "cppresult")); - return_object = checkAttribute(n, "tmap:cppouttype:retobj", "1"); - } else { - Swig_warning(WARN_C_TYPEMAP_CTYPE_UNDEF, input_file, line_number, "No cppouttype typemap defined for %s\n", SwigType_str(type, 0)); - } + SwigType *value_type = cplus_value_type(type); + SwigType* cppresult_type = value_type ? value_type : type; + SwigType* ltype = SwigType_ltype(cppresult_type); + Wrapper_add_local(wrapper, "cppresult", SwigType_str(ltype, "cppresult")); + Delete(ltype); + Delete(value_type); } // create wrapper function prototype @@ -976,35 +974,7 @@ public: Replaceall(action, Getattr(n, "name"), cbase_name); } - // handle special cases of cpp return result - if (SwigType_isenum(SwigType_base(type))) { - if (return_object) - Replaceall(action, "result =", "cppresult = (int)"); - else - Replaceall(action, "result =", "cppresult = (int*)"); - } else if (return_object && Getattr(n, "c:retval") && - !SwigType_isarray(type) && - (Cmp(storage, "static") != 0)) { - // returning object by value - String *str = SwigType_str(SwigType_add_reference(SwigType_base(type)), "_result_ref"); - String *lstr = SwigType_lstr(type, 0); - if (Cmp(Getattr(n, "kind"), "variable") == 0) { - Delete(action); - action = NewStringf("{const %s = %s;", str, Swig_cmemberget_call(Getattr(n, "name"), type, 0, 0)); - } else { - String *call_str = NewStringf("{const %s = %s", str, - SwigType_ispointer(SwigType_typedef_resolve_all(otype)) ? "*" : ""); - Replaceall(action, "result =", call_str); - Delete(call_str); - } - if (Getattr(n, "nested")) - Replaceall(action, "=", NewStringf("= *(%s)(void*) &", SwigType_str(otype, 0))); - Printf(action, "cppresult = (%s*) &_result_ref;}", lstr); - Delete(str); - Delete(lstr); - } else { - Replaceall(action, "result =", "cppresult ="); - } + Replaceall(action, "result =", "cppresult ="); // prepare action code to use, e.g. insert try-catch blocks action = emit_action(n); @@ -1013,6 +983,14 @@ public: if (!is_void_return) { String *tm; if ((tm = Swig_typemap_lookup_out("out", n, "cppresult", wrapper, action))) { + // This is ugly, but the type of our result variable is not always the same as the actual return type currently because + // get_wrapper_func_return_type() applies ctype typemap to it. These types are more or less compatible though, so we should be able to cast + // between them explicitly. + const char* start = Char(tm); + const char* p = strstr(start, "$result = "); + if (p == start || (p && p[-1] == ' ')) { + Insert(tm, p - start + strlen("$result = "), NewStringf("(%s)", return_type)); + } Replaceall(tm, "$result", "result"); Replaceall(tm, "$owner", GetFlag(n, "feature:new") ? "1" : "0"); Printf(wrapper->code, "%s", tm); @@ -1060,8 +1038,6 @@ public: { ParmList *parms = Getattr(n, "parms"); String *name = Copy(Getattr(n, "sym:name")); - SwigType *type = Getattr(n, "type"); - SwigType *tdtype = NULL; // mangle name if function is overloaded if (Getattr(n, "sym:overloaded")) { @@ -1099,10 +1075,6 @@ public: } } - // resolve correct type - if((tdtype = SwigType_typedef_resolve_all(type))) - Setattr(n, "type", tdtype); - // make sure lnames are set functionWrapperPrepareArgs(parms);