GNOME Bugzilla – Bug 644749
support setting gobjects in struct fields
Last modified: 2015-02-07 16:52:50 UTC
Right now if you try to set a gobject as a structure field we get an error that it is not supported. This patch allows us to set a gobject for structures such as GdkWindowAttrs.
Created attachment 183370 [details] [review] support setting gobjects in struct fields
How would memory management work here? Is it typically intended that the struct is then passed down to native code, which takes a reference? Otherwise, the struct field validity is tied to the GC of the runtime, and if it happens to go out of scope, you will have an invalid field. I don't know if practically speaking that's a problem for the uses in GTK, but I'm just pointing out that in the *general* case, it's not safe.
Review of attachment 183370 [details] [review]: ::: girepository/gifieldinfo.c @@ +521,3 @@ + switch (g_base_info_get_type (interface)) + { + case GI_INFO_TYPE_OBJECT: Add case GI_INFO_TYPE_INTERFACE: here too for fields which are interfaces.
In the Gdk case at least you are passing in a struct that is just a simple placeholder which is there I'm guessing so you don't have to have a huge parameter list. For the most part I don't like structs and think they shouldn't be used but this is needed to support full subclassing of widgets. Without the ability to set the visual of a WindowAttr we actually crash as the visual is set to NULL.
I think if we want to support subclassing in bindings fully, hacks like this are not the right way. Instead, we'll have to review what apis are currently used for that, and make sure that they are bindable or replace them by bindable ones.
(In reply to comment #4) > In the Gdk case at least you are passing in a struct that is just a simple > placeholder which is there I'm guessing so you don't have to have a huge > parameter list. For the most part I don't like structs and think they > shouldn't be used but this is needed to support full subclassing of widgets. > Without the ability to set the visual of a WindowAttr we actually crash as the > visual is set to NULL. Can you point me to some sample code?
Code I am trying to make work: https://bugzilla.gnome.org/attachment.cgi?id=183546 The other option here is to add accessors in Gdk or to fix how we do subclassing as Matthias says. Fixing subclassing would be ideal but only issue is I am loath to make major changes to Gtk because at some point I have to stop acting like I'm still part of the desktop team and actually do some work for Fedora. At the same time I don't want to make more work for you guys. It is just that I keep digging deeper and there is something else that needs to be fixed for introspection.
(In reply to comment #7) > Code I am trying to make work: > > https://bugzilla.gnome.org/attachment.cgi?id=183546 Okay, so in this case, the visual reference is held by the parent class, and we're just copying it. This will be a safe use. Please commit after the above fix.
Committed with fix. Thanks
(In reply to comment #5) > I think if we want to support subclassing in bindings fully, hacks like this > are not the right way. Instead, we'll have to review what apis are currently > used for that, and make sure that they are bindable or replace them by bindable > ones. Agreed, one way of doing that is changing GdkWindowAttr into construct only properties of GdkWindow.
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]