Merge remote-tracking branch 'upstream/master' into OCaml-make-j-race-conds

CHANGES.current

@@ -7,6 +7,9 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/
 Version 4.0.0 (in progress)
 ===========================
 
+2019-01-11: ZackerySpytz
+            [OCaml] #1400 Fix getters and setters.
+
 2019-01-07: wsfulton
             #358 Add VOID to windows.i
 

Doc/Manual/Contents.html

@@ -1671,6 +1671,7 @@
 <li><a href="Python.html#Python_nn62">Mapping Python tuples into small arrays</a>
 <li><a href="Python.html#Python_nn63">Mapping sequences to C arrays</a>
 <li><a href="Python.html#Python_nn64">Pointer handling</a>
+<li><a href="Python.html#Python_memory_management_member_variables">Memory management when returning references to member variables</a>
 </ul>
 <li><a href="Python.html#Python_nn65">Docstring Features</a>
 <ul>

Doc/Manual/Python.html

@@ -100,6 +100,7 @@
 <li><a href="#Python_nn62">Mapping Python tuples into small arrays</a>
 <li><a href="#Python_nn63">Mapping sequences to C arrays</a>
 <li><a href="#Python_nn64">Pointer handling</a>
+<li><a href="#Python_memory_management_member_variables">Memory management when returning references to member variables</a>
 </ul>
 <li><a href="#Python_nn65">Docstring Features</a>
 <ul>
@@ -3572,6 +3573,7 @@ proxy, just before the return statement.
 
 %feature("pythonappend") Foo::bar(int) %{
     #do something after C++ call
+    #the 'val' variable holds the return value
 %}
 
 
@@ -3601,6 +3603,7 @@ SWIG version 1.3.28 you can use the directive forms
 
 %pythonappend Foo::bar(int) %{
     #do something after C++ call
+    #the 'val' variable holds the return value
 %}
 
 
@@ -5432,6 +5435,165 @@ that has a <tt>this</tt> attribute.  In addition,
 class object (if applicable).
 </p>
 
