[gjs/wip/gcampax/70-arg-cache: 2/5] arg-cache: Save space in enum bounds



commit 8aefae19fefc3509719f7d142fa4a9c523c15bb1
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat Jul 18 12:25:26 2020 -0700

    arg-cache: Save space in enum bounds
    
    We were storing a 64-bit int for the min and max values of an enum type.
    However, the documentation of gobject-introspection notes that enum
    values are always representable in 32 bits, either signed or unsigned,
    and the use of a 64-bit return type is to easily allow for both.
    
    We store the bounds in 32-bit unsigned bitfields, and use a flag that we
    add to the slop space in GjsArgumentCache (which we can share with
    number types as well) to indicate whether the bounds should be
    interpreted as signed or unsigned.

 gi/arg-cache.cpp | 36 +++++++++++++++++++++++++-----------
 gi/arg-cache.h   |  6 +++---
 2 files changed, 28 insertions(+), 14 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index 249a373e..026fde0e 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -391,7 +391,7 @@ static bool gjs_marshal_integer_in_in(JSContext* cx, GjsArgumentCache* self,
                                       JS::HandleValue value) {
     GITypeTag tag = self->contents.number.number_tag;
 
-    if (self->contents.number.is_unsigned) {
+    if (self->is_unsigned) {
         uint32_t number;
         if (!JS::ToUint32(cx, value, &number))
             return false;
@@ -515,19 +515,27 @@ static bool gjs_marshal_enum_in_in(JSContext* cx, GjsArgumentCache* self,
     if (!JS::ToInt64(cx, value, &number))
         return false;
 
-    if (number > self->contents.enum_type.enum_max ||
-        number < self->contents.enum_type.enum_min) {
+    // Unpack the values from their uint32_t bitfield. See note in
+    // gjs_arg_cache_build_enum_bounds().
+    int64_t min, max;
+    if (self->is_unsigned) {
+        min = self->contents.enum_type.enum_min;
+        max = self->contents.enum_type.enum_max;
+    } else {
+        min = static_cast<int32_t>(self->contents.enum_type.enum_min);
+        max = static_cast<int32_t>(self->contents.enum_type.enum_max);
+    }
+
+    if (number > max || number < min) {
         gjs_throw(cx, "%" PRId64 " is not a valid value for enum argument %s",
                   number, self->arg_name);
         return false;
     }
 
-    if (self->contents.enum_type.enum_max <= G_MAXINT32)
-        arg->v_int = number;
-    else if (self->contents.enum_type.enum_max <= G_MAXUINT32)
+    if (self->is_unsigned)
         arg->v_uint = number;
     else
-        arg->v_int64 = number;
+        arg->v_int = number;
 
     return true;
 }
@@ -1026,8 +1034,14 @@ static void gjs_arg_cache_build_enum_bounds(GjsArgumentCache* self,
             min = value;
     }
 
-    self->contents.enum_type.enum_min = min;
-    self->contents.enum_type.enum_max = max;
+    // From the docs for g_value_info_get_value(): "This will always be
+    // representable as a 32-bit signed or unsigned value. The use of gint64 as
+    // the return type is to allow both."
+    // We stuff them both into unsigned 32-bit fields, and use a flag to tell
+    // whether we have to compare them as signed.
+    self->contents.enum_type.enum_min = static_cast<uint32_t>(min);
+    self->contents.enum_type.enum_max = static_cast<uint32_t>(max);
+    self->is_unsigned = min >= 0 && max > G_MAXINT32;
 }
 
 static void gjs_arg_cache_build_flags_mask(GjsArgumentCache* self,
@@ -1211,14 +1225,14 @@ static bool gjs_arg_cache_build_normal_in_arg(JSContext* cx,
         case GI_TYPE_TAG_INT32:
             self->marshal_in = gjs_marshal_integer_in_in;
             self->contents.number.number_tag = tag;
-            self->contents.number.is_unsigned = false;
+            self->is_unsigned = false;
             break;
 
         case GI_TYPE_TAG_UINT8:
         case GI_TYPE_TAG_UINT16:
             self->marshal_in = gjs_marshal_integer_in_in;
             self->contents.number.number_tag = tag;
-            self->contents.number.is_unsigned = true;
+            self->is_unsigned = true;
             break;
 
         case GI_TYPE_TAG_UINT32:
diff --git a/gi/arg-cache.h b/gi/arg-cache.h
index 50618d3f..8fa1e915 100644
--- a/gi/arg-cache.h
+++ b/gi/arg-cache.h
@@ -60,6 +60,7 @@ struct GjsArgumentCache {
     bool skip_out : 1;
     GITransfer transfer : 2;
     bool nullable : 1;
+    bool is_unsigned : 1;  // number and enum only
 
     union {
         // for explicit array only
@@ -76,7 +77,6 @@ struct GjsArgumentCache {
 
         struct {
             GITypeTag number_tag : 5;
-            bool is_unsigned : 1;
         } number;
 
         // boxed / union / GObject
@@ -90,8 +90,8 @@ struct GjsArgumentCache {
 
         // enum / flags
         struct {
-            int64_t enum_min;
-            int64_t enum_max;
+            uint32_t enum_min;
+            uint32_t enum_max;
         } enum_type;
         uint64_t flags_mask;
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]