From c794d08597e6f0c0b54cd70438f3374ccff43201 Mon Sep 17 00:00:00 2001
From: William S Fulton <wsf@fultondesigns.co.uk>
Date: Tue, 23 Aug 2011 19:29:10 +0000
Subject: [PATCH] Fix %newobject when used in conjunction with %feature(ref).
 The code from the ref feature was not always being generated for the function
 specified by %newobject. Documentation for ref and unref moved from Python to
 the C++ chapter.

git-svn-id: https://swig.svn.sourceforge.net/svnroot/swig/trunk@12783 626c5289-ae23-0410-ae9c-e8d60b6d4f22
---
 CHANGES.current                              |   5 +
 Doc/Manual/Customization.html                |   5 +
 Doc/Manual/Python.html                       | 144 +--------------
 Doc/Manual/SWIGPlus.html                     | 174 +++++++++++++++++++
 Examples/test-suite/python/refcount_runme.py |  33 +++-
 Examples/test-suite/refcount.i               |  23 ++-
 Source/Swig/cwrap.c                          |  22 +--
 Source/Swig/typemap.c                        |  16 +-
 8 files changed, 262 insertions(+), 160 deletions(-)

diff --git a/CHANGES.current b/CHANGES.current
index 35010d273..b59952e27 100644
--- a/CHANGES.current
+++ b/CHANGES.current
@@ -5,6 +5,11 @@ See the RELEASENOTES file for a summary of changes in each release.
 Version 2.0.5 (in progress)
 ===========================
 
+2011-08-23: wsfulton
+            Fix %newobject when used in conjunction with %feature("ref") as reported by Jan Becker. The
+            code from the "ref" feature was not always being generated for the function specified by %newobject.
+            Documentation for "ref" and "unref" moved from Python to the C++ chapter.
+
 2011-08-22: szager
 	    [python] Fixed memory leak with --builtin option (bug 3385089).
 
diff --git a/Doc/Manual/Customization.html b/Doc/Manual/Customization.html
index 7f164ac57..b98fbfc88 100644
--- a/Doc/Manual/Customization.html
+++ b/Doc/Manual/Customization.html
@@ -704,6 +704,11 @@ depends on the target language on implementing the 'disown' mechanism
 properly.
 </p>
 
+<p>
+The use of <tt>%newobject</tt> is also integrated with reference counting and is covered in the
+<a href="SWIGPlus.html#SWIGPlus_ref_unref">C++ reference counted objects</a> section.
+</p>
+
 <p>
 <b>Compatibility note:</b>  Previous versions of SWIG had a special <tt>%new</tt> directive.  However, unlike <tt>%newobject</tt>,
 it only applied to the next declaration.  For example:
diff --git a/Doc/Manual/Python.html b/Doc/Manual/Python.html
index fa312c4f1..78e1fbeed 100644
--- a/Doc/Manual/Python.html
+++ b/Doc/Manual/Python.html
@@ -38,7 +38,7 @@
 <li><a href="#Python_nn25">C++ namespaces</a>
 <li><a href="#Python_nn26">C++ templates</a>
 <li><a href="#Python_nn27">C++ Smart Pointers</a>
-<li><a href="#Python_nn27a">C++ Reference Counted Objects (ref/unref)</a>
+<li><a href="#Python_nn27a">C++ reference counted objects</a>
 </ul>
 <li><a href="#Python_nn28">Further details on the Python class interface</a>
 <ul>
@@ -2053,147 +2053,11 @@ simply use the <tt>__deref__()</tt> method.  For example:
 </pre>
 </div>
 
-
-<H3><a name="Python_nn27a"></a>33.3.15 C++ Reference Counted Objects (ref/unref)</H3>
-
+<H3><a name="Python_nn27a"></a>33.3.15 C++ reference counted objects</H3>
 
 <p>
