diff options
author | William S Fulton <wsf@fultondesigns.co.uk> | 2022-10-10 08:27:44 +0100 |
---|---|---|
committer | William S Fulton <wsf@fultondesigns.co.uk> | 2022-10-10 08:45:26 +0100 |
commit | 2f55379687a394e941a20e2224eb1125d66720cd (patch) | |
tree | c2fed466f30bc0866402bfa8e1ecd46da1ce3ff4 | |
parent | 4a397869a2e2be3c196fac3b404aabe0cc19f494 (diff) | |
download | swig-2f55379687a394e941a20e2224eb1125d66720cd.tar.gz |
Improve director unwrap detection for the return type
Resolve the return type to correctly determine if the type is a pointer or
reference to a director class.
SwigType_refptr_count_return() recently added as a simpler fix is no
longer needed.
The conventional approach of using the "type" rather than "decl" to
analyse the return type is used instead too.
Issue #1823
-rw-r--r-- | CHANGES.current | 7 | ||||
-rw-r--r-- | Examples/test-suite/director_unwrap_result.i | 11 | ||||
-rw-r--r-- | Examples/test-suite/ruby/director_unwrap_result_runme.rb | 14 | ||||
-rw-r--r-- | Source/CParse/templ.c | 6 | ||||
-rw-r--r-- | Source/Modules/directors.cxx | 35 | ||||
-rw-r--r-- | Source/Modules/python.cxx | 24 | ||||
-rw-r--r-- | Source/Modules/ruby.cxx | 24 | ||||
-rw-r--r-- | Source/Modules/swigmod.h | 1 | ||||
-rw-r--r-- | Source/Swig/swig.h | 1 | ||||
-rw-r--r-- | Source/Swig/typeobj.c | 43 |
10 files changed, 76 insertions, 90 deletions
diff --git a/CHANGES.current b/CHANGES.current index 224ea36d7..bc9f386d8 100644 --- a/CHANGES.current +++ b/CHANGES.current @@ -7,6 +7,13 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/ Version 4.1.0 (in progress) =========================== +2022-10-10: treitmayr, wsfulton + [Python, Ruby] #1811 #1823 Fix invalid code generated in some cases when + returning a pointer or reference to a director-enabled class instance. + This previously only worked in very simple cases, now return types are + resolved to fix. A bug in template instantations using pointers also + works now. + 2022-10-06: wsfulton [CFFI] #1966 #2200 Remove code for Common Lisp CFFI. We dropped support for it in SWIG 4.0.0 by disabling it as the first stage. This is the diff --git a/Examples/test-suite/director_unwrap_result.i b/Examples/test-suite/director_unwrap_result.i index bcb7f0fcc..1acc6146f 100644 --- a/Examples/test-suite/director_unwrap_result.i +++ b/Examples/test-suite/director_unwrap_result.i @@ -25,6 +25,8 @@ class Element { return &selfptr; } }; +typedef Element * element_ptr_t; +typedef Element & element_ref_t; class Storage { public: @@ -45,6 +47,15 @@ class Storage { Element *&getElementPtrRef() { return *getIt(); } + element_ref_t getElementRefTypedef() { + return **getIt(); + } + element_ptr_t getElementPtrTypedef() { + return *getIt(); + } + element_ptr_t &getElementPtrRefTypedef() { + return *getIt(); + } }; template<class T> class StorageTmpl { diff --git a/Examples/test-suite/ruby/director_unwrap_result_runme.rb b/Examples/test-suite/ruby/director_unwrap_result_runme.rb index 56970b3e8..26206ae5c 100644 --- a/Examples/test-suite/ruby/director_unwrap_result_runme.rb +++ b/Examples/test-suite/ruby/director_unwrap_result_runme.rb @@ -41,12 +41,26 @@ swig_assert_equal('s.getElementPtr', 'e', binding) swig_assert_equal('s.getElementRef.class', 'MyElement', binding) swig_assert_equal('s.getElementRef', 'e', binding) +# this shows that the director class was unwrapped: +swig_assert_equal('s.getElementPtrTypedef.class', 'MyElement', binding) +swig_assert_equal('s.getElementPtrTypedef', 'e', binding) + +# this shows that the director class was unwrapped: +swig_assert_equal('s.getElementRefTypedef.class', 'MyElement', binding) +swig_assert_equal('s.getElementRefTypedef', 'e', binding) + +# this is not unwrapped: swig_assert_equal('s.getElementPtrPtr.class', 'SWIG::TYPE_p_p_Element', binding) swig_assert_equal('s.getElementPtrPtr.class', 'SWIG::TYPE_p_p_Element', binding) +# this is not unwrapped: swig_assert_equal('s.getElementPtrRef.class', 'SWIG::TYPE_p_p_Element', binding) swig_assert_equal('s.getElementPtrRef.class', 'SWIG::TYPE_p_p_Element', binding) +# this is not unwrapped: +swig_assert_equal('s.getElementPtrRefTypedef.class', 'SWIG::TYPE_p_p_Element', binding) +swig_assert_equal('s.getElementPtrRefTypedef.class', 'SWIG::TYPE_p_p_Element', binding) + ############################ # test with a template class diff --git a/Source/CParse/templ.c b/Source/CParse/templ.c index e6c1da4a6..0dec21586 100644 --- a/Source/CParse/templ.c +++ b/Source/CParse/templ.c @@ -248,7 +248,7 @@ static void cparse_template_expand(Node *templnode, Node *n, String *tname, Stri static void cparse_fix_function_decl(String *name, SwigType *decl, SwigType *type) { String *prefix; int prefixLen; - SwigType* last; + SwigType *last; /* The type's prefix is what potentially has to be moved to the end of 'decl' */ prefix = SwigType_prefix(type); @@ -275,8 +275,7 @@ static void cparse_fix_function_decl(String *name, SwigType *decl, SwigType *typ Append(decl, prefix); Delslice(type, 0, prefixLen); if (template_debug) { - Printf(stdout, " change function '%s' to type='%s', decl='%s'\n", - name, type, decl); + Printf(stdout, " change function '%s' to type='%s', decl='%s'\n", name, type, decl); } } @@ -510,7 +509,6 @@ int Swig_cparse_template_expand(Node *n, String *rname, ParmList *tparms, Symtab } } } - cparse_postprocess_expanded_template(n); /* Patch bases */ diff --git a/Source/Modules/directors.cxx b/Source/Modules/directors.cxx index fbef70aa1..14e2e8466 100644 --- a/Source/Modules/directors.cxx +++ b/Source/Modules/directors.cxx @@ -233,3 +233,38 @@ void Swig_director_parms_fixup(ParmList *parms) { } } +/* ----------------------------------------------------------------------------- + * Swig_director_can_unwrap() + * + * Determine whether a function's return type can be returned as an existing + * target language object instead of creating a new target language object. + * Must be a director class and only for return by pointer or reference only + * (not by value or by pointer to pointer etc). + * ----------------------------------------------------------------------------- */ + +bool Swig_director_can_unwrap(Node *n) { + + // FIXME: this will not try to unwrap directors returned as non-director + // base class pointers! + + bool unwrap = false; + + String *type = Getattr(n, "type"); + SwigType *t = SwigType_typedef_resolve_all(type); + SwigType *t1 = SwigType_strip_qualifiers(t); + SwigType *prefix = SwigType_prefix(t1); + + if (Strcmp(prefix, "p.") == 0 || Strcmp(prefix, "r.") == 0) { + Node *parent = Swig_methodclass(n); + Node *module = Getattr(parent, "module"); + Node *target = Swig_directormap(module, t1); + if (target) + unwrap = true; + } + + Delete(prefix); + Delete(t1); + Delete(t); + + return unwrap; +} diff --git a/Source/Modules/python.cxx b/Source/Modules/python.cxx index 6a1c0a0a2..a8175eb0e 100644 --- a/Source/Modules/python.cxx +++ b/Source/Modules/python.cxx @@ -3150,25 +3150,9 @@ public: Replaceall(tm, "$owner", "0"); } } - // FIXME: this will not try to unwrap directors returned as non-director - // base class pointers! - /* New addition to unwrap director return values so that the original - * python object is returned instead. - */ -#if 1 - int unwrap = 0; - String *decl = Getattr(n, "decl"); - if (SwigType_refptr_count_return(decl) == 1) { - String *type = Getattr(n, "type"); - //Node *classNode = Swig_methodclass(n); - //Node *module = Getattr(classNode, "module"); - Node *module = Getattr(parent, "module"); - Node *target = Swig_directormap(module, type); - if (target) - unwrap = 1; - } - if (unwrap) { + // Unwrap return values that are director classes so that the original Python object is returned instead. + if (!constructor && Swig_director_can_unwrap(n)) { Wrapper_add_local(f, "director", "Swig::Director *director = 0"); Printf(f->code, "director = SWIG_DIRECTOR_CAST(%s);\n", Swig_cresult_name()); Append(f->code, "if (director) {\n"); @@ -3180,9 +3164,7 @@ public: } else { Printf(f->code, "%s\n", tm); } -#else - Printf(f->code, "%s\n", tm); -#endif + Delete(tm); } else { Swig_warning(WARN_TYPEMAP_OUT_UNDEF, input_file, line_number, "Unable to use return type %s in function %s.\n", SwigType_str(d, 0), name); diff --git a/Source/Modules/ruby.cxx b/Source/Modules/ruby.cxx index 855e8d57d..268cb2775 100644 --- a/Source/Modules/ruby.cxx +++ b/Source/Modules/ruby.cxx @@ -1871,24 +1871,8 @@ public: else Replaceall(tm, "$owner", "0"); -#if 1 - // FIXME: this will not try to unwrap directors returned as non-director - // base class pointers! - - /* New addition to unwrap director return values so that the original - * Ruby object is returned instead. - */ - bool unwrap = false; - String *decl = Getattr(n, "decl"); - if (SwigType_refptr_count_return(decl) == 1) { - String *type = Getattr(n, "type"); - Node *parent = Swig_methodclass(n); - Node *modname = Getattr(parent, "module"); - Node *target = Swig_directormap(modname, type); - if (target) - unwrap = true; - } - if (unwrap) { + // Unwrap return values that are director classes so that the original Ruby object is returned instead. + if (Swig_director_can_unwrap(n)) { Wrapper_add_local(f, "director", "Swig::Director *director = 0"); Printf(f->code, "director = dynamic_cast<Swig::Director *>(%s);\n", Swig_cresult_name()); Printf(f->code, "if (director) {\n"); @@ -1900,9 +1884,7 @@ public: } else { Printf(f->code, "%s\n", tm); } -#else - Printf(f->code, "%s\n", tm); -#endif + Delete(tm); } else { Swig_warning(WARN_TYPEMAP_OUT_UNDEF, input_file, line_number, "Unable to use return type %s.\n", SwigType_str(t, 0)); diff --git a/Source/Modules/swigmod.h b/Source/Modules/swigmod.h index 11b13f9fc..c605edf9d 100644 --- a/Source/Modules/swigmod.h +++ b/Source/Modules/swigmod.h @@ -405,6 +405,7 @@ String *Swig_method_decl(SwigType *return_base_type, SwigType *decl, const_Strin String *Swig_director_declaration(Node *n); void Swig_director_emit_dynamic_cast(Node *n, Wrapper *f); void Swig_director_parms_fixup(ParmList *parms); +bool Swig_director_can_unwrap(Node *n); /* directors.cxx end */ /* Utilities */ diff --git a/Source/Swig/swig.h b/Source/Swig/swig.h index 943f5daf0..e9f7c92ef 100644 --- a/Source/Swig/swig.h +++ b/Source/Swig/swig.h @@ -140,7 +140,6 @@ extern "C" { extern String *SwigType_pop(SwigType *t); extern void SwigType_push(SwigType *t, String *s); extern SwigType *SwigType_last(SwigType *t); - extern int SwigType_refptr_count_return(const SwigType *t); extern List *SwigType_parmlist(const SwigType *p); extern String *SwigType_parm(const SwigType *p); extern String *SwigType_str(const SwigType *s, const_String_or_char_ptr id); diff --git a/Source/Swig/typeobj.c b/Source/Swig/typeobj.c index 581362520..8cd2e28e9 100644 --- a/Source/Swig/typeobj.c +++ b/Source/Swig/typeobj.c @@ -250,49 +250,6 @@ SwigType *SwigType_last(SwigType *t) { } /* ----------------------------------------------------------------------------- - * SwigType_refptr_count_return() - * - * Returns the number of pointer and reference indirections found in the given - * type. For functions this concerns the return type. - * For example: - * r.p. => 2 - * q(const).p. => 1 - * r.f(int).p.p. => 2 - * f().p.q(const).p. => 2 - * ----------------------------------------------------------------------------- */ - -int SwigType_refptr_count_return(const SwigType *t) { - char *c; - char *last; - int sz; - int result = 0; - - if (!t) - return 0; - - c = Char(t); - last = c; - while (*c) { - last = c; - sz = element_size(c); - c = c + sz; - if (*(c-1) == '.') { - if (((last[0] == 'p') || (last[0] == 'r')) && (last[1] == '.')) { - result++; - } else if (strncmp(last, "f(", 2) == 0) { - /* restart counter if this is a function type */ - result = 0; - } - } - if (*c == '.') { - c++; - } - } - - return result; -} - -/* ----------------------------------------------------------------------------- * SwigType_parm() * * Returns the parameter of an operator as a string |