THRIFT-3178 glib C does not compile

Client: c_glib
Patch: Simon South <ssouth@simonsouth.com>

This closes #581

3 files changed, 204 insertions, 46 deletions

diff --git a/compiler/cpp/src/generate/t_c_glib_generator.cc b/compiler/cpp/src/generate/t_c_glib_generator.cc
index 9f3ff10e7..3971fc28e 100644
--- a/compiler/cpp/src/generate/t_c_glib_generator.cc
+++ b/compiler/cpp/src/generate/t_c_glib_generator.cc
@@ -525,7 +525,6 @@ bool t_c_glib_generator::is_complex_type(t_type* ttype) {
  * Maps a Thrift t_type to a C type.
  */
 string t_c_glib_generator::type_name(t_type* ttype, bool in_typedef, bool is_const) {
-  (void)in_typedef;
   if (ttype->is_base_type()) {
     string bname = base_type_name((t_base_type*)ttype);
 
@@ -543,16 +542,16 @@ string t_c_glib_generator::type_name(t_type* ttype, bool in_typedef, bool is_con
     if (tcontainer->has_cpp_name()) {
       cname = tcontainer->get_cpp_name();
     } else if (ttype->is_map()) {
-      cname = "GHashTable *";
+      cname = "GHashTable";
     } else if (ttype->is_set()) {
       // since a set requires unique elements, use a GHashTable, and
       // populate the keys and values with the same data, using keys for
       // the actual writes and reads.
       // TODO: discuss whether or not to implement TSet, THashSet or GHashSet
-      cname = "GHashTable *";
+      cname = "GHashTable";
     } else if (ttype->is_list()) {
       // TODO: investigate other implementations besides GPtrArray
-      cname = "GPtrArray *";
+      cname = "GPtrArray";
       t_type* etype = ((t_list*)ttype)->get_elem_type();
       if (etype->is_base_type()) {
         t_base_type::t_base tbase = ((t_base_type*)etype)->get_base();
@@ -565,7 +564,7 @@ string t_c_glib_generator::type_name(t_type* ttype, bool in_typedef, bool is_con
         case t_base_type::TYPE_I32:
         case t_base_type::TYPE_I64:
         case t_base_type::TYPE_DOUBLE:
-          cname = "GArray *";
+          cname = "GArray";
           break;
         case t_base_type::TYPE_STRING:
           break;
@@ -575,6 +574,13 @@ string t_c_glib_generator::type_name(t_type* ttype, bool in_typedef, bool is_con
       }
     }
 
+    /* Omit the dereference operator if we are aliasing this type within a
+       typedef, to allow the type to be used more naturally in client code;
+       otherwise, include it */
+    if (!in_typedef) {
+      cname += " *";
+    }
+
     if (is_const) {
       return "const " + cname;
     } else {
@@ -3885,7 +3891,7 @@ void t_c_glib_generator::generate_serialize_list_element(ofstream& out,
                                                          string list,
                                                          string index,
                                                          int error_ret) {
-  t_type* ttype = tlist->get_elem_type();
+  t_type* ttype = get_true_type(tlist->get_elem_type());
 
   // cast to non-const
   string cast = "";
@@ -4194,6 +4200,10 @@ void t_c_glib_generator::declare_local_variable(ofstream& out, t_type* ttype, st
     t_map* tmap = (t_map*)ttype;
     out << indent() << tname << ptr << " " << name << " = "
         << generate_new_hash_from_type(tmap->get_key_type(), tmap->get_val_type()) << endl;
+  } else if (ttype->is_list()) {
+    t_list* tlist = (t_list*)ttype;
+    out << indent() << tname << ptr << " " << name << " = "
+        << generate_new_array_from_type(tlist->get_elem_type()) << endl;
   } else if (ttype->is_enum()) {
     out << indent() << tname << ptr << " " << name << ";" << endl;
   } else {
diff --git a/lib/c_glib/test/ContainerTest.thrift b/lib/c_glib/test/ContainerTest.thrift
index c19bae45f..a92a9a51c 100644
--- a/lib/c_glib/test/ContainerTest.thrift
+++ b/lib/c_glib/test/ContainerTest.thrift
@@ -19,6 +19,9 @@
 
 namespace c_glib TTest
 
+typedef list<string> StringList
+typedef list<StringList> ListStringList
+
 struct ContainersWithDefaultValues {
   1: list<string> StringList = [ "Apache", "Thrift" ];
 }
@@ -26,4 +29,7 @@ struct ContainersWithDefaultValues {
 service ContainerService {
   void receiveStringList(1: list<string> stringList);
   list<string> returnStringList();
+
+  list<list<string>> returnListStringList();
+  ListStringList returnTypedefdListStringList();
 }
diff --git a/lib/c_glib/test/testcontainertest.c b/lib/c_glib/test/testcontainertest.c
index dc61666ba..852254b4b 100644
--- a/lib/c_glib/test/testcontainertest.c
+++ b/lib/c_glib/test/testcontainertest.c
@@ -136,6 +136,46 @@ test_container_service_handler_return_string_list (TTestContainerServiceIf *ifac
   return TRUE;
 }
 
+static gboolean
+test_container_service_handler_return_list_string_list (TTestContainerServiceIf *iface,
+                                                        GPtrArray **_return,
+                                                        GError **error)
+{
+  TestContainerServiceHandler *self = TEST_CONTAINER_SERVICE_HANDLER (iface);
+  GPtrArray *nested_list;
+
+  /* Return a list containing our list of strings */
+  nested_list
+    = g_ptr_array_new_with_free_func ((GDestroyNotify)g_ptr_array_unref);
+  g_ptr_array_add (nested_list, self->string_list);
+  g_ptr_array_ref (self->string_list);
+
+  g_ptr_array_add (*_return, nested_list);
+
+  g_clear_error (error);
+  return TRUE;
+}
+
+static gboolean
+test_container_service_handler_return_typedefd_list_string_list (TTestContainerServiceIf *iface,
+                                                                 TTestListStringList **_return,
+                                                                 GError **error)
+{
+  TestContainerServiceHandler *self = TEST_CONTAINER_SERVICE_HANDLER (iface);
+  TTestStringList *nested_list;
+
+  /* Return a list containing our list of strings */
+  nested_list
+    = g_ptr_array_new_with_free_func ((GDestroyNotify)g_ptr_array_unref);
+  g_ptr_array_add (nested_list, self->string_list);
+  g_ptr_array_ref (self->string_list);
+
+  g_ptr_array_add (*_return, nested_list);
+
+  g_clear_error (error);
+  return TRUE;
+}
+
 static void
 test_container_service_handler_finalize (GObject *object) {
   TestContainerServiceHandler *self = TEST_CONTAINER_SERVICE_HANDLER (object);
@@ -168,6 +208,10 @@ test_container_service_handler_class_init (TestContainerServiceHandlerClass *kla
     test_container_service_handler_receive_string_list;
   parent_class->return_string_list =
     test_container_service_handler_return_string_list;
+  parent_class->return_list_string_list =
+    test_container_service_handler_return_list_string_list;
+  parent_class->return_typedefd_list_string_list =
+    test_container_service_handler_return_typedefd_list_string_list;
 }
 
 /* -------------------------------------------------------------------------- */
@@ -189,6 +233,53 @@ sigchld_handler (int signal_number)
     thrift_server_stop (server);
 }
 
+/* A helper function that executes a test case against a newly constructed
+   service client */
+static void
+execute_with_service_client (void (*test_case)(TTestContainerServiceIf *,
+                                               GError **))
+{
+  ThriftSocket *socket;
+  ThriftTransport *transport;
+  ThriftProtocol *protocol;
+
+  TTestContainerServiceIf *client;
+
+  GError *error = NULL;
+
+  /* Create a client with which to access the server */
+  socket    = g_object_new (THRIFT_TYPE_SOCKET,
+                            "hostname", TEST_SERVER_HOSTNAME,
+                            "port",     TEST_SERVER_PORT,
+                            NULL);
+  transport = g_object_new (THRIFT_TYPE_BUFFERED_TRANSPORT,
+                            "transport", socket,
+                            NULL);
+  protocol  = g_object_new (THRIFT_TYPE_BINARY_PROTOCOL,
+                            "transport", transport,
+                            NULL);
+
+  thrift_transport_open (transport, &error);
+  g_assert_no_error (error);
+
+  client = g_object_new (T_TEST_TYPE_CONTAINER_SERVICE_CLIENT,
+                         "input_protocol",  protocol,
+                         "output_protocol", protocol,
+                         NULL);
+
+  /* Execute the test against this client */
+  (*test_case)(client, &error);
+  g_assert_no_error (error);
+
+  /* Clean up and exit */
+  thrift_transport_close (transport, NULL);
+
+  g_object_unref (client);
+  g_object_unref (protocol);
+  g_object_unref (transport);
+  g_object_unref (socket);
+}
+
 static void
 test_containers_with_default_values (void)
 {
@@ -212,61 +303,36 @@ test_containers_with_default_values (void)
 }
 
 static void
-test_container_service_string_list (void)
+test_container_service_string_list_inner (TTestContainerServiceIf *client,
+                                          GError **error)
 {
-  ThriftSocket *socket;
-  ThriftTransport *transport;
-  ThriftProtocol *protocol;
-
-  TTestContainerServiceIf *client;
+  gchar *test_data[] = { "one", "two", "three" };
 
   GPtrArray *outgoing_string_list;
   GPtrArray *incoming_string_list;
+  guint index;
 
-  gchar *test_data[] = { "one", "two", "three" };
-  guint8 index;
-
-  GError *error = NULL;
+  g_clear_error (error);
 
   /* Prepare our test data (our string list to send) */
   outgoing_string_list = g_ptr_array_new ();
   for (index = 0; index < 3; index += 1)
     g_ptr_array_add (outgoing_string_list, &test_data[index]);
 
-  /* Create a client with which to access the server */
-  socket    = g_object_new (THRIFT_TYPE_SOCKET,
-                            "hostname", TEST_SERVER_HOSTNAME,
-                            "port",     TEST_SERVER_PORT,
-                            NULL);
-  transport = g_object_new (THRIFT_TYPE_BUFFERED_TRANSPORT,
-                            "transport", socket,
-                            NULL);
-  protocol  = g_object_new (THRIFT_TYPE_BINARY_PROTOCOL,
-                            "transport", transport,
-                            NULL);
-
-  thrift_transport_open (transport, &error);
-  g_assert_no_error (error);
-
-  client = g_object_new (T_TEST_TYPE_CONTAINER_SERVICE_CLIENT,
-                         "input_protocol",  protocol,
-                         "output_protocol", protocol,
-                         NULL);
-
   /* Send our data to the server and make sure we get the same data back on
      retrieve */
   g_assert
     (t_test_container_service_client_receive_string_list (client,
                                                           outgoing_string_list,
-                                                          &error) &&
-     error == NULL);
+                                                          error) &&
+     *error == NULL);
 
   incoming_string_list = g_ptr_array_new ();
   g_assert
     (t_test_container_service_client_return_string_list (client,
                                                          &incoming_string_list,
-                                                         &error) &&
-     error == NULL);
+                                                         error) &&
+     *error == NULL);
 
   /* Make sure the two lists are equivalent */
   g_assert_cmpint (incoming_string_list->len, ==, outgoing_string_list->len);
@@ -276,15 +342,85 @@ test_container_service_string_list (void)
                      ((gchar **)outgoing_string_list->pdata)[index]);
 
   /* Clean up and exit */
-  thrift_transport_close (transport, NULL);
-
   g_ptr_array_unref (incoming_string_list);
   g_ptr_array_unref (outgoing_string_list);
+}
 
-  g_object_unref (client);
-  g_object_unref (protocol);
-  g_object_unref (transport);
-  g_object_unref (socket);
+static void
+test_container_service_string_list (void)
+{
+    execute_with_service_client (test_container_service_string_list_inner);
+}
+
+static void
+test_container_service_list_string_list_inner (TTestContainerServiceIf *client,
+                                               GError **error)
+{
+  GPtrArray *incoming_list;
+  GPtrArray *nested_list;
+
+  g_clear_error (error);
+
+  /* Receive a list of string lists from the server */
+  incoming_list =
+    g_ptr_array_new_with_free_func ((GDestroyNotify)g_ptr_array_unref);
+  g_assert
+    (t_test_container_service_client_return_list_string_list (client,
+                                                              &incoming_list,
+                                                              error) &&
+     *error == NULL);
+
+  /* Make sure the list and its contents are valid */
+  g_assert_cmpint (incoming_list->len, >, 0);
+
+  nested_list = (GPtrArray *)g_ptr_array_index (incoming_list, 0);
+  g_assert (nested_list != NULL);
+  g_assert_cmpint (nested_list->len, >=, 0);
+
+  /* Clean up and exit */
+  g_ptr_array_unref (incoming_list);
+}
+
+static void
+test_container_service_list_string_list (void)
+{
+  execute_with_service_client (test_container_service_list_string_list_inner);
+}
+
+static void
+test_container_service_typedefd_list_string_list_inner (TTestContainerServiceIf *client,
+                                                        GError **error)
+{
+  TTestListStringList *incoming_list;
+  TTestStringList *nested_list;
+
+  g_clear_error (error);
+
+  /* Receive a list of string lists from the server */
+  incoming_list =
+    g_ptr_array_new_with_free_func ((GDestroyNotify)g_ptr_array_unref);
+  g_assert
+    (t_test_container_service_client_return_list_string_list (client,
+                                                              &incoming_list,
+                                                              error) &&
+     *error == NULL);
+
+  /* Make sure the list and its contents are valid */
+  g_assert_cmpint (incoming_list->len, >, 0);
+
+  nested_list = (TTestStringList *)g_ptr_array_index (incoming_list, 0);
+  g_assert (nested_list != NULL);
+  g_assert_cmpint (nested_list->len, >=, 0);
+
+  /* Clean up and exit */
+  g_ptr_array_unref (incoming_list);
+}
+
+static void
+test_container_service_typedefd_list_string_list (void)
+{
+  execute_with_service_client
+    (test_container_service_typedefd_list_string_list_inner);
 }
 
 int
@@ -313,6 +449,12 @@ main(int argc, char *argv[])
     g_test_add_func
       ("/testcontainertest/ContainerTest/Services/ContainerService/StringList",
        test_container_service_string_list);
+    g_test_add_func
+      ("/testcontainertest/ContainerTest/Services/ContainerService/ListStringList",
+       test_container_service_list_string_list);
+    g_test_add_func
+      ("/testcontainertest/ContainerTest/Services/ContainerService/TypedefdListStringList",
+       test_container_service_typedefd_list_string_list);
 
     /* Run the tests and make the result available to our parent process */
     _exit (g_test_run ());