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 581685 - Allow nested parameterized types for GList, GSList, and GHash.
Allow nested parameterized types for GList, GSList, and GHash.
Status: RESOLVED FIXED
Product: gobject-introspection
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gobject-introspection Maintainer(s)
gobject-introspection Maintainer(s)
Depends on:
Blocks: 581801
 
 
Reported: 2009-05-07 03:56 UTC by C. Scott Ananian
Modified: 2015-02-07 17:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Allow-nested-parameterized-types-for-GList-GSList-and-GHash.patch (5.83 KB, patch)
2009-05-07 03:56 UTC, C. Scott Ananian
rejected Details | Review
Parse-parameterized-types-in-annotations.patch (8.67 KB, patch)
2009-05-08 18:32 UTC, C. Scott Ananian
none Details | Review
Parse-parameterized-types-in-annotations.patch (8.66 KB, patch)
2009-05-08 20:16 UTC, C. Scott Ananian
none Details | Review
Parse-parameterized-types-in-annotations.patch (8.67 KB, patch)
2009-05-11 15:15 UTC, C. Scott Ananian
accepted-commit_now Details | Review

Description C. Scott Ananian 2009-05-07 03:56:28 UTC
These types can be specified as, for example:
  <type name="GLib.HashTable<utf8,GLib.HashTable<utf8,utf8>"
        c:type="GHashTable" />
as an alternative to the existing
  <type name="GLib.HashTable" c:type="GHashTable">
     <type name="utf8" />
     <type name="GLib.HashTable" c:type="GHashTable">
        <type name="utf8" />
        <type name="utf8" />
     </type>
  </type>
syntax.

These nested types can be specified with the (type ) or (element-type )
annotations without having to define some sort of nested annotation syntax.
Comment 1 C. Scott Ananian 2009-05-07 03:56:57 UTC
Created attachment 134154 [details] [review]
Allow-nested-parameterized-types-for-GList-GSList-and-GHash.patch
Comment 2 Johan (not receiving bugmail) Dahlin 2009-05-07 12:21:58 UTC
Comment on attachment 134154 [details] [review]
Allow-nested-parameterized-types-for-GList-GSList-and-GHash.patch

It used to be like this and we removed it in favor of using proper XML which all the advantages. It is not going to be put back in the GIR. A similar syntax would be good to have in the annotationparser though.
Comment 3 C. Scott Ananian 2009-05-08 18:32:10 UTC
Created attachment 134277 [details] [review]
Parse-parameterized-types-in-annotations.patch

New version of the patch which adds support to giscanner (annotation parser), instead of to the GIR XML parser.
Comment 4 C. Scott Ananian 2009-05-08 20:16:35 UTC
Created attachment 134284 [details] [review]
Parse-parameterized-types-in-annotations.patch

Update patch to fix a minor PEP8 style issue (spaces around list brackets).
Comment 5 Johan (not receiving bugmail) Dahlin 2009-05-08 23:50:03 UTC
Comment on attachment 134284 [details] [review]
Parse-parameterized-types-in-annotations.patch

>From 6e09b030a2515b46e2f9848d9cc8b90574bd716c Mon Sep 17 00:00:00 2001
>From: C. Scott Ananian <cscott@litl.com>
>Date: Fri, 8 May 2009 11:52:17 -0400
>Subject: [PATCH] Parse parameterized types (using <>) in annotations.
>
>You can now specify a nested parameterized type in annotations as
>(for example):
>  @param: (type GLib.HashTable<utf8,GLib.HashTable<utf,utf>>)
>or
>  @param: (element-type utf8 GLib.HashTable<utf,utf>)

This looks much better, it separates the annotation syntax from the GIR.


>+    def resolve(self, type_str, orig_node=None):

Should probably be marked as private.

>diff --git a/tests/everything/everything.c b/tests/everything/everything.c

>+/**
>+ * test_ghash_nested_everything_return:
>+ * Specify nested parameterized types directly with the (type ) annotation.
>+ *
>+ * Return value: (type GLib.HashTable<utf8,GLib.HashTable<utf8,utf8>>) (transfer full):
>+ */
>+GHashTable *

>+/**
>+ * test_ghash_nested_everything_return2:
>+ * Another way of specifying nested parameterized types: using the
>+ * element-type annotation.
>+ *
>+ * Return value: (element-type utf8 GLib.HashTable<utf8,utf8>) (transfer full):
>+ */
>+GHashTable *

I am not so sure about the syntax here.

It is a bit confusing to have both () and <> in the annotation syntax.
The same thing could for instance be specified as:
  (element-type utf8 (type GLib.HashTable) (element-type utf8 utf8)) (transfer full):

