GNOME Bugzilla – Bug 629347
Missing annotations in GFile (was: Perf throws an exception in current HEAD)
Last modified: 2017-10-12 08:21:35 UTC
While trying to test the effect of some clutter material changes using perf I noticed that it stopped working: ---- JS ERROR: !!! Exception was: TypeError: f.replace is not a function JS ERROR: !!! lineNumber = '143' JS ERROR: !!! fileName = '/home/linux/gnome-shell/source/gnome-shell/js/ui/scripting.js' JS ERROR: !!! message = 'f.replace is not a function' JS ERROR: !!! stack = '_collect([object Object],"/tmp/gnome-shell-perf.I_myrB.json")@/home/linux/gnome-shell/source/gnome-shell/js/ui/scripting.js:143 ()@/home/linux/gnome-shell/source/gnome-shell/js/ui/scripting.js:253 _step([object Generator],(function () {_collect(scriptModule, outputFile);Meta.exit(Meta.ExitCode.SUCCESS);}),(function (err) {log("Script failed: " + err + "\n" + err.stack);Meta.exit(Meta.ExitCode.ERROR);}))@/home/linux/gnome-shell/source/gnome-shell/js/ui/scripting.js:114 ()@/home/linux/gnome-shell/source/gnome-shell/js/ui/scripting.js:110 ()@/home/linux/gnome-shell/source/gnome-shell/js/ui/scripting.js:42 Error("Chained exception")@:0 ("Chained exception")@gjs_throw: ---- g_file_replace seems to be annotated correctly though.
(In reply to comment #0) > g_file_replace seems to be annotated correctly though. Yes and no - the scanner now makes less assumptions, so it needs additional annotations.
Created attachment 170040 [details] [review] gio-2.0.c: Add more annotations for GFile Add annotations fixing warnings in GFile.
Hmmm, question: g_file_load_partial_contents_async() shares a single user_data parameter between two callback arguments - if I'm not mistaken, this will likely break in bindings, so should we annotate the function with (skip)?
Unfortunately, yes =(
Created attachment 170160 [details] [review] gio-2.0.c: Add more annotations for GFile (In reply to comment #4) > Unfortunately, yes =( OK.
Okay, I killed off gio-2.0.c in gobject-introspection and moved all the annotation into glib itself. Unfortunately that means that you have to redo all the annotations inside glib/gio. So this is now a gio bug, assigning over.
Created attachment 171072 [details] [review] introspection: Add more annotations for GFile Well, seems like you already added most of the annotations, so here go the remaining bits of the previous patch ...
Review of attachment 171072 [details] [review]: Rest looks good ::: gio/gfile.c @@ +1483,3 @@ * @file: input #GFile. * @flags: a set of #GFileCreateFlags. + * @cancellable: (allow-none): optional #GCancellable object, I think there's special code in g-i that makes all Gio.Cancellable allow-none, but I might be wrong. @@ +6433,2 @@ /** + * g_file_load_partial_contents_async: (skip) Why should it be skipped?
Created attachment 171078 [details] [review] introspection: Add more annotations for GFile (In reply to comment #8) > I think there's special code in g-i that makes all Gio.Cancellable allow-none, > but I might be wrong. No, I was - I thought this was one of the implicit behaviors which got removed, but it's still in the code. > > @@ +6433,2 @@ > /** > + * g_file_load_partial_contents_async: (skip) > > Why should it be skipped? Because a single closure is shared between two different callbacks - also see comment #3 and comment #4.
Review of attachment 171078 [details] [review]: Looks good to me.
Rico, is this going to cause API break problems for the bindings that we might care about?
(In reply to Philip Withnall from comment #11) > Rico, is this going to cause API break problems for the bindings that we > might care about? Looking at the conflicts when applying it, I think almost all the relevant annotations have been added already and, critically, the (skip) annotation is already there. So I don’t think this is going to cause any binding API changes.
After resolving all the conflicts, it looks like the only meaningful changes now are on the (skip)ped function, so I’m just going to push them, for documentation if nothing else. Attachment 171078 [details] pushed as 77fbc10 - introspection: Add more annotations for GFile
Created attachment 361403 [details] [review] gio: Mark callback_data of GFileReadMoreCallback as closure In conjunction with 77fbc10da658652f47b32b070f625020b671bea1
Review of attachment 361403 [details] [review]: ++
Attachment 361403 [details] pushed as 65e443d - gio: Mark callback_data of GFileReadMoreCallback as closure