GNOME Bugzilla – Bug 602512
GLib-2.0.gir does not mark error domains
Last modified: 2015-02-07 17:02:41 UTC
Error domains in GLib-2.0.gir are not recognized. There are at least two problems: 1) GLibTransformer._parse_error_quark_function() looks for methods returning "GLib.Quark" or "GQuark", but when parsing glib, the return type name at that point in parsing is just "Quark", not "GQuark" 2) GLibTransformer._resolve_quarks() seems to require that error enum types be registered as GTypes (self._uscore_type_names doesn't contain entries for non-gtype-registered enums) but of course, error enums in libglib are not GTyped. The first part is easy but I'm not sure about the second part
Created attachment 149481 [details] [review] [scanner] Fix various hardcoded type name checks This fixes up GLib.Quark vs GQuark, and related problems (such as making it possible to use (element-type) in libglib's annotations). It also fixes some places that already worked, to check ctype rather than looking for an un-prefixed type.name, since the latter approach could run into problems with types called FooList, FooError, etc, mistaking them for the GLib ones. While trying various possibilities, I discovered that methods that take a "GError *" were being treated as though they *returned* a GError, so I left that check only testing for ctype=='GError**' and not looking at type.name at all. (In most places, both checks are needed, because people using the (type Foo) annotation will only specify one of them. But it seems unlikely you'd use (type) to change something to a GError.) (This fixes #1, but not #2.)
Comment on attachment 149481 [details] [review] [scanner] Fix various hardcoded type name checks This should be three different patches [annotation, GError, type handling], for proper bisectability etc. Should be enough to check for ctype == GDestroyNotify, since we depend on GObject etc no other libraries are going to be able to define something different with the same type name. The other two patches looks good.
(In reply to comment #2) > Should be enough to check for ctype == GDestroyNotify, since we depend on > GObject etc no other libraries are going to be able to define something > different with the same type name. but that breaks if someone uses the (type) annotation. My first attempt only checked ctype and never type.name, but it broke on tests/scanner/annotation.c here: /** * AnnotationObject::list-signal: * @annotation: the annotation object * @list: (type GLib.List): (element-type utf8): (transfer container): a list of strings * * This is a signal which takes a list of strings, but it's not * known by GObject as it's only marked as G_TYPE_POINTER */ ctype was "gpointer" (or maybe None? not "GList*" anyway) so the (element-type) parser threw an exception.
Created attachment 149486 [details] [review] [scanner] "GError *" is not the same as "GError **" Only set "throws" in the latter case
Created attachment 149487 [details] [review] [scanner] Fix various hardcoded type name checks Various places that check hardcoded type names were not always handling the case of the type being used from within its own library (in which case it won't have a type prefix). Make them more consistent.
Created attachment 149488 [details] [review] annotate g_completion_complete_utf8 acts as a regression test on the recognizing-GLib.List-inside-glib part of the previous patch.
(In reply to comment #3) > (In reply to comment #2) > > Should be enough to check for ctype == GDestroyNotify, since we depend on > > GObject etc no other libraries are going to be able to define something > > different with the same type name. > > but that breaks if someone uses the (type) annotation. My first attempt only > checked ctype and never type.name, but it broke on tests/scanner/annotation.c > here: > > /** > * AnnotationObject::list-signal: > * @annotation: the annotation object > * @list: (type GLib.List): (element-type utf8): (transfer container): a list > of strings > * > * This is a signal which takes a list of strings, but it's not > * known by GObject as it's only marked as G_TYPE_POINTER > */ > > ctype was "gpointer" (or maybe None? not "GList*" anyway) so the (element-type) > parser threw an exception. You are right, you need both. Thanks for the explanation! Enforcing the type of a gpointer to something different would break without that as you point out.
Attachment 149486 [details] pushed as 6710cfa - [scanner] "GError *" is not the same as "GError **" Attachment 149487 [details] pushed as b65d598 - [scanner] Fix various hardcoded type name checks Attachment 149488 [details] pushed as 6148485 - annotate g_completion_complete_utf8
oops, there's still: (In reply to comment #0) > 2) GLibTransformer._resolve_quarks() seems to require that error enum > types be registered as GTypes (self._uscore_type_names doesn't > contain entries for non-gtype-registered enums) but of course, > error enums in libglib are not GTyped.
(In reply to comment #9) > oops, there's still: > > (In reply to comment #0) > > 2) GLibTransformer._resolve_quarks() seems to require that error enum > > types be registered as GTypes (self._uscore_type_names doesn't > > contain entries for non-gtype-registered enums) but of course, > > error enums in libglib are not GTyped. uscore_type_names was added to help resolve foo_bar_* functions against FooBar GObject/GBoxed, I think we probably want a separate variable for non-registered enums. _uscore_ctype_names ?
Created attachment 150241 [details] [review] Fix glib:error-quark scanning for unregistered enum types Here's a fix for the remaining part; I wrote it before thinking to look for open bugs, so it was done without reference to Colin's suggestion; I just on-the-fly generate a map of underscore enum types in the module.
Attachment 150241 [details] pushed as 965ff4c - Fix glib:error-quark scanning for unregistered enum types
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]