GNOME Bugzilla – Bug 752047
line numbers incorrect for invalid transfer warning
Last modified: 2015-12-27 15:57:45 UTC
Transfer annotations tend to be overused by newcomers and can be used incorrectly. This can be seen by various code bases using transfer none/full on gboolean, gint, GType, etc. It would also be great to warn when transfer container/floating are used incorrectly.
Created attachment 306976 [details] [review] scanner: Warn and ignore on incorrect transfer annotations I've tested this by generating the .gir for GLib, GObject, Gio, Gdk, Gtk, Peas, GtkSourceView and Gedit. Only a few corrections to annotations have been found, but no false positives.
Review of attachment 306976 [details] [review]: ::: tests/gimarshallingtests.c @@ +3410,3 @@ * gi_marshalling_tests_pointer_in_return: * + * Returns: The same pointer I'm not sure this is good. I would try and keep the transfer annotation for all pointer types — including gpointer/void*. The reason being that, while the annotation may be useless from an introspection perspective — i.e. gpointer is pointless for bindings, no pun intended — it's still useful from a documentation and C usage perspective. I really want to know whether a function is returning an allocated pointer or not, and the transfer annotation generates documentation for me. I guess what I'm saying is: I don't want this to be a warning for gpointer and other pointer-sized things we use as pointers; this means that transfer annotations on GType should warn.
Created attachment 307000 [details] [review] scanner: Warn and ignore on incorrect transfer annotations v2
Patch looks great to me, thanks Garrett! I rebased on master and pushed.
Review of attachment 307000 [details] [review]: LGTM.
Review of attachment 307000 [details] [review]: Pushed.
Reverted. :( This change looks awesome and I hope we can land it again soon without the breakage. It crashes when building atk: GISCAN Atk-1.0.gir Caught exception: <type 'exceptions.KeyError'> KeyError('<list>',) > /home/mcatanzaro/jhbuild/install/lib/gobject-introspection/giscanner/transformer.py(982)resolve_aliases() -> typenode = ast.type_names[typenode.target.target_fundamental] (Pdb) /home/mcatanzaro/jhbuild/install/share/gobject-introspection-1.0/Makefile.introspection:155: recipe for target 'Atk-1.0.gir' failed And it emits bogus warnings processing mutter: core/window.c:6259: Warning: Meta: invalid "transfer" annotation: only valid for array, struct, union, boxed, object and interface types core/window.c:6997: Warning: Meta: invalid "transfer" annotation: only valid for array, struct, union, boxed, object and interface types <unknown>:: Fatal: Meta: warnings configured as fatal <unknown>:: Fatal: Meta: warnings configured as fatal (Line 6259 is in the middle of a function that has no annotations. Line 6997 is in a function that returns a MetaWindow, a GObject.)
Giovanni fixed atk in https://bugzilla.gnome.org/show_bug.cgi?id=755681
/** * meta_window_get_pid: ... * Return value: (transfer none): the pid, or -1 if not known. Is clearly tripping the warning at least. I'm not sure why the line numbers are wrong yet.
I blindpushed https://git.gnome.org/browse/mutter/commit/?id=6190ae3873588aa1813ee4824bfff73bc1c75e5c And am un-reverting this patch for now. Let's keep this bug open for the line numbers.
The patch still triggers warnings for GVariants (which are not GObjects, but can nevertheless be transfer none, floating or full): https://git.gnome.org/browse/gnome-shell/tree/src/shell-global.c#n2056
Thanks, should be fixed by https://git.gnome.org/browse/gobject-introspection/commit/?id=d1086a641d6e46140b13996ff5a3bfbb90662dd5
Created attachment 317924 [details] [review] scanner: fix line number counting
Created attachment 317925 [details] line number test script - save the attached test.py somewhere - locally revert mutter commit 6190ae3873588aa1813ee4824bfff73bc1c75e5c - run the following, it will print line numbers where comment blocks start to stdout: python test.py -f /path/to/checkout/mutter/src/core/window.c - save the output somewhere - apply previously attached patch #317924 - edit giscanner/scannerlexer.l and insert around line 273, after the "c2=input()" line: char *current_file; current_file = g_file_get_uri (scanner->current_file); printf("%s:%d\n",current_file,lineno); - now build g-i and then mutter, it'll fail with: core/window.c:6311: ... core/window.c:7049: ... <unknown>:: Fatal: Meta: warnings configured as fatal - above line numbers are now correct, but we're not that easily satisfied. - search the mutter build output for: file:///path/to/checkout/mutter/src/core/window.c: - compare the printed line numbers with those of the test script - line numbers match those saved above, we're satisfied line number counting is correct. Merry Christmas :)
Review of attachment 317924 [details] [review]: LGTM, thanks!
Attachment 317924 [details] pushed as bbbf8e9 - scanner: fix line number counting