[vala/staging] codegen: Free generic elements of glib collections

[Rico Tzschichholz <ricotz src gnome org>]

commit d5265f71016c56bd234130b83cbc9dcc666af415
Author: Rico Tzschichholz <ricotz ubuntu com>
Date:   Tue Mar 20 10:29:05 2018 +0100

    codegen: Free generic elements of glib collections
    
    It needs to be possible to use parameters or fields/properties which hold
    dup/free functions for a generic type in scope.
    
    This required to make the destroy_func being a parameter with the benefit
    of being able to use g_*_free_all directly and adding a _g_node_free_all
    wrapper with a compatible signature.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=694765

 codegen/valaccodebasemodule.vala |  103 ++++++++++++++++++++++---------------
 tests/Makefile.am                |    2 +
 tests/generics/bug694765-1.vala  |   21 ++++++++
 tests/generics/bug694765-2.vala  |   42 +++++++++++++++
 4 files changed, 126 insertions(+), 42 deletions(-)
---
diff --git a/codegen/valaccodebasemodule.vala b/codegen/valaccodebasemodule.vala
index 9f02b8c..12ea2ef 100644
--- a/codegen/valaccodebasemodule.vala
+++ b/codegen/valaccodebasemodule.vala
@@ -3156,8 +3156,16 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
                                }
                        }
                        
-                       if (elements_require_free && element_destroy_func_expression is CCodeIdentifier) {
-                               return new CCodeIdentifier (generate_collection_free_wrapper (type, 
(CCodeIdentifier) element_destroy_func_expression));
+                       if (elements_require_free) {
+                               if (element_destroy_func_expression is CCodeIdentifier || 
element_destroy_func_expression is CCodeMemberAccess) {
+                                       var ccall = new CCodeFunctionCall (new CCodeIdentifier 
(generate_collection_free_wrapper (type)));
+                                       // adding second argument early, instance parameter will be inserted 
by destroy_value()
+                                       ccall.add_argument (element_destroy_func_expression);
+                                       return ccall;
+                               } else {
+                                       Report.error (null, "internal error: No useable 
element_destroy_function found");
+                                       return null;
+                               }
                        } else {
                                return new CCodeIdentifier (get_ccode_free_function (type.data_type));
                        }
@@ -3242,32 +3250,38 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
                }
        }
 