-Another usual idiom in C++ is the use of reference counted
-objects. Consider for example:
-
-<div class="code">
-<pre>
-class RCObj  {
-  // implement the ref counting mechanism
-  int add_ref();
-  int del_ref();
-  int ref_count();
-
-public:
-  virtual ~RCObj() = 0;
-
-  int ref() const {
-    return add_ref();
-  }
-
-  int unref() const   {
-    if (ref_count() == 0 || del_ref() == 0 ) {
-	delete this;
-	return 0;
-      } 
-    return ref_count();
-  }
-};
-
-
-class A : RCObj {
-public:
-  A();
-  int foo();
-};
-
-
-class B {
-  A *_a;
-
-public:
-  B(A *a) : _a(a) { 
-    a-&gt;ref(); 
-  }
-
-  ~B() { 
-    a-&gt;unref(); 
-  }
-};
-
-int main() {
-  A *a  = new A();
-  a-&gt;ref();           // 'a' is ref here
-
-  B *b1 = new B(a);   // 'a' is ref here
-  if (1 + 1 == 2) {
-    B *b2 = new B(a); // 'a' is ref here
-    delete b2;        // 'a' is unref, but not deleted   
-  }
-
-  delete b1;          // 'a' is unref, but not deleted   
-  a-&gt;unref();         // 'a' is unref and deleted
-}
-</pre>
-</div>
-
-<p>
-In the example above, the 'A' class instance 'a' is a reference counted
-object, which can't be deleted arbitrarily since it is shared between
-the objects 'b1' and 'b2'. 'A' is derived from an Reference Counted
-Object 'RCObj', which implements the ref/unref idiom.
-</p>
-
-<p>
-To tell SWIG that 'RCObj' and all its derived classes are reference
-counted objects, you use the "ref" and "unref" features.
-For example:
-</p>
-
-
-<div class="code">
-<pre>
-%module example
-...
-
-%feature("ref")   RCObj "$this-&gt;ref();"
-%feature("unref") RCObj "$this-&gt;unref();"
-
-%include "rcobj.h"
-%include "A.h"
-...
-</pre>
-</div>
-
-<p>
-where the code passed to the "ref" and "unref" features will be
-executed as needed whenever a new object is passed to python, or when
-python tries to release the proxy object instance, respectively. 
-</p>
-
-<p> 
-In the python side, the use of a reference counted object is not
-different than any other regular instance:
-</p>
-
-<div class="targetlang">
-<pre>
-def create_A():
-  a = A()         # SWIG ref 'a' (new object is passed to python)
-  b1 = B(a)       # C++  ref 'a'
-  if 1 + 1 == 2:
-     b2 = B(a)    # C++ ref 'a'
-  return a        # 'b1' and 'b2' are released, C++ unref 'a' twice
-
-a = create_A()   
-exit              # 'a' is released, SWIG unref 'a'
-</pre>
-</div>
-
-<p>
-Note that the user doesn't explicitly need to call 'a-&gt;ref()' nor 'a-&gt;unref()'
-(as neither 'delete a'). Instead, SWIG take cares of executing the "ref"
-and "unref" codes as needed.  If the user doesn't specify the
-"ref/unref" features, SWIG will produce a code equivalent to define
-them as:
-</p>
-
-<div class="code">
-<pre>
-%feature("ref")   ""
-%feature("unref") "delete $this;"
-</pre>
-</div>
-
-<p>
-In other words, SWIG will not do anything special when a new object
-is passed to python, and it will always 'delete' the object when
-python releases the proxy instance.
+The <a href="SWIGPlus.html#SWIGPlus_ref_unref">C++ reference counted objects</a> section contains
+Python examples of memory management using referencing counting.
 </p>
 
 
diff --git a/Doc/Manual/SWIGPlus.html b/Doc/Manual/SWIGPlus.html
index 97c2e058a..83b9f9da3 100644
--- a/Doc/Manual/SWIGPlus.html
+++ b/Doc/Manual/SWIGPlus.html
@@ -4618,6 +4618,180 @@ p = f.__deref__()       # Raw pointer from operator-&gt;
 <b>Note:</b> Smart pointer support was first added in SWIG-1.3.14.
 </p>
 