+<H3><a name="Python_memory_management_member_variables">38.9.7 Memory management when returning references to member variables</a></H3>
+
+
+<p>
+This example shows how to prevent premature garbage collection of objects when the underlying C++ class returns a pointer or reference to a member variable.
+The example is a direct equivalent to this <a href="Java.html#Java_memory_management_objects">Java equivalent</a>.
+</p>
+
+<p>
+Consider the following C++ code:
+</p>
+
+<div class="code">
+<pre>
+#include &lt;iostream&gt;
+struct Wheel {
+  int size;
+  Wheel(int sz) : size(sz) {}
+  ~Wheel() { std::cout &lt;&lt; "~Wheel" &lt;&lt; std::endl; }
+};
+
+class Bike {
+  Wheel wheel;
+public:
+  Bike(int val) : wheel(val) {}
+  Wheel&amp; getWheel() { return wheel; }
+};
+</pre>
+</div>
+
+<p>
+and the following usage from Python after running the code through SWIG:
+</p>
+
+
+<div class="code">
+<pre>
+bike = Bike(10)
+wheel = bike.getWheel()
+print("wheel size: {}".format(wheel.size))
+
+del bike  # Allow bike to be garbage collected
+print("wheel size: {}".format(wheel.size))
+</pre>
+</div>
+
+<p>
+Don't be surprised that if the resulting output gives strange results such as...
+</p>
+
+<div class="shell">
+<pre>
+wheel size: 10
+~Wheel
+wheel size: 135019664
+</pre>
+</div>
+
+<p>
+What has happened here is the garbage collector has collected the <tt>Bike</tt> instance as it doesn't think it is needed any more.
+The proxy instance, <tt>wheel</tt>, contains a reference to memory that was deleted when the <tt>Bike</tt> instance was collected.
+In order to prevent the garbage collector from collecting the <tt>Bike</tt> instance, a reference to the <tt>Bike</tt> must
+be added to the <tt>wheel</tt> instance. 
+</p>
+    
+<p>
+You can do this by adding the reference when the <tt>getWheel()</tt> method
+is called using one of three approaches:
+</p>
+    
+<p>
+The easier, but less optimized, way is to use the <tt>%pythonappend</tt> directive 
+(see <a href="#Python_nn42">Adding additional Python code</a>):
+</p>
+
+<div class="code">
+<pre>
+%pythonappend getWheel %{
+  # val is the Wheel proxy, self is the Bike instance
+  val.__bike_reference = self
+%}
+</pre>
+</div>
+
+<p>
+The code gets appended to the Python code generated for the 
+<tt>Bike::getWheel</tt> wrapper function, where we store the <tt>Bike</tt> proxy 
+instance onto the <tt>Wheel</tt> proxy instance before it is returned to the 
+caller as follows.
+</p>
+    
+<div class="targetlang">
+<pre>
+class Bike(object):
+    ...
+    def getWheel(self):
+        val = _example.Bike_getWheel(self)
+
+        # val is the Wheel proxy, self is the Bike instance
+        val.__bike_reference = self
+
+        return val
+</pre>
+</div>
+
+
+<p>
+The second option, which performs better and is required if you use the 
+<tt>-builtin</tt> option, is to set the reference in the CPython implementation:
+    
+<div class="code">
+<pre>
+%extend Wheel {
+// A reference to the parent class is added to ensure the underlying C++
+// object is not deleted while the item is in use
+%typemap(ret) Wheel&amp; getWheel {
+  PyObject *bike_reference_string = SWIG_Python_str_FromChar("__bike_reference");
+  PyObject_SetAttr($result, bike_reference_string, $self);
+  Py_DecRef(bike_reference_string);
+}
+}
+</pre>
+</div>
+
+<p>
+The third approach, shown below, is an optimization of the above approach and creates the "__bike_reference" Python string object just once.
+While this looks more complex, it is just a small variation on the above typemap plus a support function
+<tt>bike_reference()</tt> in a fragment called <tt>bike_reference_function</tt>.
+The <tt>bike_reference_init</tt> typemap generates code into the "init" section for an initial call to <tt>bike_reference()</tt> when the module
+is initialized and is done to create the "__bike_reference" Python string singleton in a thread-safe manner.
+</p>
+
+<div class="code">
+<pre>
+%fragment("bike_reference_init", "init") {
+  // Thread-safe initialization - initialize during Python module initialization
+  bike_reference();
+}
+
+%fragment("bike_reference_function", "header", fragment="bike_reference_init") {
+
+static PyObject *bike_reference() {
+  static PyObject *bike_reference_string = SWIG_Python_str_FromChar("__bike_reference");
+  return bike_reference_string;
+}
+
+}
+
+%extend Wheel {
+// A reference to the parent class is added to ensure the underlying C++
+// object is not deleted while the item is in use
+%typemap(ret, fragment="bike_reference_function") Wheel&amp; getWheel %{
+  PyObject_SetAttr($result, bike_reference(), $self);
+%}
+}
+</pre>
+</div>
+
+
 
 
 <H2><a name="Python_nn65">38.10 Docstring Features</a></H2>

Examples/test-suite/ocaml/Makefile.in

@@ -5,6 +5,7 @@
 LANGUAGE     = ocaml
 OCAMLP4WHERE =`$(COMPILETOOL) @CAMLP4@ -where`
 OCC          =$(COMPILETOOL) @OCAMLC@
+OCAMLPP      = -pp "camlp4o ./swigp4.cmo"
 VARIANT      = _static
 SCRIPTSUFFIX = _runme.ml
 
@@ -54,7 +55,7 @@ run_testcase = \
 		if [ $(srcdir) != . ]; then \
 			cp $(srcdir)/$(ml_runme) $(ml_runme); \
 		fi ; \
-		$(OCC) -c $(ml_runme) && \
+		$(OCC) $(OCAMLPP) -c $(ml_runme) && \
 		if [ -f $(top_srcdir)/Examples/test-suite/$*.list ]; then \
 			$(OCC) swig.cmo -custom -g -cc '$(CXX)' -o $*_runme `cat $(top_srcdir)/Examples/test-suite/$(*).list | sed -e 's/\(.*\)/\1_wrap.o \1.cmo/g'`&& $(RUNTOOL) ./$*_runme; \
 		else \

Examples/test-suite/ocaml/class_scope_weird_runme.ml

@@ -0,0 +1,6 @@
+open Swig
+open Class_scope_weird
+
+let f = new_Foo (C_void)
+let g = new_Foo (C_int 3)
+let _ = assert (get_int ((invoke f) "bar" (C_int 3)) = 3)

