codegen: Actually free data when using "remove(_all)" on GLib.Queue/(S)List

When using e.g. GLib.List.remove() there is no context/feedback whether an
item was removed or needed manual free'ing.

Replace such calls with custom wrappers where items required free'ing if
they were found.

Fixes https://gitlab.gnome.org/GNOME/vala/issues/1238

diff --git a/codegen/valaccodemethodcallmodule.vala b/codegen/valaccodemethodcallmodule.vala
index 7154c18df2c8b31ab1fbd59666199d47dda4d008..914458611d52d68ff89ea5a8200ca03551ca3c7d 100644 (file)
--- a/codegen/valaccodemethodcallmodule.vala
+++ b/codegen/valaccodemethodcallmodule.vala
@@ -826,6 +826,28 @@ public class Vala.CCodeMethodCallModule : CCodeAssignmentModule {
 
                }
 
+               // Transform and add free function argument to GLib.[List,Queue,SList].remove[_all] calls
+               unowned DataType? collection_type = null;
+               if (ma != null && ma.inner != null) {
+                       collection_type = ma.inner.value_type;
+               }
+               if (collection_type != null
+                   && (collection_type.type_symbol == glist_type || collection_type.type_symbol == gslist_type || collection_type.type_symbol == gqueue_type)
+                   && (ma.member_name == "remove" || ma.member_name == "remove_all")
+                   //FIXME Perform stricter type argument check earlier
+                   && collection_type.check_type_arguments (context)) {
+                       var remove_method = (Method) collection_type.type_symbol.scope.lookup (ma.member_name + "_full");
+                       var type_arg = collection_type.get_type_arguments ()[0];
+                       if (remove_method != null && requires_destroy (type_arg)) {
+                               // only add them once per source file
+                               if (add_generated_external_symbol (remove_method)) {
+                                       visit_method (remove_method);
+                               }
+                               ccall.call = new CCodeIdentifier (get_ccode_name (remove_method));
+                               ccall.add_argument (get_destroy0_func_expression (type_arg));
+                       }
+               }
+
                if (return_result_via_out_param) {
                        ccode.add_expression (ccall_expr);
                        ccall_expr = out_param_ref;

diff --git a/tests/Makefile.am b/tests/Makefile.am
index c7d982a507f4cceefaf375c1e1270936aa59818a..c40ec0c75d71c64858bfe5d9c06c7bbf8e8044a4 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -58,6 +58,7 @@ TESTS = \
        basic-types/sizeof.vala \
        basic-types/garray.vala \
        basic-types/glists.vala \
+       basic-types/glists_remove.vala \
        basic-types/gptrarray.vala \
        basic-types/gvariants.vala \
        basic-types/gvariants-hashtable-missing-type-arguments.test \

diff --git a/tests/basic-types/glists_remove.vala b/tests/basic-types/glists_remove.vala
new file mode 100644 (file)
index 0000000..8770f60
--- /dev/null
+++ b/tests/basic-types/glists_remove.vala
@@ -0,0 +1,113 @@
+class Foo : Object {
+}
+
+void test_glist () {
+       {
+               var list = new GLib.List<Foo> ();
+               var foo = new Foo ();
+               list.append (foo);
+               assert (list.length () == 1);
+               assert (foo.ref_count == 2);
+               list.remove (foo);
+               assert (list.length () == 0);
+               assert (foo.ref_count == 1);
+       }
+       {
+               var list = new GLib.List<Foo> ();
+               var foo = new Foo ();
+               list.append (foo);
+               list.append (foo);
+               assert (list.length () == 2);
+               assert (foo.ref_count == 3);
+               list.remove_all (foo);
+               assert (list.length () == 0);
+               assert (foo.ref_count == 1);
+       }
+       {
+               var list = new GLib.List<unowned string> ();
+               unowned var s = "foo";
+               list.append (s);
+               assert (list.length () == 1);
+               list.remove (s);
+               assert (list.length () == 0);
+               list.append (s);
+               list.remove_all (s);
+               assert (list.length () == 0);
+       }
+}
+
+void test_gslist () {
+       {
+               var list = new GLib.SList<Foo> ();
+               var foo = new Foo ();
+               list.append (foo);
+               assert (list.length () == 1);
+               assert (foo.ref_count == 2);
+               list.remove (foo);
+               assert (list.length () == 0);
+               assert (foo.ref_count == 1);
+       }
+       {
+               var list = new GLib.SList<Foo> ();
+               var foo = new Foo ();
+               list.append (foo);
+               list.append (foo);
+               assert (list.length () == 2);
+               assert (foo.ref_count == 3);
+               list.remove_all (foo);
+               assert (list.length () == 0);
+               assert (foo.ref_count == 1);
+       }
+       {
+               var list = new GLib.SList<unowned string> ();
+               unowned var s = "foo";
+               list.append (s);
+               assert (list.length () == 1);
+               list.remove (s);
+               assert (list.length () == 0);
+               list.append (s);
+               list.remove_all (s);
+               assert (list.length () == 0);
+       }
+}
+
+void test_gqueue () {
+       {
+               var queue = new GLib.Queue<Foo> ();
+               var foo = new Foo ();
+               queue.push_head (foo);
+               assert (queue.length == 1);
+               assert (foo.ref_count == 2);
+               queue.remove (foo);
+               assert (queue.length == 0);
+               assert (foo.ref_count == 1);
+       }
+       {
+               var queue = new GLib.Queue<Foo> ();
+               var foo = new Foo ();
+               queue.push_head (foo);
+               queue.push_head (foo);
+               assert (queue.length == 2);
+               assert (foo.ref_count == 3);
+               queue.remove_all (foo);
+               assert (queue.length == 0);
+               assert (foo.ref_count == 1);
+       }
+       {
+               var queue = new GLib.Queue<unowned string> ();
+               unowned var s = "foo";
+               queue.push_head (s);
+               assert (queue.length == 1);
+               queue.remove (s);
+               assert (queue.length == 0);
+               queue.push_head (s);
+               queue.remove_all (s);
+               assert (queue.length == 0);
+       }
+}
+
+void main () {
+       test_glist ();
+       test_gslist ();
+       test_gqueue ();
+}

diff --git a/vapi/glib-2.0.vapi b/vapi/glib-2.0.vapi
index 00153e31c9d73a08ac3da10cd155f3e79a5fd0b1..373877f2277d66b8a2d776bebc7a38fba15c8886 100644 (file)
--- a/vapi/glib-2.0.vapi
+++ b/vapi/glib-2.0.vapi
@@ -4893,12 +4893,42 @@ namespace GLib {
                public void insert_sorted (owned G data, CompareFunc<G> compare_func);
                [ReturnsModifiedPointer ()]
                public void remove (G data);
+               [CCode (cname = "vala_g_list_remove_full")]
+               [ReturnsModifiedPointer ()]
+               public unowned List<G> remove_full (G data, FreeFunc? func) {
+                       unowned List<G>? l = this;
+                       while (l != null) {
+                               if (((!) l).data != data) {
+                                       l = ((!) l).next;
+                               } else {
+                                       func (((!) l).data);
+                                       delete_link ((!) l);
+                                       break;
+                               }
+                       }
+                       return this;
+               }
                [ReturnsModifiedPointer ()]
                public void remove_link (List<G> llink);
                [ReturnsModifiedPointer ()]
                public void delete_link (List<G> link_);
                [ReturnsModifiedPointer ()]
                public void remove_all (G data);
+               [CCode (cname = "vala_g_list_remove_all_full")]
+               [ReturnsModifiedPointer ()]
+               public unowned List<G> remove_all_full (G data, FreeFunc? func) {
+                       unowned List<G>? l = this;
+                       while (l != null) {
+                               if (((!) l).data != data) {
+                                       l = ((!) l).next;
+                               } else {
+                                       func (((!) l).data);
+                                       delete_link ((!) l);
+                                       l = this;
+                               }
+                       }
+                       return this;
+               }
 
                public uint length ();
                public List<unowned G> copy ();
@@ -4955,12 +4985,42 @@ namespace GLib {
                public void insert_sorted (owned G data, CompareFunc<G> compare_func);
                [ReturnsModifiedPointer ()]
                public void remove (G data);
+               [CCode (cname = "vala_g_slist_remove_full")]
+               [ReturnsModifiedPointer ()]
+               public unowned SList<G> remove_full (G data, FreeFunc? func) {
+                       unowned SList<G>? l = this;
+                       while (l != null) {
+                               if (((!) l).data != data) {
+                                       l = ((!) l).next;
+                               } else {
+                                       func (((!) l).data);
+                                       delete_link ((!) l);
+                                       break;
+                               }
+                       }
+                       return this;
+               }
                [ReturnsModifiedPointer ()]
                public void remove_link (SList<G> llink);
                [ReturnsModifiedPointer ()]
                public void delete_link (SList<G> link_);
                [ReturnsModifiedPointer ()]
                public void remove_all (G data);
+               [CCode (cname = "vala_g_slist_remove_all_full")]
+               [ReturnsModifiedPointer ()]
+               public unowned SList<G> remove_all_full (G data, FreeFunc? func) {
+                       unowned SList<G>? l = this;
+                       while (l != null) {
+                               if (((!) l).data != data) {
+                                       l = ((!) l).next;
+                               } else {
+                                       func (((!) l).data);
+                                       delete_link ((!) l);
+                                       l = this;
+                               }
+                       }
+                       return this;
+               }
 
                public uint length ();
                public SList<unowned G> copy ();
@@ -5054,8 +5114,33 @@ namespace GLib {
                public int index (G data);
                [Version (since = "2.4")]
                public bool remove (G data);
+               [CCode (cname = "vala_g_queue_remove_full")]
+               public bool remove_full (G data, FreeFunc? func) {
+                       unowned List<G>? l = head.find (data);
+                       if (l != null) {
+                               func (((!) l).data);
+                               delete_link ((!) l);
+                               return true;
+                       } else {
+                               return false;
+                       }
+               }
                [Version (since = "2.4")]
                public uint remove_all (G data);
+               [CCode (cname = "vala_g_queue_remove_all_full")]
+               public uint remove_all_full (G data, FreeFunc? func) {
+                       var old_length = length;
+                       unowned List<G>? l = head;
+                       while (l != null) {
+                               unowned List<G>? next = ((!) l).next;
+                               if (((!) l).data == data) {
+                                       func (((!) l).data);
+                                       delete_link ((!) l);
+                               }
+                               l = next;
+                       }
+                       return old_length - length;
+               }
                [Version (since = "2.4")]
                public void delete_link (List<G> link);
                [Version (since = "2.4")]