spk121 / guile-gi

Bindings for GObject Introspection and libgirepository for Guile
GNU General Public License v3.0
58 stars 7 forks source link

container:get-children doesn't work #98

Closed daym closed 3 years ago

daym commented 3 years ago

(main.scm:19003): GuileGI-ERROR **: 19:16:38.914: unhandled argument type 'pointer to gpointer' src/gig_argument.c:1769

LordYuuma commented 3 years ago

I'm not sure, whether we have enough information to actually handle this meaningfully. The real type would be GList<GtkWidget*>*, right?

daym commented 3 years ago

The real type would be GList<GtkWidget>, right?

Yes. (In order to find this out I checked the parameter type of parameter widget of gtk_container_add)

C just doesn't have generics, so it cannot represent GList<GtkWidget>. Hence this weirdness (it uses gpointer for the member internally--like C would use void* in these cases for the same reason).

Note that people can and do store numbers directly as elements into GLists (see https://developer.gnome.org/glib/stable/glib-Type-Conversion-Macros.html#GINT-TO-POINTER:CAPS ). So it is not the case that one can assume that a GList always contains objects.

Therefore, the GIR contains the member type:

<class name="Container" [...]>
     <method name="get_children" c:identifier="gtk_container_get_children">
        <return-value transfer-ownership="container">
          <type name="GLib.List" c:type="GList*">
            <type name="Widget"/> <!-- !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->
          </type>
        </return-value>

Examples for the list member types (Gtk):

<type name="Action"/>
<type name="ActionGroup"/>
<type name="CellAccessible"/>
<type name="CellRenderer"/>
<type name="FlowBoxChild"/>
<type name="Gdk.EventSequence"/>
<type name="GdkPixbuf.Pixbuf"/>
<type name="Gesture"/>
<type name="Gio.File"/>
<type name="GObject.Closure"/>
<type name="ListBoxRow"/>
<type name="PaperSize"/>
<type name="RecentInfo"/>
<type name="TreePath"/>
<type name="TreeViewColumn"/>
<type name="utf8"/>
<type name="Widget"/>
<type name="Window"/>

Examples for the list member types (Gdk / GdkX11):

<type name="Atom"/>
<type name="Device"/>
<type name="GdkPixbuf.Pixbuf"/>
<type name="Seat"/>
<type name="Visual"/>
<type name="Window"/>

Example for the list member types (gobject-introspection directly):

<type name="AppInfo"/>
<type name="DBusConnection"/>
<type name="DBusInterface"/>
<type name="DBusObject"/>
<type name="DesktopAppInfo"/>
<type name="Drive"/>
<type name="Emblem"/>
<type name="File"/>
<type name="FileInfo"/>
<type name="GLib.Variant"/>
<type name="InetAddress"/>
<type name="IOExtension"/>
<type name="IOModule"/>
<type name="Mount"/>
<type name="ParamSpec"/>
<type name="SrvTarget"/>
<type name="TlsCertificate"/>
<type name="UnixMountEntry"/>
<type name="UnixMountPoint"/>
<type name="utf8"/>
<type name="Volume"/>
LordYuuma commented 3 years ago

The thing with GList is, that it's not GType'd, so we have the GType G_TYPE_POINTER with an extra flag, that it's actually a list.

Can you enable debug messages and copy the arg map for container:get-children as it's being invoked?

daym commented 3 years ago

How?

LordYuuma commented 3 years ago

https://spk121.github.io/guile-gi/GLib-Logging.html#GLib-Logging

In essence, set G_MESSAGES_DEBUG=all and have fun with loads of messages.

daym commented 3 years ago
(guix-gui.in:29793): GuileGI-DEBUG: 14:42:48.645: container:get-children - calling with 1 input and 0 output arguments
(guix-gui.in:29793): GuileGI-DEBUG: 14:42:48.645: container:get-children - argument mapping
(guix-gui.in:29793): GuileGI-DEBUG: 14:42:48.645:  SCM inputs required: 0, optional: 0, outputs: 0
(guix-gui.in:29793): GuileGI-DEBUG: 14:42:48.645:  C inputs: 0, outputs: 0
(guix-gui.in:29793): GuileGI-DEBUG: 14:42:48.645:  Return: '%return' pointer to gpointer, output, singleton, required
(guix-gui.in:29793): GuileGI-DEBUG: 14:42:48.645:     Item Type: GtkWidget of type GObject
(guix-gui.in:29793): GuileGI-DEBUG: 14:42:48.645: [C2S] On line 1115 while handing pointer to gpointer of container:get-children.
(guix-gui.in:29793): GuileGI-DEBUG: 14:42:48.645: [C2S] On line 1795 while handing pointer to gpointer of container:get-children.
(guix-gui.in:29793): GuileGI-DEBUG: 14:42:48.645: [C2S] On line 1682 while handing pointer to gpointer of container:get-children.

(guix-gui.in:29793): GuileGI-ERROR **: 14:42:48.645: unhandled argument type 'pointer to gpointer' src/gig_argument.c:1769
LordYuuma commented 3 years ago

Ahh, I think that makes more sense. The 'pointer to gpointer' is misleading here, what's not handled is the non-pointer GObject item type.

daym commented 3 years ago

I'm pretty sure the list actually contains pointers to GtkWidget (because struct _GtkWidget is private--so a client cannot get its size). But yeah, I see what you mean.

LordYuuma commented 3 years ago

Hmm, if that's the case, then perhaps the return type is incorrectly annotated? Wouldn't be the first broken gir.

daym commented 3 years ago

Maybe--but I posted a list of the element types used in the gir, above (it was retrieved using grep from the gir files). So that's how they look--all of them in gtk and gobject-introspection.

Also, https://gitlab.gnome.org/GNOME/gobject-introspection/-/issues/206 talks about unboxing etc (note especially their last comment).

Beware of https://gitlab.gnome.org/GNOME/gobject-introspection/blob/eb4e0fc8f9e278e81a3191f5e30610aaf4aaa762/tests/gimarshallingtests.c#L2528

LordYuuma commented 3 years ago

The thing is, our conversion handles either basic types or pointers to objects. We don't really have a way of unpacking plain types into lists. That wouldn't even work, as sizeof(obj) != sizeof(void*) in most of those cases, I assume.

daym commented 3 years ago

Yes, and gobject-introspection also seems to only support GList<int>, and a forced star at GList<T*> for T != int, including utf8 (utf8 as element type means GList<char*>). So that's how to interpret that.

So <type name="GList"><type name="int"/> -> GList<int>, but <type name="GList"><type name="GtkWidget"/> -> GList<GtkWidget*> (!).

daym commented 3 years ago

Thanks. It works!

But I think the g_warning is overdoing it. Almost all the element types in gobject-introspection, glib and gtk (see above) are specified without * but the * is implied.

LordYuuma commented 3 years ago

I think at least some warning should be given when two cases are treated the same. Furthermore, it makes sense to warn on the implicit pointer, since other data structures (see GLib.Array) have explicit pointers as well and the argument handler assumes those pointers to be explicit. (The reason we have that is mostly historical; Grilo used – and probably still uses – explicit pointers in its lists at the time of writing that code.) If that turns out to in fact be a well-documented feature of GI, we'll retract the warning, but for now it's better to keep it. The standard output of Gtk apps is not intended to be looked at anyway.

daym commented 3 years ago

The problem is that now I can't use --g-fatal-warnings in order to find real problems anymore.

Please ask gobject-introspection maintainers on how to handle this in order to be sure, and then we don't need to g_warning anymore.

LordYuuma commented 3 years ago

Guile-GI does not play nice with fatal warnings anyway. Loading Gtk already raises some pointer warnings before that. I'd rather recommend using our custom loggers to filter warnings coming from GLib/Gtk etc. and bail on them. Alternatively, you are free to patch out the warning on your end.

daym commented 3 years ago

Fine.

But asking for the right way should still be done :)