Examples/test-suite/ocaml/cpp_static_runme.ml

@@ -0,0 +1,17 @@
+open Swig
+open Cpp_static
+
+let _ = _StaticFunctionTest_static_func (C_void)
+let _ = _StaticFunctionTest_static_func_2 (C_int 1)
+let _ = _StaticFunctionTest_static_func_3 (C_list [C_int 1; C_int 2])
+
+let _ = assert (get_int (_StaticMemberTest_static_int (C_void)) = 99)
+let _ = _StaticMemberTest_static_int (C_int 10)
+let _ = assert (get_int (_StaticMemberTest_static_int (C_void)) = 10)
+
+let _ = assert (get_int (_StaticBase_statty (C_void)) = 11)
+let _ = assert (get_int (_StaticDerived_statty (C_void)) = 111)
+let _ = _StaticBase_statty (C_int 22)
+let _ = _StaticDerived_statty (C_int 222)
+let _ = assert (get_int (_StaticBase_statty (C_void)) = 22)
+let _ = assert (get_int (_StaticDerived_statty (C_void)) = 222)

Examples/test-suite/ocaml/ignore_parameter_runme.ml

@@ -0,0 +1,20 @@
+open Swig
+open Ignore_parameter
+
+let _ =
+  assert (get_string (_jaguar (C_list [ C_int 200 ; C_float 0. ])) = "hello");
+  assert (get_int (_lotus (C_list [ C_string "fast" ; C_float 0. ])) = 101);
+  assert (get_float (_tvr (C_list [ C_string "fast" ; C_int 200 ])) = 8.8);
+  assert (get_int (_ferrari (C_void)) = 101);
+;;
+let sc = new_SportsCars (C_void)
+let _ =
+  assert (get_string ((invoke sc) "daimler" (C_list [ C_int 200 ; C_float 0. ])) = "hello");
+  assert (get_int ((invoke sc) "astonmartin" (C_list [ C_string "fast" ; C_float 0. ])) = 101);
+  assert (get_float ((invoke sc) "bugatti" (C_list [ C_string "fast" ; C_int 200 ])) = 8.8);
+  assert (get_int ((invoke sc) "lamborghini" (C_void)) = 101);
+;;
+let mc = new_MiniCooper (C_list [ C_int 200 ; C_float 0. ])
+let mm = new_MorrisMinor (C_list [ C_string "slow" ; C_float 0. ])
+let fa = new_FordAnglia (C_list [ C_string "slow" ; C_int 200 ])
+let aa = new_AustinAllegro (C_void)

Examples/test-suite/ocaml/li_std_vector_runme.ml

@@ -0,0 +1,23 @@
+open Swig
+open Li_std_vector
+
+let _ =
+  let iv = new_IntVector '() in
+  assert (iv -> "empty" () as bool);
+  assert ((iv -> "size" () as int) = 0);
+  ignore (iv -> "push_back" (123));
+  assert ((iv -> "empty" () as bool) = false);
+  assert ((iv -> "size" () as int) = 1);
+  assert ((iv -> "[]" (0) as int) = 123);
+  ignore (iv -> "clear" ());
+  assert (iv -> "empty" () as bool);
+  assert ((iv -> "size" () as int) = 0);
+;;
+
+let _ =
+  let rv = new_RealVector '() in
+  ignore (rv -> "push_back" (100.));
+  ignore (rv -> "push_back" (200.));
+  assert ((rv -> "[]" (0) as float) = 100.);
+  assert ((rv -> "[]" (1) as float) = 200.);
+;;

Examples/test-suite/ocaml/struct_value_runme.ml

@@ -0,0 +1,11 @@
+open Swig
+open Struct_value
+
+let b = new_Bar (C_void)
+let a = (invoke b) "[a]" (C_void)
+let _ = (invoke a) "[x]" (C_int 3)
+let _ = assert((invoke a) "[x]" (C_void) = C_int 3)
+
+let bb = (invoke b) "[b]" (C_void)
+let _ = (invoke bb) "[x]" (C_int 3)
+let _ = assert((invoke bb) "[x]" (C_void) = C_int 3)

Lib/ocaml/class.swg

@@ -56,6 +56,7 @@ begin
 	  (fun mth arg -> invoke_inner raw_ptr mth arg)
 end
 
