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 691217 - "child" of clutter_actor_iter_next() and _prev() misses some GObject Introspection tags
"child" of clutter_actor_iter_next() and _prev() misses some GObject Introspe...
Status: RESOLVED INCOMPLETE
Product: clutter
Classification: Platform
Component: ClutterActor
git master
Other Linux
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2013-01-06 08:03 UTC by Kouhei Sutou
Modified: 2013-01-13 04:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
It adds missing tags (1.50 KB, patch)
2013-01-06 08:03 UTC, Kouhei Sutou
none Details | Review

Description Kouhei Sutou 2013-01-06 08:03:04 UTC
Created attachment 232848 [details] [review]
It adds missing tags

"child" parameter of clutter_actor_iter_next() and clutter_actor_iter_prev() needs to be allocated by caller but it doesn't have "caller-allocates" tag.

And the "child" parameter can be NULL but "allow-none" tag is missing.
Comment 1 Emmanuele Bassi (:ebassi) 2013-01-06 10:44:11 UTC
the "caller-allocates" annotation should not be required: the out argument is a pointer to an object instance, not to a new structure.

the allow-none should not be necessary either: the only time the child out argument is set to 'none' is if the function returns false, in which case the pointer is undefined.

can you verify with the introspection developers that both annotations are actually required?
Comment 2 Kouhei Sutou 2013-01-06 11:44:14 UTC
For "caller-allocates" annotation:

The documentation of g_arg_info_is_caller_allocate() that is a getter function for "caller-allocates" annotation says:

  Obtain if the argument is a pointer to a struct or object that will receive an output of a function. The default assumption for GI_DIRECTION_OUT arguments which have allocation is that the callee allocates; if this is TRUE, then the caller must allocate.

  http://developer.gnome.org/gi/stable/gi-GIArgInfo.html#g-arg-info-is-caller-allocates

It seems that "a pointer to a struct or object that will receive an output" in the documentation equals to "a pointer to an object instance" that you said.

Because of this, I thought "caller-allocate" annotation is needed.


For "allow-none" annotation:

The documentation "allow-none" annotation says:

  NULL is ok, both for passing and for returning.

  http://developer.gnome.org/gi/stable/annotation-glossary.html#annotation-glossterm-allow-none

In clutter_actor_iter_next() and clutter_actor_iter_prev() case, we can use them like:

  clutter_actor_iter_next(iter, NULL);

I thought that "allow-none" annotation for out argument means "we can pass NULL for out argument".

clutter_actor_iter_next() supports "child is NULL" case by the following code:

  if (child != NULL)
    *child = ri->current

  http://git.gnome.org/browse/clutter/tree/clutter/clutter-actor.c#n18287

Because of this, I thought having "allow-none" annotation is better.

> can you verify with the introspection developers that both annotations are
actually required?

Where should I talk about the introspection developers? Is gtk-devel-list@gnome.org right location? https://live.gnome.org/GObjectIntrospection says:

  For questions or additional information, please use:

  Mailing list: gtk-devel-list@gnome.org


