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

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

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 {

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
 

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",
+      Printf(stdout, "    change function '%s' to type='%s', decl='%s'\n", name, type, decl);
-	     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 */

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;
+}

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
+      // Unwrap return values that are director classes so that the original Python object is returned instead. 
-       * python object is returned instead.
+      if (!constructor && Swig_director_can_unwrap(n)) {
-       */
-#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) {
 	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);

Source/Modules/ruby.cxx

@@ -1871,24 +1871,8 @@ public:
             else
               Replaceall(tm, "$owner", "0");
 
-#if 1
+            // Unwrap return values that are director classes so that the original Ruby object is returned instead. 
-            // FIXME: this will not try to unwrap directors returned as non-director
+            if (Swig_director_can_unwrap(n)) {
-            //        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) {
               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));

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 */

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);

Source/Swig/typeobj.c

@@ -249,49 +249,6 @@ SwigType *SwigType_last(SwigType *t) {
   return result;
 }
 
-/* -----------------------------------------------------------------------------
- * 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()
  *