+<H2><a name="SWIGPlus_ref_unref"></a>C++ reference counted objects - ref/unref feature</H2>
+
+
+<p>
+Another similar idiom in C++ is the use of reference counted objects. Consider for example:
+
+<div class="code">
+<pre>
+class RCObj  {
+  // implement the ref counting mechanism
+  int add_ref();
+  int del_ref();
+  int ref_count();
+
+public:
+  virtual ~RCObj() = 0;
+
+  int ref() const {
+    return add_ref();
+  }
+
+  int unref() const   {
+    if (ref_count() == 0 || del_ref() == 0 ) {
+	delete this;
+	return 0;
+      } 
+    return ref_count();
+  }
+};
+
+
+class A : RCObj {
+public:
+  A();
+  int foo();
+};
+
+
+class B {
+  A *_a;
+
+public:
+  B(A *a) : _a(a) { 
+    a-&gt;ref(); 
+  }
+
+  ~B() { 
+    a-&gt;unref(); 
+  }
+};
+
+int main() {
+  A *a  = new A();       // (count: 0)
+  a-&gt;ref();           // 'a' ref here (count: 1)
+
+  B *b1 = new B(a);   // 'a' ref here (count: 2)
+  if (1 + 1 == 2) {
+    B *b2 = new B(a); // 'a' ref here (count: 3)
+    delete b2;        // 'a' unref, but not deleted (count: 2)
+  }
+
+  delete b1;          // 'a' unref, but not deleted (count: 1)
+  a-&gt;unref();         // 'a' unref and deleted (count: 0)
+}
+</pre>
+</div>
+
+<p>
+In the example above, the 'A' class instance 'a' is a reference counted
+object, which can't be deleted arbitrarily since it is shared between
+the objects 'b1' and 'b2'. 'A' is derived from a <i>Reference Counted
+ Object</i> 'RCObj', which implements the ref/unref idiom.
+</p>
+
+<p>
+To tell SWIG that 'RCObj' and all its derived classes are reference
+counted objects, use the "ref" and "unref" <a href="Customization.html#Customization_features">features</a>.
+These are also available as <tt>%refobject</tt> and <tt>%unrefobject</tt>, respectively.
+For example:
+</p>
+
+
+<div class="code">
+<pre>
+%module example
+...
+
+%feature("ref")   RCObj "$this-&gt;ref();"
+%feature("unref") RCObj "$this-&gt;unref();"
+
+%include "rcobj.h"
+%include "A.h"
+...
+</pre>
+</div>
+
+<p>
+where the code passed to the "ref" and "unref" features will be
+executed as needed whenever a new object is passed to python, or when
+python tries to release the proxy object instance, respectively. 
+</p>
+
+<p> 
+On the python side, the use of a reference counted object is no
+different to any other regular instance:
+</p>
+
+<div class="targetlang">
+<pre>
+def create_A():
+  a = A()         # SWIG ref 'a' - new object is passed to python (count: 1)
+  b1 = B(a)       # C++ ref 'a (count: 2)
+  if 1 + 1 == 2:
+     b2 = B(a)    # C++ ref 'a' (count: 3)
+  return a        # 'b1' and 'b2' are released and deleted, C++ unref 'a' twice (count: 1)
+
+a = create_A()    # (count: 1)
+exit              # 'a' is released, SWIG unref 'a' called in the destructor wrapper (count: 0)
+</pre>
+</div>
+
+<p>
+Note that the user doesn't explicitly need to call 'a-&gt;ref()' nor 'a-&gt;unref()'
+(and neither 'delete a'). Instead, SWIG takes cares of executing the "ref"
+and "unref" calls as needed.  If the user doesn't specify the
+"ref/unref" feature for a type, SWIG will produce code equivalent to defining these
+features:
+</p>
+
+<div class="code">
+<pre>
+%feature("ref")   ""
+%feature("unref") "delete $this;"
+</pre>
+</div>
+
+<p>
+In other words, SWIG will not do anything special when a new object
+is passed to python, and it will always 'delete' the underlying object when
+python releases the proxy instance.
+</p>
+
+<p>
+</p>
+
+<p>
+The <a href="Customization.html#Customization_ownership">%newobject feature</a> is designed to indicate to
+the target language that it should take ownership of the returned object.
+When used in conjunction with a type that has the "ref" feature associated with it, it additionally emits the
+code in the "ref" feature into the C++ wrapper.
+Consider wrapping the following factory function in addition to the above:
+</p>
+
+<div class="code">
+<pre>
+%newobject AFactory;
+A *AFactory() {
+  return new A();
+}
+</pre>
+</div>
+
+<p>
+The <tt>AFactory</tt> function now acts much like a call to the <tt>A</tt> constructor with respect to memory handling:
+</p>
+
+<div class="targetlang">
+<pre>
+a = AFactory()    # SWIG ref 'a' due to %newobject (count: 1)
+exit              # 'a' is released, SWIG unref 'a' called in the destructor wrapper (count: 0)
+</pre>
+</div>
+
+
 
 <H2><a name="SWIGPlus_nn35"></a>6.25 Using declarations and inheritance</H2>
 
