gdbus-codegen: Generate marshallers for method handlers signals

Get rid completely of the usage of the generic marshaller in gdbus
generated code, using instead specific marshallers.

Code is not yet optimized fully since we may still have duplicated
functions to be generated.

Closes: https://gitlab.gnome.org/GNOME/glib/-/issues/3028

gio/gdbus-2.0/codegen/codegen.py

@@ -2136,7 +2136,7 @@ class CodeGenerator:
                     "    G_STRUCT_OFFSET (%sIface, handle_%s),\n"
                     "    g_signal_accumulator_true_handled,\n"
                     "    NULL,\n"  # accu_data
-                    "    g_cclosure_marshal_generic,\n"
+                    f"      {i.name_lower}_method_marshal_{m.name_lower},\n"
                     "    G_TYPE_BOOLEAN,\n"
                     "    %d,\n"
                     "    G_TYPE_DBUS_METHOD_INVOCATION"
@@ -2525,34 +2525,44 @@ class CodeGenerator:
 
     # ---------------------------------------------------------------------------------------------------
 
-    def generate_marshaller(self, func_name, in_args):
+    def generate_marshaller(self, func_name, in_args, ret_arg=None):
-        self.generate_marshaller_declaration(func_name)
+        self.generate_marshaller_declaration(func_name, uses_ret=ret_arg is not None)
         self.outfile.write("{\n")
-        self.generate_marshaller_body(func_name, in_args)
+        self.generate_marshaller_body(func_name, in_args, ret_arg)
         self.outfile.write("}\n" "\n")
 
-    def generate_marshaller_declaration(self, func_name):
+    def generate_marshaller_declaration(self, func_name, uses_ret=False):
         self.outfile.write(
             "static void\n"
             f"{func_name} (\n"
             "    GClosure     *closure,\n"
-            "    GValue       *return_value G_GNUC_UNUSED,\n"
+            f"    GValue       *return_value{' G_GNUC_UNUSED' if not uses_ret else ''},\n"
             "    unsigned int  n_param_values,\n"
             "    const GValue *param_values,\n"
             "    void         *invocation_hint G_GNUC_UNUSED,\n"
             "    void         *marshal_data)\n"
         )
 
-    def generate_marshaller_body(self, func_name, in_args=[]):
+    def generate_marshaller_body(self, func_name, in_args=[], ret_arg=None):
         marshal_func_type = f"{utils.uscore_to_camel_case(func_name)}Func"
         self.outfile.write(
-            f"  typedef void (*{marshal_func_type})\n"
+            f"  typedef {ret_arg.ctype_in if ret_arg else 'void '}(*{marshal_func_type})\n"
             "       (void *data1,\n"
             + "".join([f"        {a.ctype_in}arg_{a.name},\n" for a in in_args])
             + "        void *data2);\n"
             f"  {marshal_func_type} callback;\n"
             "  GCClosure *cc = (GCClosure*) closure;\n"
             f"  void *data1, *data2;\n"
+        )
+
+        if ret_arg:
+            self.outfile.write(
+                f"  {ret_arg.ctype_in}v_return;\n"
+                "\n"
+                "  g_return_if_fail (return_value != NULL);"
+            )
+
+        self.outfile.write(
             "\n"
             f"  g_return_if_fail (n_param_values == {len(in_args) + 1});\n"
             "\n"
@@ -2570,30 +2580,48 @@ class CodeGenerator:
             f"  callback = ({marshal_func_type})\n"
             "    (marshal_data ? marshal_data : cc->callback);\n"
             "\n"
-            "  callback (data1,\n"
+        )
+
+        prefix = ""
+        if ret_arg:
+            self.outfile.write("  v_return =\n")
+            prefix = 2 * " "
+
+        self.outfile.write(
+            f"{prefix}  callback (data1,\n"
             + "".join(
                 [
-                    f"            {in_args[i].gvalue_get} (param_values + {i+1}),\n"
+                    f"{prefix}            {in_args[i].gvalue_get} (param_values + {i+1}),\n"
                     for i in range(len(in_args))
                 ]
             )
-            + "            data2);\n"
+            + f"{prefix}            data2);\n"
+        )
+
+        if ret_arg:
+            self.outfile.write(
+                "\n" f"  {ret_arg.gvalue_set} (return_value, v_return);\n"
             )
 
     def generate_marshaller_for_type(self, i, t):
