GNOME Bugzilla – Bug 668427
Provide better support for GTypes
Last modified: 2012-01-26 15:07:09 UTC
GTypes can't be represented as a primitive type -- JS integers are 30 bits, and I hate to think of what would happen if we stored it as a double. Because we need to be able to provide accurate GType information if we're going to have any place in the GNOME introspection stack, this means that we need to make a new boxed wrapper for GTypes. Guess what this patch stack does! Additionally, with this patch landed, we can now do things like: const GObject = imports.gi.GObject; const Gtk = imports.gi.Gtk; let model = new Gtk.ListStore(); model.set_column_types([GObject.TYPE_STRING, GObject.TYPE_INT]); and all that fun stuff. Before, it would break on 64-bit systems, because we didn't handle integer arrays that were composed of 8-byte integers, like GType.
Created attachment 205790 [details] [review] tests: Remove all useless logging from tests
Created attachment 205791 [details] [review] util: Correct the create_proto functions created by DEFINE_PROTO Before, the code was checking to see if the class existed, but didn't return the correct value in the case that the class *did* exist.
Created attachment 205792 [details] [review] Represent GTypes as a new boxed wrapper type We can't natively represent a pointer as a regular value in JS (we have at most 30-bit integers or doubles), so to store GTypes correctly, we need to box them. Create a new native object that simply represents a GType, give it a 'g_type_name' wrapper, and fix up the rest of the world so we no longer treat a GType as a special integer.
Review of attachment 205790 [details] [review]: Looks good.
Review of attachment 205791 [details] [review]: Looks right.
Review of attachment 205792 [details] [review]: What about making GType a special case of a new "64 bit integer" wrapper? We can easily accept e.g. new GUint64(0xdeadbeef, 0xdeadbeef) to compose two 32 bit integers on input to functions expecting GI_TYPE_TAG_UINT64. The further question is whether to start returning this boxed type instead of doubles by default.
I'm not sure a GUint64 is worth it -- you can't really do anything with the number besides storing it, unless we wanted to implement "number1.add(number2)".
Review of attachment 205792 [details] [review]: ::: gi/gtype.c @@ +43,3 @@ +{ + static GQuark value = 0; + if (G_UNLIKELY (value == 0)) I'd probably make new quark wrappers threadsafe via g_once_init_enter(). Probably doesn't matter for gjs, but it could bite you later hacking on a different library. @@ +132,3 @@ + goto out; + + JS_SetPrivate(context, object, GUINT_TO_POINTER(gtype)); GSIZE_TO_POINTER @@ +144,3 @@ + JSObject *object) +{ + GType gtype = G_TYPE_NONE; You want to default here to G_TYPE_INVALID, not G_TYPE_NONE, right? Looking at what your arg.c code does.
Created attachment 206146 [details] [review] Represent GTypes as a new boxed wrapper type We can't natively represent a pointer as a regular value in JS (we have at most 30-bit integers or doubles), so to store GTypes correctly, we need to box them. Create a new native object that simply represents a GType, give it a 'g_type_name' wrapper, and fix up the rest of the world so we no longer treat a GType as a special integer.
Review of attachment 206146 [details] [review]: Looks good, thanks.
Attachment 205790 [details] pushed as 350727c - tests: Remove all useless logging from tests Attachment 205791 [details] pushed as 97f0be7 - util: Correct the create_proto functions created by DEFINE_PROTO Attachment 206146 [details] pushed as f1dc318 - Represent GTypes as a new boxed wrapper type