Created attachment 149806 [details] [review]
Assume allow-none for GCancellable

Review of attachment 149806 [details] [review]:

::: giscanner/annotationparser.py
@@ +559,3 @@
+            node.allow_none = True
+            node.type.ctype == 'GCancellable*'):
+        if (OPT_ALLOW_NONE in options or

What about introducing something like meta-annotations, where we can add an annotation
for the GCancellable-struct saying that the default value is to allow none.

And if we're defaulting to allow-none, is there a way to revert that? I might have a reason to write a library where a GCancellable is required, either consciously or not.

(In reply to comment #2)
> Review of attachment 149806 [details] [review]:
> 
> ::: giscanner/annotationparser.py
> @@ +559,3 @@
> +            node.allow_none = True
> +            node.type.ctype == 'GCancellable*'):
> +        if (OPT_ALLOW_NONE in options or
> 
> What about introducing something like meta-annotations, where we can add an
> annotation
> for the GCancellable-struct saying that the default value is to allow none.

If meta-annotations were just "apply this annotation at every argument using this type" it seems relatively straightforward, but I think what we really want is plugins for the scanner, so you can say g-ir-scanner --plugin=myplugin.py

Or you could just post-process the .gir which is fairly easy.

> And if we're defaulting to allow-none, is there a way to revert that? I might
> have a reason to write a library where a GCancellable is required, either
> consciously or not.

Hmm...I can't think of a scenario in which it would make sense to require a cancellable deliberately; and accidentally seems like it'd always be a bug.  In other words it really does seem like it's part of the API contract for GCancellable, like the rules around GError.

(In reply to comment #3)
> (In reply to comment #2)
> > Review of attachment 149806 [details] [review] [details]:
> > 
> > ::: giscanner/annotationparser.py
> > @@ +559,3 @@
> > +            node.allow_none = True
> > +            node.type.ctype == 'GCancellable*'):
> > +        if (OPT_ALLOW_NONE in options or
> > 
> > What about introducing something like meta-annotations, where we can add an
> > annotation
> > for the GCancellable-struct saying that the default value is to allow none.
> 
> If meta-annotations were just "apply this annotation at every argument using
> this type" it seems relatively straightforward

With regards to this - can you think of any other annotations of this form?

(In reply to comment #3)
> Hmm...I can't think of a scenario in which it would make sense to require a
> cancellable deliberately

g_cancellable_push_current() for instance, and a few other GCancellable methods, though some of those might be bugs.

> In other words it really does seem like it's part of the API contract for
> GCancellable, like the rules around GError.

which has the same issues with, eg, g_propagate_error() taking a GError** which does not mean "throws an error". But it's much less likely that you'd work directly with GError methods from a binding, so probably not worth fixing there.

The answer might be "ignore the special automatic-annotation rule in the type's own methods"?

(In reply to comment #4)
> With regards to this - can you think of any other annotations of this form?

the GAsyncReadyCallback ones we just did. There might be other always-allow-none types...

bug 601425 changed all GtkIconSize params to (type int), and Johan mentioned that GtkResponse has the same problem. This could be another case for meta-annotations; being able to specify that a particular type should always be converted to another type in function calls.

Comment on attachment 149806 [details] [review]
Assume allow-none for GCancellable

I accidentally committed this while looking at patches...going to keep it in since meta-annotations would be hard and it's better to have something here.

[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]

Hasn't happened in 9 years, it's unlikely it'll happen now.

I'm fairly comfortable with the idea of adding heuristics based on GLib/GObject/GIO types inside gobject-introspection. If other projects wish to add additional heuristics, we can revisit the idea of having annotations for default behaviours.