[gjs: 2/5] gi: Support caller-allocates GValue arguments

[Philip Chimento <pchimento src gnome org>]

commit 3500148f375310e18dfe894e3c61906d74ea739e
Author: Marco Trevisan (Treviño) <mail 3v1n0 net>
Date:   Mon Sep 28 16:24:49 2020 +0200

    gi: Support caller-allocates GValue arguments
    
    When we were handling a GValue caller-allocates argument we actually
    were changing a new temporary GArgument copy that we actually didn't end
    up passing to the function that was changing it.
    
    To ensure that this doesn't happen:
     - When a caller-allocates argument is passed to a function, use the
       actual GIArgument pointer and not deference it.
     - Include an argument flag that can be used at conversion time to make
       clear that we're working on the actual caller allocated argument and
       not in a gjs copy of it.
     - When converting a JSValue to GValue, act on the caller allocated
       (possibly by arg-cache) memory, instead of setting a new GValue.
    
    As per this we can enable the `vfunc_caller_allocated_out_parameter`
    test now.
    
    This doesn't seem enough to fix GNOME/gjs#74 testcase though as the gtk
    introspection doesn't give us enough information about being
    caller-allocated.

 gi/arg-cache.cpp                        |  5 ++++-
 gi/arg-cache.h                          |  2 +-
 gi/arg.cpp                              | 16 +++++++++++++---
 gi/arg.h                                |  1 +
 gi/function.cpp                         | 17 ++++++++++++++---
 installed-tests/js/testGIMarshalling.js |  2 +-
 6 files changed, 34 insertions(+), 9 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index 550314b0..1f294797 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -1616,6 +1616,8 @@ bool gjs_arg_cache_build_arg(JSContext* cx, GjsArgumentCache* self,
     GjsArgumentFlags flags = GjsArgumentFlags::NONE;
     if (g_arg_info_may_be_null(arg))
         flags |= GjsArgumentFlags::MAY_BE_NULL;
+    if (g_arg_info_is_caller_allocates(arg))
+        flags |= GjsArgumentFlags::CALLER_ALLOCATES;
     self->flags = flags;
 
     if (direction == GI_DIRECTION_IN)
@@ -1625,7 +1627,8 @@ bool gjs_arg_cache_build_arg(JSContext* cx, GjsArgumentCache* self,
     *inc_counter_out = true;
 
     GITypeTag type_tag = g_type_info_get_tag(&self->type_info);
-    if (direction == GI_DIRECTION_OUT && g_arg_info_is_caller_allocates(arg)) {
+    if (direction == GI_DIRECTION_OUT &&
+        (flags & GjsArgumentFlags::CALLER_ALLOCATES)) {
         if (type_tag != GI_TYPE_TAG_INTERFACE) {
             gjs_throw(cx,
                       "Unsupported type %s for argument %s with (out "
diff --git a/gi/arg-cache.h b/gi/arg-cache.h
index 70b8c64e..669ff454 100644
--- a/gi/arg-cache.h
+++ b/gi/arg-cache.h
@@ -44,7 +44,7 @@ struct GjsArgumentCache {
     bool skip_in : 1;
     bool skip_out : 1;
     GITransfer transfer : 2;
-    GjsArgumentFlags flags : 1;
+    GjsArgumentFlags flags : 2;
     bool is_unsigned : 1;  // number and enum only
 
     union {
diff --git a/gi/arg.cpp b/gi/arg.cpp
index 0a5d16db..0923c41f 100644
--- a/gi/arg.cpp
+++ b/gi/arg.cpp
@@ -1329,7 +1329,8 @@ intern_gdk_atom(const char *name,
 static bool value_to_interface_gi_argument(
     JSContext* cx, JS::HandleValue value, GIBaseInfo* interface_info,
     GIInfoType interface_type, GITransfer transfer, bool expect_object,
-    GIArgument* arg, GjsArgumentType arg_type, bool* report_type_mismatch) {
+    GIArgument* arg, GjsArgumentType arg_type, GjsArgumentFlags flags,
+    bool* report_type_mismatch) {
     g_assert(report_type_mismatch);
     GType gtype;
 
@@ -1360,8 +1361,15 @@ static bool value_to_interface_gi_argument(
                           g_type_name(gtype));
 
     if (gtype == G_TYPE_VALUE) {
-        GValue gvalue = G_VALUE_INIT;
+        if (flags & GjsArgumentFlags::CALLER_ALLOCATES) {
+            if (!gjs_value_to_g_value_no_copy(cx, value,
+                                              gjs_arg_get<GValue*>(arg)))
+                return false;
+
+            return true;
+        }
 
+        GValue gvalue = G_VALUE_INIT;
         if (!gjs_value_to_g_value(cx, value, &gvalue)) {
             gjs_arg_unset<void*>(arg);
             return false;
@@ -1771,7 +1779,7 @@ bool gjs_value_to_g_argument(JSContext* context, JS::HandleValue value,
 
             if (!value_to_interface_gi_argument(
                     context, value, interface_info, interface_type, transfer,
-                    expect_object, arg, arg_type, &report_type_mismatch))
+                    expect_object, arg, arg_type, flags, &report_type_mismatch))
                 wrong = true;
         }
         break;
@@ -2030,6 +2038,8 @@ gjs_value_to_arg(JSContext      *context,
 
     if (g_arg_info_may_be_null(arg_info))
         flags |= GjsArgumentFlags::MAY_BE_NULL;
+    if (g_arg_info_is_caller_allocates(arg_info))
+        flags |= GjsArgumentFlags::CALLER_ALLOCATES;
 
     return gjs_value_to_g_argument(
         context, value, &type_info, g_base_info_get_name(arg_info),
diff --git a/gi/arg.h b/gi/arg.h
index 8b2f4d61..64e0f345 100644
--- a/gi/arg.h
+++ b/gi/arg.h
@@ -32,6 +32,7 @@ typedef enum {
 enum class GjsArgumentFlags : uint8_t {
     NONE = 0,
     MAY_BE_NULL = 1 << 0,
+    CALLER_ALLOCATES = 1 << 1,
 };
 
 [[nodiscard]] char* gjs_argument_display_name(const char* arg_name,
diff --git a/gi/function.cpp b/gi/function.cpp
index 689b8e52..9186d157 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -327,6 +327,14 @@ void GjsCallbackTrampoline::callback_closure(GIArgument** args, void* result) {
     }
 }
 
+inline GIArgument* get_argument_for_arg_info(GIArgInfo* arg_info,
+                                             GIArgument** args, int index) {
+    if (!g_arg_info_is_caller_allocates(arg_info))
+        return *reinterpret_cast<GIArgument**>(args[index]);
+    else
+        return args[index];
+}
+
 bool GjsCallbackTrampoline::callback_closure_inner(
     JSContext* context, JS::HandleObject this_object,
     JS::MutableHandleValue rval, GIArgument** args, GITypeInfo* ret_type,
@@ -393,7 +401,8 @@ bool GjsCallbackTrampoline::callback_closure_inner(
                     g_error("Unable to grow vector");
 
                 GIArgument* arg = args[i + c_args_offset];
-                if (g_arg_info_get_direction(&arg_info) == GI_DIRECTION_INOUT)
+                if (g_arg_info_get_direction(&arg_info) == GI_DIRECTION_INOUT &&
+                    !g_arg_info_is_caller_allocates(&arg_info))
                     arg = *reinterpret_cast<GIArgument**>(arg);
 
                 if (!gjs_value_from_g_argument(context, jsargs[n_jsargs++],
@@ -439,7 +448,8 @@ bool GjsCallbackTrampoline::callback_closure_inner(
                 continue;
 
             if (!gjs_value_to_arg(context, rval, &arg_info,
-                                  *reinterpret_cast<GIArgument **>(args[i + c_args_offset])))
+                                  get_argument_for_arg_info(&arg_info, args,
+                                                            i + c_args_offset)))
                 return false;
 
             break;
@@ -493,7 +503,8 @@ bool GjsCallbackTrampoline::callback_closure_inner(
                 return false;
 
             if (!gjs_value_to_arg(context, elem, &arg_info,
-                                  *(GIArgument **)args[i + c_args_offset]))
+                                  get_argument_for_arg_info(&arg_info, args,
+                                                            i + c_args_offset)))
                 return false;
 
             elem_idx++;
diff --git a/installed-tests/js/testGIMarshalling.js b/installed-tests/js/testGIMarshalling.js
index ddc427c3..b5fbf801 100644
--- a/installed-tests/js/testGIMarshalling.js
+++ b/installed-tests/js/testGIMarshalling.js
@@ -1250,7 +1250,7 @@ describe('Virtual function', function () {
 
     it('marshals a caller-allocated GValue out parameter', function () {
         expect(tester.vfunc_caller_allocated_out_parameter()).toEqual(52);
-    }).pend('https://gitlab.gnome.org/GNOME/gjs/issues/74');
+    });
 
     it('marshals an error out parameter when no error', function () {
         expect(tester.vfunc_meth_with_error(-1)).toBeTruthy();