gobject: Reimplement g_param_values_cmp() for GParamSpecVariant

The existing implementation was completely incorrect (despite the fix in
commit 566e64a66) — it always compared GVariants by pointer, rather than
by value.

Reimplement it to compare them by value where possible, depending on
their type. The core of this implementation is g_variant_compare(). See
the documentation and tests for further details of the new sort order.

This adds documentation and tests.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

https://bugzilla.gnome.org/show_bug.cgi?id=795735

gobject/gparamspecs.c

@@ -1147,6 +1147,20 @@ param_variant_validate (GParamSpec *pspec,
   return FALSE;
 }
 
+/* g_variant_compare() can only be used with scalar types. */
+static gboolean
+variant_is_incomparable (GVariant *v)
+{
+  GVariantClass v_class = g_variant_classify (v);
+
+  return (v_class == G_VARIANT_CLASS_HANDLE ||
+          v_class == G_VARIANT_CLASS_VARIANT ||
+          v_class ==  G_VARIANT_CLASS_MAYBE||
+          v_class == G_VARIANT_CLASS_ARRAY ||
+          v_class ==  G_VARIANT_CLASS_TUPLE ||
+          v_class == G_VARIANT_CLASS_DICT_ENTRY);
+}
+
 static gint
 param_variant_values_cmp (GParamSpec   *pspec,
                           const GValue *value1,
@@ -1155,7 +1169,19 @@ param_variant_values_cmp (GParamSpec   *pspec,
   GVariant *v1 = value1->data[0].v_pointer;
   GVariant *v2 = value2->data[0].v_pointer;
 
-  return v1 < v2 ? -1 : v2 > v1;
+  if (v1 == NULL && v2 == NULL)
+    return 0;
+  else if (v1 == NULL && v2 != NULL)
+    return -1;
+  else if (v1 != NULL && v2 == NULL)
+    return 1;
+
+  if (!g_variant_type_equal (g_variant_get_type (v1), g_variant_get_type (v2)) ||
+      variant_is_incomparable (v1) ||
+      variant_is_incomparable (v2))
+    return g_variant_equal (v1, v2) ? 0 : (v1 < v2 ? -1 : 1);
+
+  return g_variant_compare (v1, v2);
 }
 
 /* --- type initialization --- */

gobject/gparamspecs.h

@@ -962,6 +962,12 @@ struct _GParamSpecGType
  *
  * A #GParamSpec derived structure that contains the meta data for #GVariant properties.
  *
+ * When comparing values with g_param_values_cmp(), scalar values with the same
+ * type will be compared with g_variant_compare(). Other non-%NULL variants will
+ * be checked for equality with g_variant_equal(), and their sort order is
+ * otherwise undefined. %NULL is ordered before non-%NULL variants. Two %NULL
+ * values compare equal.
+ *
  * Since: 2.26
  */
 struct _GParamSpecVariant

tests/gobject/paramspec-test.c

@@ -228,6 +228,10 @@ test_param_spec_variant (void)
 {
   GParamSpec *pspec;
   GValue value = G_VALUE_INIT;
+  GValue value2 = G_VALUE_INIT;
+  GValue value3 = G_VALUE_INIT;
+  GValue value4 = G_VALUE_INIT;
+  GValue value5 = G_VALUE_INIT;
   gboolean modified;
 
   pspec = g_param_spec_variant ("variant", "nick", "blurb",
@@ -238,21 +242,114 @@ test_param_spec_variant (void)
   g_value_init (&value, G_TYPE_VARIANT);
   g_value_set_variant (&value, g_variant_new_int32 (42));
 
-  g_assert (g_param_value_defaults (pspec, &value));
+  g_value_init (&value2, G_TYPE_VARIANT);
+  g_value_set_variant (&value2, g_variant_new_int32 (43));
+
+  g_value_init (&value3, G_TYPE_VARIANT);
+  g_value_set_variant (&value3, g_variant_new_int16 (42));
+
+  g_value_init (&value4, G_TYPE_VARIANT);
+  g_value_set_variant (&value4, g_variant_new_parsed ("[@u 15, @u 10]"));
+
+  g_value_init (&value5, G_TYPE_VARIANT);
+  g_value_set_variant (&value5, NULL);
+
+  g_assert_true (g_param_value_defaults (pspec, &value));
+  g_assert_false (g_param_value_defaults (pspec, &value2));
+  g_assert_false (g_param_value_defaults (pspec, &value3));
+  g_assert_false (g_param_value_defaults (pspec, &value4));
+  g_assert_false (g_param_value_defaults (pspec, &value5));
 
   modified = g_param_value_validate (pspec, &value);
-  g_assert (!modified);
+  g_assert_false (modified);
 
   g_value_reset (&value);
   g_value_set_variant (&value, g_variant_new_uint32 (41));
   modified = g_param_value_validate (pspec, &value);
-  g_assert (modified);
+  g_assert_true (modified);
   g_assert_cmpint (g_variant_get_int32 (g_value_get_variant (&value)), ==, 42);
   g_value_unset (&value);
 
+  g_value_unset (&value5);
+  g_value_unset (&value4);
+  g_value_unset (&value3);
+  g_value_unset (&value2);
+
   g_param_spec_unref (pspec);
 }
 
+/* Test g_param_values_cmp() for #GParamSpecVariant. */
+static void
+test_param_spec_variant_cmp (void)
+{
+  const struct
+    {
+      const GVariantType *pspec_type;
+      const gchar *v1;
+      enum
+        {
+          LESS_THAN = -1,
+          EQUAL = 0,
+          GREATER_THAN = 1,
+          NOT_EQUAL,
+        } expected_result;
+      const gchar *v2;
+    }
+  vectors[] =
+    {
+      { G_VARIANT_TYPE ("i"), "@i 1", LESS_THAN, "@i 2" },
+      { G_VARIANT_TYPE ("i"), "@i 2", EQUAL, "@i 2" },
+      { G_VARIANT_TYPE ("i"), "@i 3", GREATER_THAN, "@i 2" },
+      { G_VARIANT_TYPE ("i"), NULL, LESS_THAN, "@i 2" },
+      { G_VARIANT_TYPE ("i"), NULL, EQUAL, NULL },
+      { G_VARIANT_TYPE ("i"), "@i 1", GREATER_THAN, NULL },
+      { G_VARIANT_TYPE ("i"), "@u 1", LESS_THAN, "@u 2" },
+      { G_VARIANT_TYPE ("i"), "@as ['hi']", NOT_EQUAL, "@u 2" },
+      { G_VARIANT_TYPE ("i"), "@as ['hi']", NOT_EQUAL, "@as ['there']" },
+      { G_VARIANT_TYPE ("i"), "@as ['hi']", EQUAL, "@as ['hi']" },
+    };
+  gsize i;
+
+  for (i = 0; i < G_N_ELEMENTS (vectors); i++)
+    {
+      GParamSpec *pspec;
+      GValue v1 = G_VALUE_INIT;
+      GValue v2 = G_VALUE_INIT;
+      gint cmp;
+
+      pspec = g_param_spec_variant ("variant", "nick", "blurb",
+                                    vectors[i].pspec_type,
+                                    NULL,
+                                    G_PARAM_READWRITE);
+
+      g_value_init (&v1, G_TYPE_VARIANT);
+      g_value_set_variant (&v1, (vectors[i].v1 != NULL) ? g_variant_new_parsed (vectors[i].v1) : NULL);
+
+      g_value_init (&v2, G_TYPE_VARIANT);
+      g_value_set_variant (&v2, (vectors[i].v2 != NULL) ? g_variant_new_parsed (vectors[i].v2) : NULL);
+
+      cmp = g_param_values_cmp (pspec, &v1, &v2);
+
+      switch (vectors[i].expected_result)
+        {
+        case LESS_THAN:
+        case EQUAL:
+        case GREATER_THAN:
+          g_assert_cmpint (cmp, ==, vectors[i].expected_result);
+          break;
+        case NOT_EQUAL:
+          g_assert_cmpint (cmp, !=, 0);
+          break;
+        default:
+          g_assert_not_reached ();
+        }
+
+      g_value_unset (&v2);
+      g_value_unset (&v1);
+      g_param_spec_unref (pspec);
+    }
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -263,6 +360,7 @@ main (int argc, char *argv[])
   g_test_add_func ("/paramspec/override", test_param_spec_override);
   g_test_add_func ("/paramspec/gtype", test_param_spec_gtype);
   g_test_add_func ("/paramspec/variant", test_param_spec_variant);
+  g_test_add_func ("/paramspec/variant/cmp", test_param_spec_variant_cmp);
 
   return g_test_run ();
 }