After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 729543 - [girepository] Add support for "throws" flag to callbacks
[girepository] Add support for "throws" flag to callbacks
Status: RESOLVED FIXED
Product: gobject-introspection
Classification: Platform
Component: libgirepository
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gobject-introspection Maintainer(s)
gobject-introspection Maintainer(s)
Depends on:
Blocks: 710671 729015
 
 
Reported: 2014-05-04 23:26 UTC by Simon Feltman
Modified: 2015-07-11 23:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
girepository: Add g_struct_info_find_field function (6.70 KB, patch)
2014-05-05 05:34 UTC, Simon Feltman
none Details | Review
tests: Convert gitestthrows to use GTest framework (1.10 KB, patch)
2014-05-05 05:34 UTC, Simon Feltman
committed Details | Review
tests: Add failing test for g_callable_info_can_throw_gerror (4.36 KB, patch)
2014-05-05 05:34 UTC, Simon Feltman
none Details | Review
girepository: Support GError exceptions on callbacks (7.35 KB, patch)
2014-05-05 05:34 UTC, Simon Feltman
none Details | Review
girepository: Add g_struct_info_find_field() v2 (6.84 KB, patch)
2015-06-03 14:08 UTC, Garrett Regier
none Details | Review
girepository: Support GError exceptions on callbacks v2 (10.13 KB, patch)
2015-06-03 14:10 UTC, Garrett Regier
committed Details | Review
girepository: Add g_struct_info_find_field() v3 (7.72 KB, patch)
2015-06-03 14:36 UTC, Garrett Regier
committed Details | Review

Description Simon Feltman 2014-05-04 23:26:37 UTC
The callback field for vfuncs correctly uses the "throws" attribute in gir, but this does not seem to translate through to the typelib/repository API:

For instance, looking at Gio-2.0.gir we see the following vfunc setup:

    <interface name="Initable"
      ...
      <method name="init"
              c:identifier="g_initable_init"
              version="2.22"
              throws="1">

      <virtual-method name="init" invoker="init" version="2.22" throws="1">
    </interface>

    <record name="InitableIface"
      <field name="init">
        <callback name="init" throws="1">
      ...
    </record>

All three of the invoker, vfunc, and callback field infos correctly contain throws="1". But interacting with the typelib from Python, can_throw_gerror returns False for the CallbackInfo.

# Invoker:
>>> from gi.repository import Gio
>>> invoker = Gio.Initable.init
>>> invoker.can_throw_gerror()
True

# VFunc:
>>> vfunc = invoker.get_vfunc()
>>> vfunc.can_throw_gerror()
True

# Callback Field:
>>> iface = Gio.InitableIface.__info__
>>> iface
StructInfo(InitableIface)
>>> field = iface.get_fields()[1]
>>> field
gi.FieldInfo(init)
>>> callback = field.get_type().get_interface()
>>> callback
gi.CallbackInfo(init)
>>> callback.get_arguments()  # Note the GError is correctly not included as an arg
(gi.ArgInfo(initable), gi.ArgInfo(cancellable))
>>> callback.can_throw_gerror()
False

We can see in the implementation of g_callable_info_can_throw_gerror [1], callbacks and signals always return false for this. The information is either not included in the typelib or the implementation needs to be updated to look for this.

[1] https://git.gnome.org/browse/gobject-introspection/tree/girepository/gicallableinfo.c?id=GOBJECT_INTROSPECTION_1_40_0#n94
Comment 1 Simon Feltman 2014-05-05 05:34:20 UTC
Created attachment 275859 [details] [review]
girepository: Add g_struct_info_find_field function

Add find_field utility function for finding a field info by name.
Beyond convenience, this should be faster than manually using the
get_n_fields and get_field calls because get_field has an
additional iteration for each field to calculate offsets O(n^2).
find_field combines the offset and comparison computations
into a single loop O(n).
Comment 2 Simon Feltman 2014-05-05 05:34:23 UTC
Created attachment 275860 [details] [review]
tests: Convert gitestthrows to use GTest framework
Comment 3 Simon Feltman 2014-05-05 05:34:26 UTC
Created attachment 275861 [details] [review]
tests: Add failing test for g_callable_info_can_throw_gerror
Comment 4 Simon Feltman 2014-05-05 05:34:29 UTC
Created attachment 275862 [details] [review]
girepository: Support GError exceptions on callbacks

