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 618660 - vapigen fails to parse containers' element types
vapigen fails to parse containers' element types
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Code Generator
0.8.x
Other Linux
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on: 619545
Blocks:
 
 
Reported: 2010-05-14 17:42 UTC by Travis Reitter
Modified: 2010-06-18 22:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
The full .gir file I'm working with (314.68 KB, text/plain)
2010-05-18 17:19 UTC, Travis Reitter
  Details
girparser: improve support for generic type arguments (2.24 KB, patch)
2010-05-21 08:12 UTC, Evan Nemerson
none Details | Review
test case (966 bytes, application/octet-stream)
2010-05-24 17:30 UTC, Evan Nemerson
  Details
GIR parser: add special case for GPtrArray and GArray (2.63 KB, patch)
2010-05-28 01:42 UTC, Evan Nemerson
none Details | Review
GIR file to parse (332.42 KB, application/octet-stream)
2010-05-29 00:10 UTC, Travis Reitter
  Details
girparser: prefer GLib.GenericArray over GLib.PtrArray (1.55 KB, patch)
2010-06-11 07:51 UTC, Evan Nemerson
none Details | Review

Description Travis Reitter 2010-05-14 17:42:52 UTC
Eg, this annotation:

/**
 * Returns: (element-type GLib.Value) the value
 */
 GPtrArray * get_value (Foo *src) {}

ends up with this .gir fragment:

<array name="GLib.PtrArray" c:type="GPtrArray*">
    <type name="GLib.Value"/>
</array>

But vapigen ends up creating this .vapi fragment:

public GLib.PtrArray get_channel_classes ();

when it should instead generate this:

public GLib.PtrArray<GLib.Value> get_channel_classes ();



So, the missing support seems to be for:

* GPtrArrays and GArrays with this syntax:

<array name="GLib.PtrArray" c:type="GPtrArray*">
    <type name="GLib.Value"/>
</array>

* other containers, with this syntax like:

<type name="GLib.HashTable" c:type="GHashTable*">
    <type name="utf8"/>
    <type name="GObject.Value"/>
</type>

or

<type name="GLib.List" c:type="GList*">
    <type name="TelepathyGLib.Channel"/>
</type>
Comment 1 Travis Reitter 2010-05-14 18:37:40 UTC
This is blocking inclusion of Vala bindings into Telepathy-glib. See https://bugs.freedesktop.org/show_bug.cgi?id=27792
Comment 2 Travis Reitter 2010-05-18 17:19:00 UTC
Created attachment 161368 [details]
The full .gir file I'm working with
Comment 3 Travis Reitter 2010-05-18 17:21:59 UTC
This URL is the current state of my telepathy-glib g-i-based Vala bindings:

http://git.collabora.co.uk/?p=user/treitter/telepathy-glib.git;a=shortlog;h=refs/heads/vala-bindings-gi
Comment 4 Evan Nemerson 2010-05-21 08:12:23 UTC
Created attachment 161614 [details] [review]
girparser: improve support for generic type arguments

This patch adds support for the second pattern you mention (<type><type/></type>).

I'm a bit confused about what's going on with the other syntax, since my test case generates very different code. Ping me in IRC and we'll talk, then summarize here.
Comment 5 Evan Nemerson 2010-05-24 17:30:36 UTC
Created attachment 161886 [details]
test case