-       private string generate_collection_free_wrapper (DataType collection_type, CCodeIdentifier 
element_destroy_func_expression) {
-               string destroy_func = "_%s_%s".printf (get_ccode_free_function (collection_type.data_type), 
element_destroy_func_expression.name);
+       private string generate_collection_free_wrapper (DataType collection_type) {
+               string destroy_func;
 
-               if (!add_wrapper (destroy_func)) {
-                       // wrapper already defined
-                       return destroy_func;
-               }
+               if (collection_type.data_type == gnode_type) {
+                       destroy_func = "_g_node_free_all";
+                       if (!add_wrapper (destroy_func)) {
+                               // wrapper already defined
+                               return destroy_func;
+                       }
 
-               var function = new CCodeFunction (destroy_func, "void");
-               function.add_parameter (new CCodeParameter ("self", get_ccode_name (collection_type)));
+                       var function = new CCodeFunction (destroy_func, "void");
+                       function.add_parameter (new CCodeParameter ("self", get_ccode_name 
(collection_type)));
+                       function.add_parameter (new CCodeParameter ("free_func", "GDestroyNotify"));
 
-               push_function (function);
+                       push_function (function);
 
-               if (collection_type.data_type == gnode_type) {
                        CCodeFunctionCall element_free_call;
-                       /* A wrapper which converts GNodeTraverseFunc into GDestroyNotify */
                        string destroy_node_func = "%s_node".printf (destroy_func);
                        var wrapper = new CCodeFunction (destroy_node_func, "gboolean");
                        wrapper.modifiers = CCodeModifiers.STATIC;
                        wrapper.add_parameter (new CCodeParameter ("node", get_ccode_name (collection_type)));
-                       wrapper.add_parameter (new CCodeParameter ("unused", "gpointer"));
+                       wrapper.add_parameter (new CCodeParameter ("free_func", "GDestroyNotify"));
                        push_function (wrapper);
 
-                       var free_call = new CCodeFunctionCall (element_destroy_func_expression);
-                       free_call.add_argument (new CCodeMemberAccess.pointer(new CCodeIdentifier("node"), 
"data"));
-                       ccode.add_expression (free_call);
+                       var free_call = new CCodeFunctionCall (new CCodeIdentifier ("free_func"));
+                       free_call.add_argument (new CCodeMemberAccess.pointer (new CCodeIdentifier("node"), 
"data"));
+
+                       var data_isnull = new CCodeBinaryExpression (CCodeBinaryOperator.EQUALITY, new 
CCodeMemberAccess.pointer (new CCodeIdentifier("node"), "data"), new CCodeConstant ("NULL"));
+                       var ccomma_data = new CCodeCommaExpression ();
+                       ccomma_data.append_expression (new CCodeConditionalExpression (data_isnull, new 
CCodeConstant ("NULL"), free_call));
+                       ccode.add_expression (ccomma_data);
+
                        ccode.add_return (new CCodeConstant ("FALSE"));
 
                        pop_function ();
@@ -3280,37 +3294,34 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
                        element_free_call.add_argument (new CCodeConstant ("G_POST_ORDER"));
                        element_free_call.add_argument (new CCodeConstant ("G_TRAVERSE_ALL"));
                        element_free_call.add_argument (new CCodeConstant ("-1"));
-                       element_free_call.add_argument (new CCodeIdentifier (destroy_node_func));
-                       element_free_call.add_argument (new CCodeConstant ("NULL"));
-                       ccode.add_expression (element_free_call);
+                       element_free_call.add_argument (new CCodeCastExpression (new CCodeIdentifier 
(destroy_node_func), "GNodeTraverseFunc"));
+                       element_free_call.add_argument (new CCodeIdentifier ("free_func"));
+
+                       var free_func_isnull = new CCodeBinaryExpression (CCodeBinaryOperator.EQUALITY, new 
CCodeIdentifier ("free_func"), new CCodeConstant ("NULL"));
+                       var ccomma = new CCodeCommaExpression ();
+                       ccomma.append_expression (new CCodeConditionalExpression (free_func_isnull, new 
CCodeConstant ("NULL"), element_free_call));
+
+                       ccode.add_expression (ccomma);
 
                        var cfreecall = new CCodeFunctionCall (new CCodeIdentifier (get_ccode_free_function 
(gnode_type)));
                        cfreecall.add_argument (new CCodeIdentifier ("self"));
                        ccode.add_expression (cfreecall);
 
                        function.modifiers = CCodeModifiers.STATIC;
-               } else {
-                       CCodeFunctionCall collection_free_call;
-                       if (collection_type.data_type == glist_type) {
-                               collection_free_call = new CCodeFunctionCall (new CCodeIdentifier 
("g_list_free_full"));
-                       } else if (collection_type.data_type == gslist_type) {
-                               collection_free_call = new CCodeFunctionCall (new CCodeIdentifier 
("g_slist_free_full"));
-                       } else {
-                               collection_free_call = new CCodeFunctionCall (new CCodeIdentifier 
("g_queue_free_full"));
-                       }
-
-                       collection_free_call.add_argument (new CCodeIdentifier ("self"));
-                       collection_free_call.add_argument (new CCodeCastExpression 
(element_destroy_func_expression, "GDestroyNotify"));
-                       ccode.add_expression (collection_free_call);
+                       pop_function ();
 
-                       function.modifiers = CCodeModifiers.STATIC | CCodeModifiers.INLINE;
+                       cfile.add_function_declaration (function);
+                       cfile.add_function (function);
+               } else if (collection_type.data_type == glist_type) {
+                       destroy_func = "g_list_free_full";
+               } else if (collection_type.data_type == gslist_type) {
+                       destroy_func = "g_slist_free_full";
+               } else if (collection_type.data_type == gqueue_type) {
+                       destroy_func = "g_queue_free_full";
+               } else {
+                       assert_not_reached ();
                }
 
-               pop_function ();
-
-               cfile.add_function_declaration (function);
-               cfile.add_function (function);
-
                return destroy_func;
        }
 
@@ -3359,7 +3370,14 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
                        return ccomma;
                }
 
-               var ccall = new CCodeFunctionCall (get_destroy_func_expression (type));
+               bool is_gcollection = (type.data_type == glist_type || type.data_type == gslist_type || 
type.data_type == gnode_type || type.data_type == gqueue_type);
+               CCodeFunctionCall ccall;
+               var cexpr = get_destroy_func_expression (type);
+               if (is_gcollection) {
+                       ccall = (CCodeFunctionCall) cexpr;
+               } else {
+                       ccall = new CCodeFunctionCall (cexpr);
+               }
 
                if (type is ValueType && !type.nullable) {
                        // normal value type, no null check
@@ -3395,7 +3413,7 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
                        }
                }
 