Generalize "throws" attribute to SignatureBlob which can be used by all
callable blob types. Keep FunctionBlob and VFuncBlob throw attributes
around and functional for compatibility. Refactor girwriter.c to write
out throws attribute for all callable types.
Comment 5 André Klapper 2015-02-07 17:18:35 UTC
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]
Comment 6 Garrett Regier 2015-06-03 14:08:47 UTC
Created attachment 304517 [details] [review]
girepository: Add g_struct_info_find_field() v2

Add find_field utility function for finding a field info by name.
Beyond convenience, this should be faster than manually using
the get_n_fields and get_field functions because get_field does
an additional iteration for each field to calculate offsets O(n^2).
Thus find_field combines the offset and comparison
computations into a single loop O(n).
Comment 7 Garrett Regier 2015-06-03 14:10:40 UTC
Created attachment 304518 [details] [review]
girepository: Support GError exceptions on callbacks v2

Generalize "throws" attribute to SignatureBlob which can be used by all
callable blob types. Keep FunctionBlob and VFuncBlob throw attributes
around and functional for compatibility. Refactor girwriter.c to write
out throws attribute for all callable types.
Comment 8 Emmanuele Bassi (:ebassi) 2015-06-03 14:18:55 UTC
Review of attachment 304517 [details] [review]:

It looks good to me.

Maybe we should also get a find_method(), since get_method() calls get_field() as well.

::: girepository/gistructinfo.c
@@ +125,3 @@
+ *
+ * Returns: (transfer full): the #GIFieldInfo or %NULL if not found,
+ * free it with g_base_info_unref() when done.

Should probably add a Since:

::: girepository/gistructinfo.h
@@ +49,3 @@
 						gint          n);
 
 GI_AVAILABLE_IN_ALL

Do we have versioned macros yet? If we don't, then it doesn't matter.
Comment 9 Emmanuele Bassi (:ebassi) 2015-06-03 14:21:45 UTC
Review of attachment 304518 [details] [review]:

Looks generally good to me.
Comment 10 Garrett Regier 2015-06-03 14:36:36 UTC
Created attachment 304523 [details] [review]
girepository: Add g_struct_info_find_field() v3

(In reply to Emmanuele Bassi (:ebassi) from comment #8)
> Review of attachment 304517 [details] [review] [review]:
> 
> It looks good to me.
> 
> Maybe we should also get a find_method(), since get_method() calls
> get_field() as well.
> 

g_struct_info_find_method() already exists.

> ::: girepository/gistructinfo.c
> @@ +125,3 @@
> + *
> + * Returns: (transfer full): the #GIFieldInfo or %NULL if not found,
> + * free it with g_base_info_unref() when done.
> 
> Should probably add a Since:
> 

Done.

> ::: girepository/gistructinfo.h
> @@ +49,3 @@
>  						gint          n);
>  
>  GI_AVAILABLE_IN_ALL
> 
> Do we have versioned macros yet? If we don't, then it doesn't matter.

Fixed.
Comment 11 Emmanuele Bassi (:ebassi) 2015-06-03 14:39:48 UTC
Review of attachment 304523 [details] [review]:

Looks good.
Comment 12 Garrett Regier 2015-06-21 20:09:14 UTC
This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.
Comment 13 Simon Feltman 2015-07-11 23:10:29 UTC
Garret, Emmanuele,

Thanks for updating the patches and getting them committed. Although I would have preferred if you kept myself as the primary commit author since I did the majority of work (it looks like only minimal formatting and build system stuff was changed). Then add yourself using a "Co-Authored-By:" label (or vise versa depending on where the majority of the code came from). Or is this not how it should work?

In any event, I'm glad this stuff is finally committed.