From d7e83c1cbcc21a1252c800d6fb367f48771628a9 Mon Sep 17 00:00:00 2001 From: Olly Betts Date: Tue, 8 Mar 2022 16:21:53 +1300 Subject: [PATCH] Fix checking of "optimal" typemap attribute Previously SWIG checked that the typemap action contained ";\n" not followed by an identifier character, and that it contained no other `;`, but that incorrectly allows some cases it shouldn't. Instead check that the action ends with `;\n` and contains no other `;`, which is simpler and correctly rejects these cases. --- .../errors/cpp_typemap_out_optimal_bug.i | 45 +++++++++++++++++++ .../errors/cpp_typemap_out_optimal_bug.stderr | 4 ++ Source/Swig/typemap.c | 26 +++++------ 3 files changed, 59 insertions(+), 16 deletions(-) create mode 100644 Examples/test-suite/errors/cpp_typemap_out_optimal_bug.i create mode 100644 Examples/test-suite/errors/cpp_typemap_out_optimal_bug.stderr diff --git a/Examples/test-suite/errors/cpp_typemap_out_optimal_bug.i b/Examples/test-suite/errors/cpp_typemap_out_optimal_bug.i new file mode 100644 index 000000000..e8168ec46 --- /dev/null +++ b/Examples/test-suite/errors/cpp_typemap_out_optimal_bug.i @@ -0,0 +1,45 @@ +%module x + +// Just the following languages tested +#if defined (SWIGCSHARP) || defined (SWIGD) +%typemap(out, optimal="1") SWIGTYPE %{ + $result = new $1_ltype((const $1_ltype &)$1); +%} +#elif defined (SWIGJAVA) +%typemap(out, optimal="1") SWIGTYPE %{ + *($&1_ltype*)&$result = new $1_ltype((const $1_ltype &)$1); +%} +#elif defined (SWIGUTL) +%typemap(out,noblock="1", optimal="1") SWIGTYPE { + %set_output(SWIG_NewPointerObj(%new_copy($1, $ltype), $&descriptor, SWIG_POINTER_OWN | %newpointer_flags)); +} +#endif + +// This results in an action which SWIG should disable "optimal" for, but +// it was failing to. Fixed in SWIG 4.1.0. +%exception XX::create() "$action\n{while(sleep(1)){}}" + +%ignore XX::operator=; + +#ifdef SWIGD +%rename(trace) XX::debug; +#endif + +%inline %{ +#include +using namespace std; + +struct XX { + XX() { if (debug) cout << "XX()" << endl; } + XX(int i) { if (debug) cout << "XX(" << i << ")" << endl; } + XX(const XX &other) { if (debug) cout << "XX(const XX &)" << endl; } + XX& operator =(const XX &other) { if (debug) cout << "operator=(const XX &)" << endl; return *this; } + ~XX() { if (debug) cout << "~XX()" << endl; } + static XX create() { + return XX(123); + } + static bool debug; +}; +bool XX::debug = true; +%} + diff --git a/Examples/test-suite/errors/cpp_typemap_out_optimal_bug.stderr b/Examples/test-suite/errors/cpp_typemap_out_optimal_bug.stderr new file mode 100644 index 000000000..15bf64f46 --- /dev/null +++ b/Examples/test-suite/errors/cpp_typemap_out_optimal_bug.stderr @@ -0,0 +1,4 @@ +cpp_typemap_out_optimal_bug.i:40: Warning 474: Method XX::create() usage of the optimal attribute ignored +cpp_typemap_out_optimal_bug.i:15: Warning 474: in the out typemap as the following cannot be used to generate optimal code: result = XX::create(); +{while(sleep(1)){}} + diff --git a/Source/Swig/typemap.c b/Source/Swig/typemap.c index 3273c19db..58346abb9 100644 --- a/Source/Swig/typemap.c +++ b/Source/Swig/typemap.c @@ -1443,7 +1443,6 @@ static String *Swig_typemap_lookup_impl(const_String_or_char_ptr tmap_method, No * ie, not use the typemap code, otherwise both f and actioncode must be non null. */ if (actioncode) { const String *result_equals = NewStringf("%s = ", Swig_cresult_name()); - clname = Copy(actioncode); /* check that the code in the typemap can be used in this optimal way. * The code should be in the form "result = ...;\n". We need to extract * the "..." part. This may not be possible for various reasons, eg @@ -1451,22 +1450,17 @@ static String *Swig_typemap_lookup_impl(const_String_or_char_ptr tmap_method, No * hack and circumvents the normal requirement for a temporary variable * to hold the result returned from a wrapped function call. */ - if (Strncmp(clname, result_equals, 9) == 0) { - int numreplacements = Replace(clname, result_equals, "", DOH_REPLACE_ID_BEGIN); - if (numreplacements == 1) { - numreplacements = Replace(clname, ";\n", "", DOH_REPLACE_ID_END); - if (numreplacements == 1) { - if (Strchr(clname, ';') == 0) { - lname = clname; - actioncode = 0; - optimal_substitution = 1; - } - } - } - } - if (!optimal_substitution) { + if (Strncmp(actioncode, result_equals, Len(result_equals)) == 0 && + Strchr(actioncode, ';') == Char(actioncode) + Len(actioncode) - 2 && + Char(actioncode)[Len(actioncode) - 1] == '\n') { + clname = NewStringWithSize(Char(actioncode) + Len(result_equals), + Len(actioncode) - Len(result_equals) - 2); + lname = clname; + actioncode = 0; + optimal_substitution = 1; + } else { Swig_warning(WARN_TYPEMAP_OUT_OPTIMAL_IGNORED, Getfile(node), Getline(node), "Method %s usage of the optimal attribute ignored\n", Swig_name_decl(node)); - Swig_warning(WARN_TYPEMAP_OUT_OPTIMAL_IGNORED, Getfile(s), Getline(s), "in the out typemap as the following cannot be used to generate optimal code: %s\n", clname); + Swig_warning(WARN_TYPEMAP_OUT_OPTIMAL_IGNORED, Getfile(s), Getline(s), "in the out typemap as the following cannot be used to generate optimal code: %s\n", actioncode); delete_optimal_attribute = 1; } } else {