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

diff --git a/codegen/valaccodebasemodule.vala b/codegen/valaccodebasemodule.vala
index 3de943864a9c9c7d9f4d3829d23ba53dc54bbecd..0d8629e675e3b81296cae1350b0bc8904284bec6 100644 (file)
--- a/codegen/valaccodebasemodule.vala
+++ b/codegen/valaccodebasemodule.vala
@@ -3147,17 +3147,30 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
                        // create wrapper function to free list elements if necessary
 
                        bool elements_require_free = false;
+                       bool generic_elements = false;
                        CCodeExpression element_destroy_func_expression = null;
 
                        foreach (DataType type_arg in type.get_type_arguments ()) {
                                elements_require_free = requires_destroy (type_arg);
                                if (elements_require_free) {
                                        element_destroy_func_expression = get_destroy0_func_expression (type_arg);
+                                       generic_elements = (type_arg is GenericType);
                                }
                        }
                        
-                       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) {
+                               CCodeExpression? cexpr = null;
+                               if (element_destroy_func_expression is CCodeIdentifier || element_destroy_func_expression is CCodeMemberAccess) {
+                                       cexpr = new CCodeIdentifier (generate_collection_free_wrapper (type, (generic_elements ? null : element_destroy_func_expression as CCodeIdentifier)));
+                                       if (generic_elements) {
+                                               // adding second argument early, instance parameter will be inserted by destroy_value()
+                                               cexpr = new CCodeFunctionCall (cexpr);
+                                               ((CCodeFunctionCall) cexpr).add_argument (element_destroy_func_expression);
+                                       }
+                               } else {
+                                       Report.error (null, "internal error: No useable element_destroy_function found");
+                               }
+                               return cexpr;
                        } else {
                                return new CCodeIdentifier (get_ccode_free_function (type.data_type));
                        }
@@ -3242,32 +3255,47 @@ 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, CCodeIdentifier? element_destroy_func_expression) {
+               string destroy_func;
 
-               if (!add_wrapper (destroy_func)) {
-                       // wrapper already defined
-                       return destroy_func;
+               string? destroy_func_wrapper = null;
+               if (element_destroy_func_expression != null) {
+                       destroy_func_wrapper = "_%s_%s".printf (get_ccode_free_function (collection_type.data_type), element_destroy_func_expression.name);
+                       if (!add_wrapper (destroy_func_wrapper)) {
+                               // wrapper already defined
+                               return destroy_func_wrapper;
+                       }
                }
 
-               var function = new CCodeFunction (destroy_func, "void");
-               function.add_parameter (new CCodeParameter ("self", get_ccode_name (collection_type)));
+               if (collection_type.data_type == gnode_type) {
+                       destroy_func = "_g_node_free_all";
+                       if (!add_wrapper (destroy_func)) {
+                               // wrapper already defined
+                               return destroy_func;
+                       }
 
-               push_function (function);
+                       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);
 
-               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,36 +3308,54 @@ 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;
+                       pop_function ();
+
+                       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 {
-                       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"));
-                       }
+                       Report.error (null, "internal error: type of collection not supported");
+                       return "";
+               }
+
+               if (element_destroy_func_expression != null) {
+                       var function = new CCodeFunction (destroy_func_wrapper, "void");
+                       function.add_parameter (new CCodeParameter ("self", get_ccode_name (collection_type)));
+
+                       push_function (function);
 
+                       var collection_free_call = new CCodeFunctionCall (new CCodeIdentifier (destroy_func));
                        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);
 
                        function.modifiers = CCodeModifiers.STATIC | CCodeModifiers.INLINE;
-               }
+                       pop_function ();
 
-               pop_function ();
+                       cfile.add_function_declaration (function);
+                       cfile.add_function (function);
 
-               cfile.add_function_declaration (function);
-               cfile.add_function (function);
+                       return destroy_func_wrapper;
+               }
 
                return destroy_func;
        }
@@ -3359,7 +3405,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 && cexpr is CCodeFunctionCall) {
+                       ccall = (CCodeFunctionCall) cexpr;
+               } else {
+                       ccall = new CCodeFunctionCall (cexpr);
+               }
 
                if (type is ValueType && !type.nullable) {
                        // normal value type, no null check
@@ -3395,7 +3448,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 +3484,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 f0f49ce75b3e9aa6bbe30d101ff9f39967eb8ec2..2f3490d0f2a21b8e6975008a6a221dd682086abb 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -354,6 +354,9 @@ TESTS = \
        asynchronous/closures.vala \
        asynchronous/generator.vala \
        asynchronous/yield.vala \
+       generics/bug694765-1.vala \
+       generics/bug694765-2.vala \
+       generics/bug694765-3.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 (file)
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 (file)
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);
+}

diff --git a/tests/generics/bug694765-3.vala b/tests/generics/bug694765-3.vala
new file mode 100644 (file)
index 0000000..92ccc87
--- /dev/null
+++ b/tests/generics/bug694765-3.vala
@@ -0,0 +1,4 @@
+void main () {
+       var table = new HashTable<string, List<string>> (str_hash, str_equal);
+       table = null;
+}