But OTOH that is more to type and the gtk-doc usually lack space.

Colin: any thoughts or suggestions here?
Comment 6 C. Scott Ananian 2009-05-11 15:15:06 UTC
Created attachment 134402 [details] [review]
Parse-parameterized-types-in-annotations.patch

Made resolve() method private.
Comment 7 Colin Walters 2009-05-11 17:26:19 UTC
I guess a highlevel question is - are we doing nested container types as a one-off for an odd NM method, or is the intent to fully support them everywhere?

In terms of syntax maybe

(element-type utf-8 (element-type GLib.HashTable utf-8 utf-8)) 

So in nesting cases we require the container type to come first.
Comment 8 C. Scott Ananian 2009-05-11 18:21:23 UTC
My patches support nested container types everywhere.  I'll note that the annotation parser is currently not very smart: using a syntax involving parens will require it to get much smarter, since it's not counting nesting levels at all.  And (element-type utf8 (element-type ...)) is semantically incorrect; it should be:
  (element-type utf8 (type GLib.HashTable utf8 utf8))
or
  (type GLib.HashTable utf8 (type GLib.HashTable utf8 utf8))
ie, element-type presupposes a type annotation, which is only valid at top-level.

And this way lies madness.  If you wanted to be strictly consistent, you'd use:
  (type GLib.HashTable (type utf8) (type GLib.HashTable (type utf8) (type utf8)))
which is, of course, how it looks in the XML -- but I think humans would much prefer less typing.
Comment 9 C. Scott Ananian 2009-05-18 17:20:33 UTC
The test in bug 581695 can be enabled if/when this patch is committed.
Comment 10 C. Scott Ananian 2009-06-04 19:26:59 UTC
Ping!  The patch is lonely. 
Comment 11 Colin Walters 2009-06-04 20:26:53 UTC
Looking over this again, I agree with Johan; I like the "generics" syntax for annoations, but not in the .gir.  This patch doesn't do recursive nesting in the .gir, or does it?
Comment 12 C. Scott Ananian 2009-06-04 21:03:50 UTC
Nope, the <> syntax is in annotations only; the GIR is proper XML nested tags.  (Sorry, the initial bug summary has drifted away from the patch.)
Comment 13 C. Scott Ananian 2009-06-10 18:25:56 UTC
So is this patch cleared to commit, since the <> syntax is confined to annotations?
Comment 14 Colin Walters 2009-06-11 20:02:59 UTC
Sorry for dragging my feet on this one, I'm not really happy with the typelib side (bug 581686) mostly.  Let's go ahead and put this in though under the assumption that we will support nested types in some form.

On this patch though, I'd prefer if the parsing functions for nested types weren't themselves nested python functions =) Maybe move grab_one to be a toplevel function parse_nested_type or something?  I'm not sure why the function takes other functions as parameters when the same functions are always passed?
Comment 15 C. Scott Ananian 2009-06-11 20:32:47 UTC
Well, the combiner parameter changes if you read carefully: the topmost "combiner" tries to recycle a given base type object, which preserves additional annotation information already present in the type object, but this can't be done for lower 'combiners'.  So we receive 'top_combiner, combiner' but always pass in 'combiner, combiner' to sub-invocations.

I don't strictly speaking have to make resolver() a passed-in function, but (in my mind) it helped me separate out the generic parsing code from the transformer-specific resolution code.  grab_one is really doing parsing only, and delegating all the semantic actions to resolver/combiner.

I seem to have a fixation on keeping namespaces clean, so I like to use nested functions if the functionality has no reason to escape its lexical context.  That's what they're in the language for, after all.  I'm just decomposing a pretty hairy parsing/resolution function into separate 'parse' and 'resolve' pieces for better readability.
Comment 16 Colin Walters 2009-06-11 21:00:12 UTC
Ah, ok.  Yeah, I understand the reasons for nesting, it just seems not unlikely to me that at some point some other bit of code will need to parse these types.  It's fine though to split it out at that time though.

My main concern is figuring out the typelib implications.
Comment 17 C. Scott Ananian 2009-06-12 14:10:12 UTC
Typelib implications?  The typelib already supports nested <type> elements.  This patch is all you need.  The code to use these nested types in gjs is already present as well.
Comment 18 C. Scott Ananian 2009-06-12 16:46:45 UTC
Pushed the patch and closing the bug; colin reopen this (or a different bug) if there are still typelib issues you're concerned about.
Comment 19 André Klapper 2015-02-07 17:00:01 UTC
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]