-               if (ccall.call is CCodeIdentifier && !(type is ArrayType) && !is_macro_definition) {
+               if (!is_gcollection && ccall.call is CCodeIdentifier && !(type is ArrayType) && 
!is_macro_definition) {
                        // generate and use NULL-aware free macro to simplify code
 
                        var freeid = (CCodeIdentifier) ccall.call;
@@ -3431,7 +3449,8 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
                        cisnull = new CCodeBinaryExpression (CCodeBinaryOperator.OR, cisnull, cunrefisnull);
                }
 
-               ccall.add_argument (cvar);
+               // glib collections already have the free_func argument, so make sure the instance parameter 
gets first
+               ccall.insert_argument (0, cvar);
 
                /* set freed references to NULL to prevent further use */
                var ccomma = new CCodeCommaExpression ();
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f0f49ce..69dbd0e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -354,6 +354,8 @@ TESTS = \
        asynchronous/closures.vala \
        asynchronous/generator.vala \
        asynchronous/yield.vala \
+       generics/bug694765-1.vala \
+       generics/bug694765-2.vala \
        dbus/basic-types.test \
        dbus/arrays.test \
        dbus/structs.test \
diff --git a/tests/generics/bug694765-1.vala b/tests/generics/bug694765-1.vala
new file mode 100644
index 0000000..f723216
--- /dev/null
+++ b/tests/generics/bug694765-1.vala
@@ -0,0 +1,21 @@
+List<G> copy_list<G> (List<G> list) {
+       var result = new List<G> ();
+
+       foreach (var item in list)
+               result.prepend (item);
+       result.reverse ();
+
+       return result;
+}
+
+void main () {
+       List<string> list = new List<string> ();
+       list.prepend ("foo");
+
+       var copy = copy_list (list);
+       list = null;
+
+       assert (copy.nth_data (0) == "foo");
+
+       copy = null;
+}
diff --git a/tests/generics/bug694765-2.vala b/tests/generics/bug694765-2.vala
new file mode 100644
index 0000000..2e228e8
--- /dev/null
+++ b/tests/generics/bug694765-2.vala
@@ -0,0 +1,42 @@
+class Foo : GLib.Object {
+}
+
+class Bar<G> : GLib.Object {
+       GLib.List<G> list;
+
+       construct {
+               list = new GLib.List<G> ();
+       }
+
+       public void add (G item) {
+               list.append (item);
+       }
+}
+
+class Baz<G> : GLib.Object {
+       GLib.Node<G> node;
+
+       construct {
+               node = new GLib.Node<G> ();
+       }
+
+       public void add (G item) {
+               node.append_data (item);
+       }
+}
+
+void main () {
+       var foo = new Foo ();
+
+       var bar = new Bar<Foo> ();
+       bar.add (foo);
+       assert (foo.ref_count == 2);
+       bar = null;
+       assert (foo.ref_count == 1);
+
+       var baz = new Baz<Foo> ();
+       baz.add (foo);
+       assert (foo.ref_count == 2);
+       baz = null;
+       assert (foo.ref_count == 1);
+}