GNOME Bugzilla – Bug 795025
g_object_new* not consistent with return value transfer annotation
Last modified: 2018-04-10 11:58:43 UTC
g_object_new, g_object_newv and g_object_new_with_properties have the annotation (transfer full) on their return value. They return a reference for the caller to assume ownership of if the class of the new object is not derived from GInitiallyUnowned. However, if the class is derived from GInitiallyUnowned, they do not return a reference for the caller to assume ownership of --- they return a floating reference unless the class is also derived from GtkWindow in which case no reference is returned --- so the annotation (transfer none) is required. I don't think there is any annotation that can generally describe the transfer mode of the return value for these functions. This behaviour is surely expected of g_object_new so perhaps the documentation should be fixed. The transfer annotation may as well be removed since it isn't introspectable. This behaviour of g_object_newv has been recognised - here is an example of code to work around it: https://gitlab.gnome.org/GNOME/gtk/blob/3.93.0/gtk/gtkbuilder.c#L776 The intention of g_object_new_with_properties seems to be to provide an introspectable replacement for g_object_newv. If so, shouldn't this work-around code actually be part of g_object_new_with_properties to ensure that a reference is always returned for the caller to assume ownership of, to be consistent with (transfer full)?
The fact that g_object_new_with_properties is so easily bindable is double-edged sword. For example, gi-haskell bindings once use it manually and handle the case GInitiallyUnowned as necessary, but they also contain auto-generated bindings for it that are incorrect for reasons described here. Maybe there is a space for yet another variant of g_object_new_* that clearly indicates ownership aspect in its name and takes into account GInitiallyUnowned.
It does look like another variant is required that is actually introspectable. Perhaps its name should have the suffix '_ref' to indicate that the returns object with a reference that the caller can assume ownership of. I think g_object_new_with_properties is still useful in C because it provides a non-varargs variant of g_object_new so, for example, enables an object with no properties to be created without varargs overhead.
Created attachment 370725 [details] [review] Add g_object_newv_ref, an introspectable variant of g_object_new_with_properties Here's a patch to add a function g_object_newv_ref which is an introspectable variant of g_object_new_with_properties. The function name is based on 'newv' because it has similar arguments to g_object_setv and g_object_getv. It has the suffix '_ref' because the returned object always has a reference that the caller assumes ownership of.
See also: https://gitlab.gnome.org/GNOME/gobject-introspection/issues/54 I really don’t want to add another variant of g_object_new(), especially since GInitiallyUnowned is a bit of a corner case — people should be calling more specific constructors to create GInitiallyUnowned subclass instances. Since g_object_new() is a core, low-level part of GObject, bindings are basically expected to wrap it manually already, so should be able to handle floating refs from it explicitly (as GtkBuilder and gi-haskell already do). ⇒ WONTFIX
Created attachment 370730 [details] [review] Mark g_object_new_with_properties as non-introspectable. Could we mark it as skip to clearly indicate the need for manual binding? Otherwise some language bindings will either: generate it automatically with implementation that won't work correctly with GInitiallyUnowned or have to ignore it manually. Attached patch does that and generates following diff in gir: ``` --- GObject-2.0.gir 2018-04-10 12:54:20.476553919 +0200 +++ GObject-2.0.gir.new 2018-04-10 13:13:16.576540767 +0200 @@ -3902,6 +3902,3 @@ to the #GObject implementation and should never be accessed directly.</doc> - <constructor name="new" - c:identifier="g_object_new" - shadowed-by="new_with_properties" - introspectable="0"> + <constructor name="new" c:identifier="g_object_new" introspectable="0"> <doc xml:space="preserve">Creates a new instance of a #GObject subtype and sets its properties. @@ -3960,4 +3957,4 @@ c:identifier="g_object_new_with_properties" - shadows="new" - version="2.54"> + version="2.54" + introspectable="0"> <doc xml:space="preserve">Creates a new instance of a #GObject subtype and sets its properties using ```
Review of attachment 370730 [details] [review]: Fine. This change might upset the people from bug #709865 who have been using GObject.new_with_properties() from Python; but pygobject can always wrap it manually.
Pushed to master.