FYI: I am writing Ruby bindings for Clutter with GObject Introspection. This bug report and Bug 691114 ( https://bugzilla.gnome.org/show_bug.cgi?id=691114 ) are occurred in the work.
Comment 3 Emmanuele Bassi (:ebassi) 2013-01-06 17:42:10 UTC
(In reply to comment #2)
> For "caller-allocates" annotation:
> 
> The documentation of g_arg_info_is_caller_allocate() that is a getter function
> for "caller-allocates" annotation says:
> 
>   Obtain if the argument is a pointer to a struct or object that will receive
> an output of a function. The default assumption for GI_DIRECTION_OUT arguments
> which have allocation is that the callee allocates; if this is TRUE, then the
> caller must allocate.
> 
>  
> http://developer.gnome.org/gi/stable/gi-GIArgInfo.html#g-arg-info-is-caller-allocates
> 
> It seems that "a pointer to a struct or object that will receive an output" in
> the documentation equals to "a pointer to an object instance" that you said.

I don't think it is; caller-allocates usually works for API like:

  void
  clutter_actor_get_background_color (ClutterActor*, ClutterColor*)

i.e. ClutterColor needs to be a real structure that the function can copy data into, which means that ClutterColor* needs to point to a structure that has been allocated prior to calling the function.

in the case of clutter_actor_iter_next(), ClutterActor** child does not need to be allocated: it's a pointer to a ClutterActor instance.
 
> Because of this, I thought "caller-allocate" annotation is needed.

I think this should be clarified by the introspection developers.

> For "allow-none" annotation:
> 
> The documentation "allow-none" annotation says:
> 
>   NULL is ok, both for passing and for returning.
> 
>  
> http://developer.gnome.org/gi/stable/annotation-glossary.html#annotation-glossterm-allow-none
> 
> In clutter_actor_iter_next() and clutter_actor_iter_prev() case, we can use
> them like:
> 
>   clutter_actor_iter_next(iter, NULL);
> 
> I thought that "allow-none" annotation for out argument means "we can pass NULL
> for out argument".

not really: (allow-none) means that you can pass NULL for an argument, though it's generally done for 'in' arguments, AFAIR.

> clutter_actor_iter_next() supports "child is NULL" case by the following code:
> 
>   if (child != NULL)
>     *child = ri->current
> 
>   http://git.gnome.org/browse/clutter/tree/clutter/clutter-actor.c#n18287
> 
> Because of this, I thought having "allow-none" annotation is better.

I don't disagree, but, again, I'd rather have some confirmation, because adding annotations changes the introspection ABI.
 
> > can you verify with the introspection developers that both annotations are
> actually required?
> 
> Where should I talk about the introspection developers? Is
> gtk-devel-list@gnome.org right location?
> https://live.gnome.org/GObjectIntrospection says:
> 
>   For questions or additional information, please use:
> 
>   Mailing list: gtk-devel-list@gnome.org

gtk-devel-list is fine; there's also the #introspection IRC channel on irc.gnome.org.
Comment 4 Kouhei Sutou 2013-01-07 13:35:05 UTC
I sent an e-mail to gtk-devel-list: https://mail.gnome.org/archives/gtk-devel-list/2013-January/msg00014.html
Comment 5 Kouhei Sutou 2013-01-12 04:47:17 UTC
I got a response: https://mail.gnome.org/archives/gtk-devel-list/2013-January/msg00015.html

But it is not from a GObject Introspection developer.

I give up resolving this bug report... Sorry...

In my opinion, "caller-allocate" is not needed and "allow-none" is needed.


Thanks for replying my report.
Comment 6 Simon Feltman 2013-01-12 19:27:07 UTC
Kouhei,
Is there a specific problem you are running into with the API annotations ?

I think either one of the proposed annotations would make the API un-usable. The child arg receives a pointer to an exiting actor so it does not need caller allocation. As for allow-none, I do see a NULL check in the code, but making this available to introspection would mean rendering the arg as inout which doesn't really make sense. With the current annotations the API works well from python:

>>> from gi.repository import Clutter
>>> Clutter.init(None)
(<enum CLUTTER_INIT_SUCCESS of type ClutterInitError>, [])
>>> root = Clutter.Actor(name='root')
>>> root.add_child(Clutter.Actor(name='a'))
>>> root.add_child(Clutter.Actor(name='b'))
>>> it = Clutter.ActorIter()
>>> it.init(root)
>>> has_next, actor = it.next()
>>> while has_next:
...      print(actor.props.name)
...      has_next, actor = it.next()
... 
a
b
>>>
Comment 7 Kouhei Sutou 2013-01-13 04:20:50 UTC
(In reply to comment #6)

> Is there a specific problem you are running into with the API annotations ?

No.
I changed Ruby bindings for GObject Introspection (Ruby/GObjectIntrospection) to work with the current annotation. It works well. :-)

(Bug 691114 has a problem for me. Because Ruby/GObjectIntrospection can't detect c clutter_color_alloc() as constructor automatically.)

> The child arg receives a pointer to an exiting actor so it does not need caller
> allocation.

I think you're right and I misunderstood because both Emmanuele Bassi and Simon McVittie also say so.

> As for allow-none, I do see a NULL check in the code, but making
> this available to introspection would mean rendering the arg as inout which
> doesn't really make sense.

I don't know about it... So I also want to hear about an opinion from any GObject Introspection developer.

In Ruby/GObjectIntrospection, this is no matter because it always passes a return location to the "child" argument.


Thanks for your comment.