diff --git a/Examples/test-suite/python/refcount_runme.py b/Examples/test-suite/python/refcount_runme.py
index b429404e7..ab1803f24 100644
--- a/Examples/test-suite/python/refcount_runme.py
+++ b/Examples/test-suite/python/refcount_runme.py
@@ -10,14 +10,14 @@ b2 = B.create(a)
 
 
 if a.ref_count() != 3:
-  print "This program will crash... now"
+  raise RuntimeError("Count = %d" % a.ref_count())
 
 
 rca = b2.get_rca()
 b3 = B.create(rca)
 
 if a.ref_count() != 5:
-  print "This program will crash... now"
+  raise RuntimeError("Count = %d" % a.ref_count())
 
 
 v = vector_A(2)
@@ -27,6 +27,35 @@ v[1] = a
 x = v[0]
 del v
 
+if a.ref_count() != 6:
+  raise RuntimeError("Count = %d" % a.ref_count())
+
+# Check %newobject
+b4 = b2.cloner()
+if b4.ref_count() != 1:
+  raise RuntimeError
+
+b5 = global_create(a)
+if b5.ref_count() != 1:
+  raise RuntimeError
+
+b6 = Factory.create(a)
+if b6.ref_count() != 1:
+  raise RuntimeError
+
+b7 = Factory().create2(a)
+if b7.ref_count() != 1:
+  raise RuntimeError
 
 
+if a.ref_count() != 10:
+  raise RuntimeError("Count = %d" % a.ref_count())
+
+del b4
+del b5
+del b6
+del b7
+
+if a.ref_count() != 6:
+  raise RuntimeError("Count = %d" % a.ref_count())
 
diff --git a/Examples/test-suite/refcount.i b/Examples/test-suite/refcount.i
index 8352b508a..28b55545b 100644
--- a/Examples/test-suite/refcount.i
+++ b/Examples/test-suite/refcount.i
@@ -8,8 +8,8 @@
 %}
 
 //
-// using the %refobject/%unrefobject directives you can active the
-// ref. counting for RCObj and all its descendents at once
+// using the %refobject/%unrefobject directives you can activate the
+// reference counting for RCObj and all its descendents at once
 //
 
 %refobject   RCObj "$this->addref();"
@@ -18,7 +18,10 @@
 %include "refcount.h"
 
 %newobject B::create(A* a);
+%newobject global_create(A* a);
 %newobject B::cloner();
+%newobject Factory::create(A* a);
+%newobject Factory::create2(A* a);
 
 
  
@@ -94,6 +97,22 @@
     RCPtr<A> _a;
   };
 
+class B* global_create(A* a)
+{
+  return new B(a);
+}
+
+struct Factory {
+  static B* create(A* a)
+  {
+    return new B(a);
+  }
+  B* create2(A* a)
+  {
+    return new B(a);
+  }
+};
+
 %}
 
 #if defined(SWIGPYTHON) || defined(SWIGOCTAVE)
diff --git a/Source/Swig/cwrap.c b/Source/Swig/cwrap.c
index c6750b205..a996c6e29 100644
--- a/Source/Swig/cwrap.c
+++ b/Source/Swig/cwrap.c
@@ -546,7 +546,7 @@ String *Swig_cppconstructor_director_call(const_String_or_char_ptr name, ParmLis
 }
 
 /* -----------------------------------------------------------------------------
- * Swig_rflag_search()
+ * recursive_flag_search()
  *
  * This function searches for the class attribute 'attr' in the class
  * 'n' or recursively in its bases.
@@ -567,7 +567,7 @@ String *Swig_cppconstructor_director_call(const_String_or_char_ptr name, ParmLis
  * ----------------------------------------------------------------------------- */
 
 /* #define SWIG_FAST_REC_SEARCH 1 */