+let _ = register_class_byname "$realname" create_$classname_from_ptr
 let _ = Callback.register 
           "create_$normalized_from_ptr"
           create_$classname_from_ptr

Lib/ocaml/swig.ml

@@ -155,5 +155,5 @@ let _ = Callback.register "swig_set_type_info" set_type_info
 let class_master_list = Hashtbl.create 20
 let register_class_byname nm co = 
   Hashtbl.replace class_master_list nm (Obj.magic co)
-let create_class nm arg = 
+let create_class nm =
   try (Obj.magic (Hashtbl.find class_master_list nm)) with _ -> raise (NoSuchClass nm)

Lib/ocaml/typemaps.i

@@ -27,7 +27,16 @@
     $result = caml_val_ptr($1,$descriptor);
 }
 
-#ifdef __cplusplus
+%typemap(in) char *& (char *temp) {
+  /* %typemap(in) char *& */
+  temp = (char*)caml_val_ptr($1,$descriptor);
+  $1 = &temp;
+}
+
+%typemap(argout) char *& {
+  /* %typemap(argout) char *& */
+  swig_result =	caml_list_append(swig_result,caml_val_string_len(*$1, strlen(*$1)));
+}
 
 %typemap(in) SWIGTYPE & {
     /* %typemap(in) SWIGTYPE & */
@@ -105,6 +114,8 @@
     $1 = *(($&1_ltype) caml_ptr_val($input,$&1_descriptor)) ;
 }
 
+#ifdef __cplusplus
+
 %typemap(out) SWIGTYPE {
     /* %typemap(out) SWIGTYPE */
     $&1_ltype temp = new $ltype((const $1_ltype &) $1);
@@ -116,23 +127,8 @@
     }
 }
 
-%typemap(in) char *& (char *temp) {
-  /* %typemap(in) char *& */
-  temp = (char*)caml_val_ptr($1,$descriptor);
-  $1 = &temp;
-}
-
-%typemap(argout) char *& {
-  /* %typemap(argout) char *& */
-  swig_result =	caml_list_append(swig_result,caml_val_string_len(*$1, strlen(*$1)));
-}
-
 #else
 
-%typemap(in) SWIGTYPE {
-    $1 = *(($&1_ltype) caml_ptr_val($input,$&1_descriptor)) ;
-}
-
 %typemap(out) SWIGTYPE {
     /* %typemap(out) SWIGTYPE */
     void *temp = calloc(1,sizeof($ltype));
@@ -145,9 +141,6 @@
     }
 }
 
-%apply SWIGTYPE { const SWIGTYPE & };
-%apply SWIGTYPE { const SWIGTYPE && };
-
 #endif
 
 /* The SIMPLE_MAP macro below defines the whole set of typemaps needed

Source/Modules/ocaml.cxx

@@ -417,6 +417,29 @@ public:
     return SwigType_isarray(SwigType_typedef_resolve_all(t));
   }
 
+  virtual int membervariableHandler(Node *n) {
+    String *symname = Getattr(n, "sym:name");
+    Language::membervariableHandler(n);
+
+    String *mname = Swig_name_member(NSPACE_TODO, classname, symname);
+    String *getname = Swig_name_get(NSPACE_TODO, mname);
+    String *mangled_getname = mangleNameForCaml(getname);
+    Delete(getname);
+
+    if (!GetFlag(n, "feature:immutable")) {
+      String *setname = Swig_name_set(NSPACE_TODO, mname);
+      String *mangled_setname = mangleNameForCaml(setname);
+      Delete(setname);
+      Printf(f_class_ctors, "    \"[%s]\", (fun args -> " "if args = (C_list [ raw_ptr ]) then _%s args else _%s args) ;\n", symname, mangled_getname, mangled_setname);
+      Delete(mangled_setname);
+    } else {
+      Printf(f_class_ctors, "    \"[%s]\", (fun args -> " "if args = (C_list [ raw_ptr ]) then _%s args else C_void) ;\n", symname, mangled_getname);
+    }
+    Delete(mangled_getname);
+    Delete(mname);
+    return SWIG_OK;
+  }
+
   /* ------------------------------------------------------------
    * functionWrapper()
    * Create a function declaration and register it with the interpreter.
@@ -477,26 +500,12 @@ public:
       Delete(mangled_name_nounder);
     } else if (classmode && in_destructor) {
       Printf(f_class_ctors, "    \"~\", %s ;\n", mangled_name);
-    } else if (classmode && !in_constructor && !in_destructor && !static_member_function) {
+    } else if (classmode && !in_constructor && !in_destructor && !static_member_function &&
+	    !Getattr(n, "membervariableHandler:sym:name")) {
       String *opname = Copy(Getattr(n, "memberfunctionHandler:sym:name"));
 
       Replaceall(opname, "operator ", "");
-
-      if (strstr(Char(mangled_name), "__get__")) {
-	String *set_name = Copy(mangled_name);
-	if (!GetFlag(n, "feature:immutable")) {
-	  Replaceall(set_name, "__get__", "__set__");
-	  Printf(f_class_ctors, "    \"%s\", (fun args -> " "if args = (C_list [ raw_ptr ]) then %s args else %s args) ;\n", opname, mangled_name, set_name);
-	  Delete(set_name);
-	} else {
-	  Printf(f_class_ctors, "    \"%s\", (fun args -> " "if args = (C_list [ raw_ptr ]) then %s args else C_void) ;\n", opname, mangled_name);
-	}
-      } else if (strstr(Char(mangled_name), "__set__")) {
-	;			/* Nothing ... handled by the case above */
-      } else {
-	Printf(f_class_ctors, "    \"%s\", %s ;\n", opname, mangled_name);
-      }
-
+      Printf(f_class_ctors, "    \"%s\", %s ;\n", opname, mangled_name);
       Delete(opname);
     }
 