-        assert isinstance(t, dbustypes.Signal)
+        assert isinstance(t, (dbustypes.Signal, dbustypes.Method))
 
         kind_uscore = utils.camel_case_to_uscore(t.__class__.__name__.lower())
 
         self.generate_marshaller(
             func_name=f"{i.name_lower}_{kind_uscore}_marshal_{t.name_lower}",
-            in_args=t.args,
+            in_args=t.marshaller_in_args,
+            ret_arg=t.marshaller_ret_arg,
         )
 
     def generate_signal_marshallers(self, i):
         for s in i.signals:
             self.generate_marshaller_for_type(i, s)
 
+    def generate_method_marshallers(self, i):
+        for m in i.methods:
+            self.generate_marshaller_for_type(i, m)
+
     # ---------------------------------------------------------------------------------------------------
 
     def generate_method_calls(self, i):
@@ -5321,6 +5349,7 @@ class CodeGenerator:
             self.generate_signals_enum_for_interface(i)
             self.generate_introspection_for_interface(i)
             self.generate_signal_marshallers(i)
+            self.generate_method_marshallers(i)
             self.generate_interface(i)
             self.generate_property_accessors(i)
             self.generate_signal_emitters(i)

gio/gdbus-2.0/codegen/dbustypes.py

@@ -84,6 +84,7 @@ class Arg:
         self.format_out = "@" + self.signature
         self.gvariant_get = "XXX"
         self.gvalue_get = "g_value_get_variant"