-String *Swig_rflag_search(Node *n, const String *attr, const String *noattr) {
+static String *recursive_flag_search(Node *n, const String *attr, const String *noattr) {
   String *f = 0;
   n = Swig_methodclass(n);
   if (GetFlag(n, noattr)) {
@@ -581,7 +581,7 @@ String *Swig_rflag_search(Node *n, const String *attr, const String *noattr) {
     if (bl) {
       Iterator bi;
       for (bi = First(bl); bi.item; bi = Next(bi)) {
-	f = Swig_rflag_search(bi.item, attr, noattr);
+	f = recursive_flag_search(bi.item, attr, noattr);
 	if (f) {
 #ifdef SWIG_FAST_REC_SEARCH
 	  SetFlagAttr(n, attr, f);
@@ -600,12 +600,11 @@ String *Swig_rflag_search(Node *n, const String *attr, const String *noattr) {
 /* -----------------------------------------------------------------------------
  * Swig_unref_call()
  *
- * find the unref call, if any.
+ * Find the "feature:unref" call, if any.
  * ----------------------------------------------------------------------------- */
 
 String *Swig_unref_call(Node *n) {
-  Node *cn = Swig_methodclass(n);
-  String *unref = Swig_rflag_search(cn, "feature:unref", "feature:nounref");
+  String *unref = recursive_flag_search(n, "feature:unref", "feature:nounref");
   if (unref) {
     String *pname = Swig_cparm_name(0, 0);
     unref = NewString(unref);
@@ -619,12 +618,11 @@ String *Swig_unref_call(Node *n) {
 /* -----------------------------------------------------------------------------
  * Swig_ref_call()
  *
- * find the ref call, if any.
+ * Find the "feature:ref" call, if any.
  * ----------------------------------------------------------------------------- */
 
 String *Swig_ref_call(Node *n, const String *lname) {
-  Node *cn = Swig_methodclass(n);
-  String *ref = Swig_rflag_search(cn, "feature:ref", "feature:noref");
+  String *ref = recursive_flag_search(n, "feature:ref", "feature:noref");
   if (ref) {
     ref = NewString(ref);
     Replaceall(ref, "$this", lname);
@@ -642,7 +640,8 @@ String *Swig_ref_call(Node *n, const String *lname) {
  * ----------------------------------------------------------------------------- */
 
 String *Swig_cdestructor_call(Node *n) {
-  String *unref = Swig_unref_call(n);
+  Node *cn = Swig_methodclass(n);
+  String *unref = Swig_unref_call(cn);
 
   if (unref) {
     return unref;
@@ -664,7 +663,8 @@ String *Swig_cdestructor_call(Node *n) {
  * ----------------------------------------------------------------------------- */
 
 String *Swig_cppdestructor_call(Node *n) {
-  String *unref = Swig_unref_call(n);
+  Node *cn = Swig_methodclass(n);
+  String *unref = Swig_unref_call(cn);
   if (unref) {
     return unref;
   } else {
diff --git a/Source/Swig/typemap.c b/Source/Swig/typemap.c
index 8f5d07158..0d582a157 100644
--- a/Source/Swig/typemap.c
+++ b/Source/Swig/typemap.c
@@ -1309,15 +1309,21 @@ static String *Swig_typemap_lookup_impl(const_String_or_char_ptr tmap_method, No
   int optimal_substitution = 0;
   int num_substitutions = 0;
 
-  /* special case, we need to check for 'ref' call and set the default code 'sdef' */
-  if (node && Cmp(tmap_method, "newfree") == 0) {
-    sdef = Swig_ref_call(node, lname);
-  }
-
   type = Getattr(node, "type");
   if (!type)
     return sdef;
 
+  /* Special hook (hack!). Check for the 'ref' feature and add code it contains to any 'newfree' typemap code.
+   * We could choose to put this hook into a number of different typemaps, not necessarily 'newfree'... 
+   * Rather confusingly 'newfree' is used to release memory and the 'ref' feature is used to add in memory references - yuck! */
+  if (node && Cmp(tmap_method, "newfree") == 0) {
+    String *base = SwigType_base(type);
+    Node *typenode = Swig_symbol_clookup(base, 0);
+    if (typenode)
+      sdef = Swig_ref_call(typenode, lname);
+    Delete(base);
+  }
+
   pname = Getattr(node, "name");
 
   if (pname && node && checkAttribute(node, "kind", "function")) {