@@ -1102,11 +1111,12 @@ public:
 
   int classHandler(Node *n) {
     String *name = Getattr(n, "name");
+    classname = Getattr(n, "sym:name");
 
     if (!name)
       return SWIG_OK;
 
-    String *mangled_sym_name = mangleNameForCaml(name);
+    String *mangled_name = mangleNameForCaml(name);
     String *this_class_def = NewString(f_classtemplate);
     String *name_normalized = normalizeTemplatedClassName(name);
     String *old_class_ctors = f_class_ctors;
@@ -1115,7 +1125,6 @@ public:
     bool sizeof_feature = generate_sizeof && isSimpleType(name);
 
 
-    classname = mangled_sym_name;
     classmode = true;
     int rv = Language::classHandler(n);
     classmode = false;
@@ -1123,15 +1132,15 @@ public:
     if (sizeof_feature) {
       Printf(f_wrappers,
 	     "SWIGEXT CAML_VALUE _wrap_%s_sizeof( CAML_VALUE args ) {\n"
-	     "    CAMLparam1(args);\n" "    CAMLreturn(Val_int(sizeof(%s)));\n" "}\n", mangled_sym_name, name_normalized);
+	     "    CAMLparam1(args);\n" "    CAMLreturn(Val_int(sizeof(%s)));\n" "}\n", mangled_name, name_normalized);
 
-      Printf(f_mlbody, "external __%s_sizeof : unit -> int = " "\"_wrap_%s_sizeof\"\n", classname, mangled_sym_name);
+      Printf(f_mlbody, "external __%s_sizeof : unit -> int = " "\"_wrap_%s_sizeof\"\n", mangled_name, mangled_name);
     }
 
 
     /* Insert sizeof operator for concrete classes */
     if (sizeof_feature) {
-      Printv(f_class_ctors, "\"sizeof\" , (fun args -> C_int (__", classname, "_sizeof ())) ;\n", NIL);
+      Printv(f_class_ctors, "\"sizeof\" , (fun args -> C_int (__", mangled_name, "_sizeof ())) ;\n", NIL);
     }
     /* Handle up-casts in a nice way */
     List *baselist = Getattr(n, "bases");
@@ -1150,7 +1159,7 @@ public:
       }
     }
 
-    Replaceall(this_class_def, "$classname", classname);
+    Replaceall(this_class_def, "$classname", mangled_name);
     Replaceall(this_class_def, "$normalized", name_normalized);
     Replaceall(this_class_def, "$realname", name);
     Replaceall(this_class_def, "$baselist", base_classes);
@@ -1163,7 +1172,7 @@ public:
 
     Multiwrite(this_class_def);
 
-    Setattr(n, "ocaml:ctor", classname);
+    Setattr(n, "ocaml:ctor", mangled_name);
 
     return rv;
   }