Here is a much simpler test case. <type><type/></type> works, but <array c:type="GArray*"><array name="GLib.Value"/></type> generates GLib.Value[]. This makes sense to me, but I'd like to hear what the gobject-introspection people have to say.
Comment 6 Travis Reitter 2010-05-24 19:02:37 UTC
(In reply to comment #5)
> Created an attachment (id=161886) [details]
> test case
> 
> Here is a much simpler test case. <type><type/></type> works, but <array
> c:type="GArray*"><array name="GLib.Value"/></type> generates GLib.Value[]. This
> makes sense to me, but I'd like to hear what the gobject-introspection people
> have to say.

I believe you're correct that <array><type/></array> doesn't make sense. GIR doesn't currently support <type><type/></type>, but it (or an equivalent) will hopefully be added due to bug #619545
Comment 7 Travis Reitter 2010-05-27 16:29:01 UTC
OK, so Johan committed a change (bug #619545 comment #3) that should make it possible to distinguish between, eg, GLib.Value[] and GLib.PtrArray<GLib.Value>.

It appears that this .gir fragment:

<array name="GLib.PtrArray" c:type="GPtrArray*">
  <type name="GLib.Value"/>
</array>

should map to GLib.PtrArray<GLib.Value> in Vala,

<type name="GObject.Value" c:type="GValue**"/>

should map to GLib.Value[],

<array name="GLib.PtrArray" c:type="GPtrArray*">
</array>

should map to GLib.PtrArray<> (ie, the element type wasn't properly annotated for this function, so this should probably produce an error)


And the GHashTable and GList .gir fragments listed in the original bug description are already produced by g-ir-scanner, but not yet supported by vapigen.
Comment 8 Evan Nemerson 2010-05-28 01:42:47 UTC
Created attachment 162161 [details] [review]
GIR parser: add special case for GPtrArray and GArray

This patch builds on the previous one (attachment #161614 [details], from comment #4).

Can you confirm that it works as expected?
Comment 9 Travis Reitter 2010-05-29 00:10:26 UTC
Created attachment 162247 [details]
GIR file to parse

With the latest patches, this fails with:


vala-intro $ PATH=/opt/gnome/bin:$PATH vapigen --vapidir=. --library=telepathy-glib TelepathyGLib-0.12.gir
error: too many type arguments
error: too many type arguments
Generation failed: 2 error(s), 0 warning(s)


the "too many type arguments" string is from valaobjecttype.vala:check()

I couldn't figure out exactly why it's hitting that - it sounds like it's detecting a _get_type() function with too many parameters, but all the ones I could find had exactly zero (maybe I'm missing some of the macros that generate get_type(), but I believe I've accounted for those).

This is based on a fairly recent mainline telepathy-glib.
Comment 10 Evan Nemerson 2010-06-02 05:33:13 UTC
The problem in comment #9 is this snippet (lines 1089-1091 of attachment #162247 [details]):

<array name="GLib.PtrArray" c:type="GPtrArray*">
  <type name="GLib.Value"/>
</array>

Currently, GLib.PtrArray is not a generic in the glib-2.0 vapi, so the single type argument is one too many. It would be nice if the error were more descriptive, but I don't think it's wrong.

I'm open to changing GPtrArray to be a generic (I want to talk to Jürg first), but that is outside the scope of this bug.
Comment 11 Travis Reitter 2010-06-02 16:32:08 UTC
(In reply to comment #10)
> The problem in comment #9 is this snippet (lines 1089-1091 of attachment
> #162247 [details]):
> 
> <array name="GLib.PtrArray" c:type="GPtrArray*">
>   <type name="GLib.Value"/>
> </array>
> 
> Currently, GLib.PtrArray is not a generic in the glib-2.0 vapi, so the single
> type argument is one too many. It would be nice if the error were more
> descriptive, but I don't think it's wrong.
> 
> I'm open to changing GPtrArray to be a generic (I want to talk to Jürg first),
> but that is outside the scope of this bug.

In that case, I say we open a new bug for that (could you?) and commit what's here to close this bug. I think I can at least make more progress with the attached patches (possibly enough to merge the initial Vala bindings to tp-glib).
Comment 12 Travis Reitter 2010-06-08 17:14:03 UTC
(In reply to comment #10)
> The problem in comment #9 is this snippet (lines 1089-1091 of attachment
> #162247 [details]):
> 
> <array name="GLib.PtrArray" c:type="GPtrArray*">
>   <type name="GLib.Value"/>
> </array>
> 
> Currently, GLib.PtrArray is not a generic in the glib-2.0 vapi, so the single
> type argument is one too many. It would be nice if the error were more
> descriptive, but I don't think it's wrong.
> 
> I'm open to changing GPtrArray to be a generic (I want to talk to Jürg first),
> but that is outside the scope of this bug.

As we discussed on IRC, we should just make a GLib.GenericArray that refers to GPtrArray's constructor/free/etc. functions by cname (to make it usable with C libraries that use GPtrArray).
Comment 13 Travis Reitter 2010-06-10 14:07:48 UTC
Anything else I can do to help here? This is one of a couple bugs blocking a release of Vala bindings for Telepathy (which blocks an initial release of libfolks, which is a new external dependency for Empathy in Gnome 3.0).
Comment 14 Evan Nemerson 2010-06-11 07:51:41 UTC
Created attachment 163365 [details] [review]
girparser: prefer GLib.GenericArray over GLib.PtrArray

Assumes the previous two patches in this bug have been applied, and that an implementation of GLib.GenericArray exists
Comment 15 Travis Reitter 2010-06-11 16:41:40 UTC
(In reply to comment #14)
> Created an attachment (id=163365) [details] [review]
> girparser: prefer GLib.GenericArray over GLib.PtrArray
> 
> Assumes the previous two patches in this bug have been applied, and that an
> implementation of GLib.GenericArray exists

Cool - looking forward to that implementation :)
Comment 16 Jürg Billeter 2010-06-15 18:21:23 UTC
I've now pushed GenericArray support to master.
Comment 17 Travis Reitter 2010-06-16 16:51:11 UTC
(In reply to comment #16)
> I've now pushed GenericArray support to master.

OK, now that this is in master, I believe we can push the patches on this bug and close it.
Comment 18 Evan Nemerson 2010-06-18 22:12:22 UTC
commit ffba491cadd4e1515d182dba92d1fadc4e94b15d
Author: Evan Nemerson <evan@coeus-group.com>
Date:   Fri Jun 18 15:08:49 2010 -0700

    girparser: add special case for GPtrArray and GArray
    
    Fixes bug 618660.

commit 7023a531dffa5d167bada9a33b87e8dbd98dc628
Author: Evan Nemerson <evan@coeus-group.com>
Date:   Fri May 21 01:04:48 2010 -0700

    girparser: improve support for generic type arguments