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 581691 - Improve support for integer types and arrays.
Improve support for integer types and arrays.
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on: 581682
Blocks:
 
 
Reported: 2009-05-07 04:13 UTC by C. Scott Ananian
Modified: 2009-05-18 17:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Improve-support-for-machine-dependent-integer-types.patch (12.91 KB, patch)
2009-05-07 04:13 UTC, C. Scott Ananian
needs-work Details | Review
Allow-passing-a-JS-string-as-a-C-array-of-int8-uint8.patch (6.13 KB, patch)
2009-05-07 04:13 UTC, C. Scott Ananian
needs-work Details | Review

Description C. Scott Ananian 2009-05-07 04:13:06 UTC
We use a centralized function to convert all the machine-dependent integer
types into explicitly-sized machine-independent types.  This code should
constant-fold nicely under optimization.  We also remove some dead code
involving type tags.  We translate GType as an appropriately sized integer,
to allow invocation of generic functions which take a GType parameter.

We implement support for marshalling arrays of integer types from
javascript to C code.  Note that we don't check ranges here, we just use
truncation, so it is the caller's responsibility to ensure that the input
to a GI function is of appropriate magnitude.  Test cases against the
'Everything' module show how this works.

The second patch allows passing values into C functions which take arrays of small integers using the standard "one value per character" encoding of buffers as JavaScript strings.  This is primarily useful for methods which take binary buffers as gint8* or uint8* (instead of char *).  More test cases included to show how it works.

Depends on the improvements to the Everything typelib in bug 581682.
Comment 1 C. Scott Ananian 2009-05-07 04:13:27 UTC
Created attachment 134160 [details] [review]
Improve-support-for-machine-dependent-integer-types.patch
Comment 2 C. Scott Ananian 2009-05-07 04:13:45 UTC
Created attachment 134161 [details] [review]
Allow-passing-a-JS-string-as-a-C-array-of-int8-uint8.patch
Comment 3 Johan Bilien 2009-05-12 15:40:04 UTC
(In reply to comment #1)
> Created an attachment (id=134160) [edit]
> Improve-support-for-machine-dependent-integer-types.patch
> 

I think this one changes how 64bit long are handled? They used to be converted to JS double (JS_ValueToNumber), now it seems we'd reach the g_assert_not_reached();
 in gjs_array_to_intarray?
Comment 4 Johan Bilien 2009-05-12 15:43:41 UTC
(In reply to comment #2)
> Created an attachment (id=134161) [edit]
> Allow-passing-a-JS-string-as-a-C-array-of-int8-uint8.patch
> 

I think gjs_string_to_intarray should throw an exception when it fails

Looks good to me otherwise

Comment 5 Havoc Pennington 2009-05-13 22:51:08 UTC
Pushed with an exception added to gjs_string_to_intarray

jobi it looks like 64-bit long are still converted to double when standalone, it's just that they aren't handled in arrays?
Comment 6 C. Scott Ananian 2009-05-18 16:52:07 UTC
havoc: i believe that's right, although I'm not certain that's the question jobi was asking.  Jobi: I believe the assert_not_reached is in fact never reached; you'd never get there.  I think you were concerned about:


-#if (GLIB_SIZEOF_LONG == 8)
-    case GI_TYPE_TAG_ULONG:
-    case GI_TYPE_TAG_SIZE:
-#endif
     case GI_TYPE_TAG_UINT64: {
         double v;
         if (!JS_ValueToNumber(context, value, &v))
@@ -644,6 +741,16 @@ gjs_value_to_g_argument(JSContext      *context,
         }
         break;
 
+    case GI_TYPE_TAG_INT:
+    case GI_TYPE_TAG_UINT:
+    case GI_TYPE_TAG_LONG:
+    case GI_TYPE_TAG_ULONG:
+    case GI_TYPE_TAG_SIZE:
+    case GI_TYPE_TAG_SSIZE:
+    case GI_TYPE_TAG_GTYPE:
+        /* these types are converted by normalize_int_types */
+        g_assert_not_reached();

but GI_TYPE_TAG_ULONG (etc) is never seen in that switch statement; normalize_int_types ensures that it would be converted to GI_TYPE_TAG_UINT32 or GI_TYPE_TAG_UINT64 as appropriate, so you'd never reach the g_assert_not_reached().
Comment 7 Johan Bilien 2009-05-18 17:19:50 UTC
(In reply to comment #6)
> havoc: i believe that's right, although I'm not certain that's the question
> jobi was asking.  Jobi: I believe the assert_not_reached is in fact never
> reached; you'd never get there.  I think you were concerned about:
> 
> 
> -#if (GLIB_SIZEOF_LONG == 8)
> -    case GI_TYPE_TAG_ULONG:
> -    case GI_TYPE_TAG_SIZE:
> -#endif
>      case GI_TYPE_TAG_UINT64: {
>          double v;
>          if (!JS_ValueToNumber(context, value, &v))
> @@ -644,6 +741,16 @@ gjs_value_to_g_argument(JSContext      *context,
>          }
>          break;
> 
> +    case GI_TYPE_TAG_INT:
> +    case GI_TYPE_TAG_UINT:
> +    case GI_TYPE_TAG_LONG:
> +    case GI_TYPE_TAG_ULONG:
> +    case GI_TYPE_TAG_SIZE:
> +    case GI_TYPE_TAG_SSIZE:
> +    case GI_TYPE_TAG_GTYPE:
> +        /* these types are converted by normalize_int_types */
> +        g_assert_not_reached();
> 
> but GI_TYPE_TAG_ULONG (etc) is never seen in that switch statement;
> normalize_int_types ensures that it would be converted to GI_TYPE_TAG_UINT32 or
> GI_TYPE_TAG_UINT64 as appropriate, so you'd never reach the
> g_assert_not_reached().
> 


Yeah it's right, I somehow got confused by gjs_array_to_array not handling 64bit ints, but i think that's fine