+        self.gvalue_set = "g_value_take_variant"
         self.array_annotation = ""
 
         if not utils.lookup_annotation(
@@ -100,6 +101,7 @@ class Arg:
                 self.format_out = "b"
                 self.gvariant_get = "g_variant_get_boolean"
                 self.gvalue_get = "g_value_get_boolean"
+                self.gvalue_set = "g_value_set_boolean"
             elif self.signature == "y":
                 self.ctype_in_g = "guchar "
                 self.ctype_in = "guchar "
@@ -111,6 +113,7 @@ class Arg:
                 self.format_out = "y"
                 self.gvariant_get = "g_variant_get_byte"
                 self.gvalue_get = "g_value_get_uchar"
+                self.gvalue_set = "g_value_set_uchar"
             elif self.signature == "n":
                 self.ctype_in_g = "gint "
                 self.ctype_in = "gint16 "
@@ -122,6 +125,7 @@ class Arg:
                 self.format_out = "n"
                 self.gvariant_get = "g_variant_get_int16"
                 self.gvalue_get = "g_value_get_int"
+                self.gvalue_set = "g_value_set_int"
             elif self.signature == "q":
                 self.ctype_in_g = "guint "
                 self.ctype_in = "guint16 "
@@ -133,6 +137,7 @@ class Arg:
                 self.format_out = "q"
                 self.gvariant_get = "g_variant_get_uint16"
                 self.gvalue_get = "g_value_get_uint"
+                self.gvalue_set = "g_value_set_uint"
             elif self.signature == "i":
                 self.ctype_in_g = "gint "
                 self.ctype_in = "gint "
@@ -144,6 +149,7 @@ class Arg:
                 self.format_out = "i"
                 self.gvariant_get = "g_variant_get_int32"
                 self.gvalue_get = "g_value_get_int"
+                self.gvalue_set = "g_value_set_int"
             elif self.signature == "u":
                 self.ctype_in_g = "guint "
                 self.ctype_in = "guint "
@@ -155,6 +161,7 @@ class Arg:
                 self.format_out = "u"
                 self.gvariant_get = "g_variant_get_uint32"
                 self.gvalue_get = "g_value_get_uint"
+                self.gvalue_set = "g_value_set_uint"
             elif self.signature == "x":
                 self.ctype_in_g = "gint64 "
                 self.ctype_in = "gint64 "
@@ -166,6 +173,7 @@ class Arg:
                 self.format_out = "x"
                 self.gvariant_get = "g_variant_get_int64"
                 self.gvalue_get = "g_value_get_int64"
+                self.gvalue_set = "g_value_set_int64"
             elif self.signature == "t":
                 self.ctype_in_g = "guint64 "
                 self.ctype_in = "guint64 "
@@ -177,6 +185,7 @@ class Arg:
                 self.format_out = "t"
                 self.gvariant_get = "g_variant_get_uint64"
                 self.gvalue_get = "g_value_get_uint64"
+                self.gvalue_set = "g_value_set_uint64"
             elif self.signature == "d":
                 self.ctype_in_g = "gdouble "
                 self.ctype_in = "gdouble "
@@ -188,6 +197,7 @@ class Arg:
                 self.format_out = "d"
                 self.gvariant_get = "g_variant_get_double"
                 self.gvalue_get = "g_value_get_double"
+                self.gvalue_set = "g_value_set_double"
             elif self.signature == "s":
                 self.ctype_in_g = "const gchar *"
                 self.ctype_in = "const gchar *"
@@ -200,6 +210,7 @@ class Arg:
                 self.format_out = "s"
                 self.gvariant_get = "g_variant_get_string"
                 self.gvalue_get = "g_value_get_string"
+                self.gvalue_set = "g_value_set_string"
             elif self.signature == "o":
                 self.ctype_in_g = "const gchar *"
                 self.ctype_in = "const gchar *"
@@ -212,6 +223,7 @@ class Arg:
                 self.format_out = "o"
                 self.gvariant_get = "g_variant_get_string"
                 self.gvalue_get = "g_value_get_string"
+                self.gvalue_set = "g_value_set_string"
             elif self.signature == "g":
                 self.ctype_in_g = "const gchar *"
                 self.ctype_in = "const gchar *"
@@ -224,6 +236,7 @@ class Arg:
                 self.format_out = "g"
                 self.gvariant_get = "g_variant_get_string"
                 self.gvalue_get = "g_value_get_string"
+                self.gvalue_set = "g_value_set_string"
             elif self.signature == "ay":
                 self.ctype_in_g = "const gchar *"
                 self.ctype_in = "const gchar *"
@@ -236,6 +249,7 @@ class Arg:
                 self.format_out = "^ay"
                 self.gvariant_get = "g_variant_get_bytestring"
                 self.gvalue_get = "g_value_get_string"
+                self.gvalue_set = "g_value_set_string"
             elif self.signature == "as":
                 self.ctype_in_g = "const gchar *const *"
                 self.ctype_in = "const gchar *const *"
@@ -248,6 +262,7 @@ class Arg:
                 self.format_out = "^as"
                 self.gvariant_get = "g_variant_get_strv"
                 self.gvalue_get = "g_value_get_boxed"
+                self.gvalue_set = "g_value_take_boxed"
                 self.array_annotation = "(array zero-terminated=1)"
             elif self.signature == "ao":
                 self.ctype_in_g = "const gchar *const *"
@@ -261,6 +276,7 @@ class Arg:
                 self.format_out = "^ao"
                 self.gvariant_get = "g_variant_get_objv"
                 self.gvalue_get = "g_value_get_boxed"
+                self.gvalue_set = "g_value_take_boxed"
                 self.array_annotation = "(array zero-terminated=1)"
             elif self.signature == "aay":
                 self.ctype_in_g = "const gchar *const *"
@@ -274,6 +290,7 @@ class Arg:
                 self.format_out = "^aay"
                 self.gvariant_get = "g_variant_get_bytestring_array"
                 self.gvalue_get = "g_value_get_boxed"
+                self.gvalue_set = "g_value_take_boxed"
                 self.array_annotation = "(array zero-terminated=1)"
 
         for a in self.annotations:
@@ -336,6 +353,20 @@ class Method:
         if utils.lookup_annotation(self.annotations, "org.gtk.GDBus.C.UnixFD"):
             self.unix_fd = True
 
+        self.marshaller_ret_arg = Arg("return", "b")
+        self.marshaller_ret_arg.post_process(None, None, None, None, None)
+
+        method_invocation_arg = Arg("method_invocation", None)
+        method_invocation_arg.ctype_in = "GDBusMethodInvocation *"
+        method_invocation_arg.gvalue_get = "g_value_get_object"
+        self.marshaller_in_args = [method_invocation_arg] + self.in_args
+
+        if self.unix_fd:
+            fd_list_arg = Arg("fd_list", None)
+            fd_list_arg.ctype_in = "GUnixFDList *"
+            fd_list_arg.gvalue_get = "g_value_get_object"
+            self.marshaller_in_args.insert(0, fd_list_arg)
+
         for a in self.annotations:
             a.post_process(interface_prefix, cns, cns_upper, cns_lower, self)
 
@@ -386,6 +417,9 @@ class Signal:
         ):
             self.deprecated = True
 
+        self.marshaller_ret_arg = None
+        self.marshaller_in_args = self.args
+
         for a in self.annotations:
             a.post_process(interface_prefix, cns, cns_upper, cns_lower, self)
 

gio/tests/codegen.py

@@ -903,6 +903,247 @@ G_END_DECLS
             )
             index += 1
 
+    @unittest.skipIf(on_win32(), "requires /dev/stdout")
+    def test_generate_methods_marshaller_simple_method(self):
+        """Test that methods marshaller is generated for simple method"""
+        interface_xml = """
+            <node>
+              <interface name="org.project.CallableIface">
+                <method name="SimpleMethod"/>
+              </interface>
+              <interface name="org.project.OtherCallableIface">
+                <method name="SimpleMethod"/>
+              </interface>
+            </node>"""
+
+        result = self.runCodegenWithInterface(
+            interface_xml, "--output", "/dev/stdout", "--body"
+        )
+        stripped_out = result.out.strip()
+        self.assertFalse(result.err)
+        self.assertIs(stripped_out.count("g_cclosure_marshal_generic"), 0)
+
+        func_name = "org_project_callable_iface_method_marshal_simple_method"
+        self.assertIs(stripped_out.count(f"{func_name},"), 1)
+        self.assertIs(stripped_out.count(f"{func_name} ("), 1)
+
+        func_name = "org_project_other_callable_iface_method_marshal_simple_method"
+        self.assertIs(stripped_out.count(f"{func_name},"), 1)
+        self.assertIs(stripped_out.count(f"{func_name} ("), 1)
+
+        self.assertIs(stripped_out.count("g_value_get_object (param_values + 1)"), 2)
+        self.assertIs(
+            stripped_out.count("g_value_set_boolean (return_value, v_return);"), 2
+        )
+
+    @unittest.skipIf(on_win32(), "requires /dev/stdout")
+    def test_generate_methods_marshaller_single_typed_in_args(self):
+        """Test that methods marshallers are generated for each known type"""
+        for t, props in self.ARGUMENTS_TYPES.items():
+            camel_type = t[0].upper() + t[1:]
+            interface_xml = f"""
+            <node>
+            <interface name="org.project.UsefulInterface">
+                <method name="SingleArgMethod{camel_type}">
+                    <arg name="arg_{t}" type="{props.get("variant_type", t)}"/>
+                </method>
+            </interface>
+            </node>"""
+
+            result = self.runCodegenWithInterface(
+                interface_xml, "--output", "/dev/stdout", "--body"
+            )
+            stripped_out = result.out.strip()
+            self.assertFalse(result.err)
+            self.assertEqual(stripped_out.count("g_cclosure_marshal_generic"), 0)
+
+            func_name = (
+                f"org_project_useful_interface_method_marshal_single_arg_method_{t}"
+            )
+            self.assertIs(stripped_out.count(f"{func_name},"), 1)
+            self.assertIs(stripped_out.count(f"{func_name} ("), 1)
+            self.assertIs(
+                stripped_out.count("g_value_get_object (param_values + 1)"), 1
+            )
+            self.assertIs(
+                stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1
+            )
+            self.assertIs(
+                stripped_out.count(
+                    f"g_value_get_{props['value_type']} (param_values + 2)"
+                ),
+                1,
+            )
+
+    @unittest.skipIf(on_win32(), "requires /dev/stdout")
+    def test_generate_methods_marshaller_single_typed_out_args(self):
+        """Test that methods marshallers are generated for each known type"""
+        for t, props in self.ARGUMENTS_TYPES.items():
+            camel_type = t[0].upper() + t[1:]
+            interface_xml = f"""
+            <node>
+            <interface name="org.project.UsefulInterface">
+                <method name="SingleArgMethod{camel_type}">
+                    <arg name="arg_{t}" type="{props.get("variant_type", t)}" direction="out"/>
+                </method>
+            </interface>
+            </node>"""
+
+            result = self.runCodegenWithInterface(
+                interface_xml, "--output", "/dev/stdout", "--body"
+            )
+            stripped_out = result.out.strip()
+            self.assertFalse(result.err)
+            self.assertEqual(stripped_out.count("g_cclosure_marshal_generic"), 0)
+
+            func_name = (
+                f"org_project_useful_interface_method_marshal_single_arg_method_{t}"
+            )
+            self.assertIs(stripped_out.count(f"{func_name},"), 1)
+            self.assertIs(stripped_out.count(f"{func_name} ("), 1)
+            self.assertIs(
+                stripped_out.count("g_value_get_object (param_values + 1)"), 1
+            )
+            self.assertIs(
+                stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1
+            )
+            self.assertIs(stripped_out.count("(param_values + 2)"), 0)
+
+    @unittest.skipIf(on_win32(), "requires /dev/stdout")
+    def test_generate_methods_marshallers_multiple_in_args(self):
+        """Test that methods marshallers are generated"""
+        generated_args = [
+            f"<arg name='an_{t}' type='{props.get('variant_type', t)}'/>\n"
+            for t, props in self.ARGUMENTS_TYPES.items()
+        ]
+
+        interface_xml = f"""
+            <node>
+              <interface name="org.project.CallableIface">
+                <method name="MethodWithManyArgs">
+                    {''.join(generated_args)}
+                </method>
+              </interface>
+            </node>"""
+
+        result = self.runCodegenWithInterface(
+            interface_xml, "--output", "/dev/stdout", "--body"
+        )
+        stripped_out = result.out.strip()
+        self.assertFalse(result.err)
+        self.assertIs(stripped_out.count("g_cclosure_marshal_generic"), 0)
+
+        func_name = f"org_project_callable_iface_method_marshal_method_with_many_args"
+        self.assertIs(stripped_out.count(f"{func_name},"), 1)
+        self.assertIs(stripped_out.count(f"{func_name} ("), 1)
+
+        # Check access to MultipleArgsMethod arguments
+        index = 1
+        self.assertIs(
+            stripped_out.count(f"g_value_get_object (param_values + {index})"), 1
+        )
+        index += 1
+
+        for props in self.ARGUMENTS_TYPES.values():
+            self.assertIs(
+                stripped_out.count(
+                    f"g_value_get_{props['value_type']} (param_values + {index})"
+                ),
+                1,
+            )
+            index += 1
+
+        self.assertIs(
+            stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1
+        )
+
+    @unittest.skipIf(on_win32(), "requires /dev/stdout")
+    def test_generate_methods_marshallers_multiple_out_args(self):
+        """Test that methods marshallers are generated"""
+        generated_args = [
+            f"<arg name='an_{t}' type='{props.get('variant_type', t)}' direction='out'/>\n"
+            for t, props in self.ARGUMENTS_TYPES.items()
+        ]
+
+        interface_xml = f"""
+            <node>
+              <interface name="org.project.CallableIface">
+                <method name="MethodWithManyArgs">
+                    {''.join(generated_args)}
+                </method>
+              </interface>
+            </node>"""
+
+        result = self.runCodegenWithInterface(
+            interface_xml, "--output", "/dev/stdout", "--body"
+        )
+        stripped_out = result.out.strip()
+        self.assertFalse(result.err)
+        self.assertIs(stripped_out.count("g_cclosure_marshal_generic"), 0)
+
+        func_name = f"org_project_callable_iface_method_marshal_method_with_many_args"
+        self.assertIs(stripped_out.count(f"{func_name},"), 1)
+        self.assertIs(stripped_out.count(f"{func_name} ("), 1)
+
+        # Check access to MultipleArgsMethod arguments
+        index = 1
+        self.assertIs(
+            stripped_out.count(f"g_value_get_object (param_values + {index})"), 1
+        )
+        index += 1
+
+        for index in range(index, len(self.ARGUMENTS_TYPES)):
+            self.assertIs(stripped_out.count(f"(param_values + {index})"), 0)
+
+        self.assertIs(
+            stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1
+        )
+
+    @unittest.skipIf(on_win32(), "requires /dev/stdout")
+    def test_generate_methods_marshallers_with_unix_fds(self):
+        """Test an interface with `h` arguments"""
+        interface_xml = """
+            <node>
+              <interface name="test.FDPassing">
+                <method name="HelloFD">
+                  <annotation name="org.gtk.GDBus.C.UnixFD" value="1"/>
+                  <arg name="greeting" direction="in" type="s"/>
+                  <arg name="response" direction="out" type="s"/>
+                </method>
+              </interface>
+            </node>"""
+
+        result = self.runCodegenWithInterface(
+            interface_xml, "--output", "/dev/stdout", "--body"
+        )
+        stripped_out = result.out.strip()
+        self.assertFalse(result.err)
+        self.assertIs(stripped_out.count("g_cclosure_marshal_generic"), 0)
+
+        func_name = f"test_fdpassing_method_marshal_hello_fd"
+        self.assertIs(stripped_out.count(f"{func_name},"), 1)
+        self.assertIs(stripped_out.count(f"{func_name} ("), 1)
+
+        index = 1
+        self.assertIs(
+            stripped_out.count(f"g_value_get_object (param_values + {index})"), 1
+        )
+        index += 1
+
+        self.assertIs(
+            stripped_out.count(f"g_value_get_object (param_values + {index})"), 1
+        )
+
+        index += 1
+        self.assertIs(
+            stripped_out.count(f"g_value_get_string (param_values + {index})"), 1
+        )
+        index += 1
+
+        self.assertIs(
+            stripped_out.count("g_value_set_boolean (return_value, v_return);"), 1
+        )
+
     def test_generate_valid_docbook(self):
         """Test the basic functionality of the docbook generator."""
         xml_contents = """