Allow registering instance private data during get_type()

For static types, it should be possible to register a private data
structure right when we are registering the type, i.e. from the
get_type() implementation. By allowing this, we can take advantage of
the existing type definition macros to cut down the amount of code
necessary (as well as the knowledge baggage) when creating a new type.

The main issue with this new feature is that it cannot be mixed with the
old idiomatic way of adding private instance data by calling a function
in the middle of the class_init() implementation, as that imposes the
additional constraint of initializing the whole type hierarchy in order
to retrieve the offset of the private data in the GTypeInstance
allocation.

For this reason we are going to follow a two-step process; in the first
step, we are going to introduce the new (semi-private) API to register
the intent to add private instance data from within the get_type()
implementation, and hide it behind a macro; at the same time, the
G_DEFINE_TYPE_EXTENDED macro is going to be modified so that it will
register the private instance data if the macro was used, using a new
(semi-private) function as well. Once we have migrated all our code, we
will make the first new function perform the actual private data
registration, and turn the second new function into a no-op. This should
guarantee a transparent migration of existing code to the new idiomatic
form.

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

5 files changed, 457 insertions, 0 deletions

diff --git a/gobject/gtype.c b/gobject/gtype.c
index 8d5df09d0..23f503d6e 100644
--- a/gobject/gtype.c
+++ b/gobject/gtype.c
@@ -4509,6 +4509,111 @@ g_type_class_add_private (gpointer g_class,
   G_WRITE_UNLOCK (&type_rw_lock);
 }
 
+/* semi-private, called only by the G_ADD_PRIVATE macro */
+gint
+g_type_add_instance_private (GType class_gtype,
+                             gsize private_size)
+{
+  TypeNode *node = lookup_type_node_I (class_gtype);
+
+  g_return_val_if_fail (private_size > 0, 0);
+  g_return_val_if_fail (private_size <= 0xffff, 0);
+
+  if (!node || !node->is_classed || !node->is_instantiatable || !node->data)
+    {
+      g_warning ("cannot add private field to invalid (non-instantiatable) type '%s'",
+		 type_descriptive_name_I (class_gtype));
+      return 0;
+    }
+
+  if (node->plugin != NULL)
+    {
+      g_warning ("cannot use g_type_add_instance_private() with dynamic type '%s'",
+                 type_descriptive_name_I (class_gtype));
+      return 0;
+    }
+
+  /* in the future, we want to register the private data size of a type
+   * directly from the get_type() implementation so that we can take full
+   * advantage of the type definition macros that we already have.
+   *
+   * unfortunately, this does not behave correctly if a class in the middle
+   * of the type hierarchy uses the "old style" of private data registration
+   * from the class_init() implementation, as the private data offset is not
+   * going to be known until the full class hierarchy is initialized.
+   *
+   * in order to transition our code to the Glorious New Future™, we proceed
+   * with a two-step implementation: first, we provide this new function to
+   * register the private data size in the get_type() implementation and we
+   * hide it behind a macro. the function will return the private size, instead
+   * of the offset, which will be stored inside a static variable defined by
+   * the G_DEFINE_TYPE_EXTENDED macro. the G_DEFINE_TYPE_EXTENDED macro will
+   * check the variable and call g_type_class_add_instance_private(), which
+   * will use the data size and actually register the private data, then
+   * return the computed offset of the private data, which will be stored
+   * inside the static variable, so we can use it to retrieve the pointer
+   * to the private data structure.
+   *
+   * once all our code has been migrated to the new idiomatic form of private
+   * data registration, we will change the g_type_add_instance_private()
+   * function to actually perform the registration and return the offset
+   * of the private data; g_type_class_add_instance_private() already checks
+   * if the passed argument is negative (meaning that it's an offset in the
+   * GTypeInstance allocation) and becomes a no-op if that's the case. this
+   * should make the migration fully transparent even if we're effectively
+   * copying this macro into everybody's code.
+   */
+  return private_size;
+}
+
+/* semi-private function, should only be used by G_DEFINE_TYPE_EXTENDED */
+void
+g_type_class_adjust_private_offset (gpointer  g_class,
+                                    gint     *private_size_or_offset)
+{
+  GType class_gtype = ((GTypeClass *) g_class)->g_type;
+  TypeNode *node = lookup_type_node_I (class_gtype);
+
+  g_return_if_fail (private_size_or_offset != NULL);
+
+  /* if we have been passed the offset instead of the private data size,
+   * then we consider this as a no-op, and just return the value. see the
+   * comment in g_type_add_instance_private() for the full explanation.
+   */
+  if (*private_size_or_offset > 0)
+    g_return_if_fail (*private_size_or_offset <= 0xffff);
+  else
+    return;
+
+  if (!node || !node->is_classed || !node->is_instantiatable || !node->data)
+    {
+      g_warning ("cannot add private field to invalid (non-instantiatable) type '%s'",
+		 type_descriptive_name_I (class_gtype));
+      *private_size_or_offset = 0;
+      return;
+    }
+
+  if (NODE_PARENT_TYPE (node))
+    {
+      TypeNode *pnode = lookup_type_node_I (NODE_PARENT_TYPE (node));
+      if (node->data->instance.private_size != pnode->data->instance.private_size)
+	{
+	  g_warning ("g_type_add_instance_private() called multiple times for the same type");
+          *private_size_or_offset = 0;
+	  return;
+	}
+    }
+
+  G_WRITE_LOCK (&type_rw_lock);
+
+  node->data->instance.private_size = ALIGN_STRUCT (node->data->instance.private_size + *private_size_or_offset);
+  g_assert (node->data->instance.private_size <= 0xffff);
+
+  *private_size_or_offset = -(gint) node->data->instance.private_size;
+
+  G_WRITE_UNLOCK (&type_rw_lock);
+}
+
 gpointer
 g_type_instance_get_private (GTypeInstance *instance,
 			     GType          private_type)
diff --git a/gobject/gtype.h b/gobject/gtype.h
index a0a57f03e..c27574011 100644
--- a/gobject/gtype.h
+++ b/gobject/gtype.h
@@ -1284,9 +1284,15 @@ GType*g_type_interface_prerequisites    (GType                       interface_t
 GLIB_AVAILABLE_IN_ALL
 void     g_type_class_add_private       (gpointer                    g_class,
                                          gsize                       private_size);
+GLIB_AVAILABLE_IN_2_38
+gint     g_type_add_instance_private    (GType                       class_type,
+                                         gsize                       private_size);
 GLIB_AVAILABLE_IN_ALL
 gpointer g_type_instance_get_private    (GTypeInstance              *instance,
                                          GType                       private_type);
+GLIB_AVAILABLE_IN_2_38
+void     g_type_class_adjust_private_offset (gpointer                g_class,
+                                             gint                   *private_size_or_offset);
 
 GLIB_AVAILABLE_IN_ALL
 void      g_type_add_class_private      (GType    		     class_type,
@@ -1335,6 +1341,22 @@ guint     g_type_get_type_registration_serial (void);
  */
 #define G_DEFINE_TYPE_WITH_CODE(TN, t_n, T_P, _C_)	    _G_DEFINE_TYPE_EXTENDED_BEGIN (TN, t_n, T_P, 0) {_C_;} _G_DEFINE_TYPE_EXTENDED_END()
 /**
+ * G_DEFINE_TYPE_WITH_PRIVATE:
+ * @TN: The name of the new type, in Camel case.
+ * @t_n: The name of the new type, in lowercase, with words 
+ *  separated by '_'.
+ * @T_P: The #GType of the parent type.
+ * 
+ * A convenience macro for type implementations, which declares a 
+ * class initialization function, an instance initialization function (see #GTypeInfo for information about 
+ * these), a static variable named @t_n<!-- -->_parent_class pointing to the parent class, and adds private
+ * instance data to the type. Furthermore, it defines a *_get_type() function. See G_DEFINE_TYPE_EXTENDED()
+ * for an example.
+ * 
+ * Since: 2.38
+ */
+#define G_DEFINE_TYPE_WITH_PRIVATE(TN, t_n, T_P)            G_DEFINE_TYPE_EXTENDED (TN, t_n, T_P, 0, G_ADD_PRIVATE (TN))
+/**
  * G_DEFINE_ABSTRACT_TYPE:
  * @TN: The name of the new type, in Camel case.
  * @t_n: The name of the new type, in lowercase, with words 
@@ -1365,6 +1387,19 @@ guint     g_type_get_type_registration_serial (void);
  */
 #define G_DEFINE_ABSTRACT_TYPE_WITH_CODE(TN, t_n, T_P, _C_) _G_DEFINE_TYPE_EXTENDED_BEGIN (TN, t_n, T_P, G_TYPE_FLAG_ABSTRACT) {_C_;} _G_DEFINE_TYPE_EXTENDED_END()
 /**
+ * G_DEFINE_ABSTRACT_TYPE_WITH_PRIVATE:
+ * @TN: The name of the new type, in Camel case.
+ * @t_n: The name of the new type, in lowercase, with words 
+ *  separated by '_'.
+ *
+ * @T_P: The #GType of the parent type.
+ * Similar to G_DEFINE_TYPE_WITH_PRIVATE(), but defines an abstract type. 
+ * See G_DEFINE_TYPE_EXTENDED() for an example.
+ * 
+ * Since: 2.4
+ */
+#define G_DEFINE_ABSTRACT_TYPE_WITH_PRIVATE(TN, t_n, T_P)   G_DEFINE_TYPE_EXTENDED (TN, t_n, T_P, G_TYPE_FLAG_ABSTRACT, G_ADD_PRIVATE (TN))
+/**
  * G_DEFINE_TYPE_EXTENDED:
  * @TN: The name of the new type, in Camel case.
  * @t_n: The name of the new type, in lowercase, with words
@@ -1487,17 +1522,86 @@ guint     g_type_get_type_registration_serial (void);
   g_type_add_interface_static (g_define_type_id, TYPE_IFACE, &g_implement_interface_info); \
 }
 
+/**
+ * G_ADD_PRIVATE:
+ * @TypeName: the name of the type in CamelCase
+ *
+ * A convenience macro to ease adding private data to instances of a new type
+ * in the @_C_ section of G_DEFINE_TYPE_WITH_CODE() or
+ * G_DEFINE_ABSTRACT_TYPE_WITH_CODE().
+ *
+ * For instance:
+ *
+ * |[
+ *   typedef struct _MyObject MyObject;
+ *   typedef struct _MyObjectClass MyObjectClass;
+ *
+ *   typedef struct {
+ *     gint foo;
+ *     gint bar;
+ *   } MyObjectPrivate;
+ *
+ *   G_DEFINE_TYPE_WITH_CODE (MyObject, my_object, G_TYPE_OBJECT,
+ *                            G_ADD_PRIVATE (MyObject))
+ * ]|
+ *
+ * Will add MyObjectPrivate as the private data to any instance of the MyObject
+ * type.
+ *
+ * G_DEFINE_TYPE_* macros will automatically create a private function
+ * based on the arguments to this macro, which can be used to safely
+ * retrieve the private data from an instance of the type; for instance:
+ *
+ * |[
+ *   gint
+ *   my_object_get_foo (MyObject *obj)
+ *   {
+ *     MyObjectPrivate *priv = my_object_get_private (obj);
+ *
+ *     return priv->foo;
+ *   }
+ *
+ *   void
+ *   my_object_set_bar (MyObject *obj,
+ *                      gint      bar)
+ *   {
+ *     MyObjectPrivate *priv = my_object_get_private (obj);
+ *
+ *     if (priv->bar != bar)
+ *       priv->bar = bar;
+ *   }
+ * ]|
+ *
+ * Note that this macro can only be used together with the G_DEFINE_TYPE_*
+ * macros, since it depends on variable names from those macros.
+ *
+ * Since: 2.38
+ */
+#define G_ADD_PRIVATE(TypeName) { \
+  TypeName##_private_offset = \
+    g_type_add_instance_private (g_define_type_id, sizeof (TypeName##Private)); \
+}
+
 #define _G_DEFINE_TYPE_EXTENDED_BEGIN(TypeName, type_name, TYPE_PARENT, flags) \
 \
 static void     type_name##_init              (TypeName        *self); \
 static void     type_name##_class_init        (TypeName##Class *klass); \
 static gpointer type_name##_parent_class = NULL; \
+static gint     TypeName##_private_offset; \
 static void     type_name##_class_intern_init (gpointer klass) \
 { \
   type_name##_parent_class = g_type_class_peek_parent (klass); \
+  if (TypeName##_private_offset != 0) \
+    g_type_class_adjust_private_offset (klass, &TypeName##_private_offset); \
   type_name##_class_init ((TypeName##Class*) klass); \
 } \
 \
+static inline gpointer \
+type_name##_get_private (TypeName *self) \
+{ \
+  return (G_STRUCT_MEMBER_P (self, TypeName##_private_offset)); \
+} \
+\
 GType \
 type_name##_get_type (void) \
 { \
diff --git a/gobject/tests/.gitignore b/gobject/tests/.gitignore
index 30fcd4116..274099393 100644
--- a/gobject/tests/.gitignore
+++ b/gobject/tests/.gitignore
@@ -10,4 +10,5 @@ reference
 signals
 threadtests
 valuearray
+private
 marshalers.[ch]
diff --git a/gobject/tests/Makefile.am b/gobject/tests/Makefile.am
index 044ccb350..1b8965442 100644
--- a/gobject/tests/Makefile.am
+++ b/gobject/tests/Makefile.am
@@ -19,6 +19,7 @@ test_programs = \
 	reference			\
 	valuearray			\
 	type				\
+	private				\
 	$(NULL)
 
 # -----------------------------------------------------------------------------
diff --git a/gobject/tests/private.c b/gobject/tests/private.c
new file mode 100644
index 000000000..6a95200ff
--- /dev/null
+++ b/gobject/tests/private.c
@@ -0,0 +1,246 @@
+#include <glib-object.h>
+
+typedef struct {
+  GObject parent_instance;
+} TestObject;
+
+typedef struct {
+  int dummy_0;
+  float dummy_1;
+} TestObjectPrivate;
+
+typedef struct {
+  GObjectClass parent_class;
+} TestObjectClass;
+
+GType test_object_get_type (void);
+
+G_DEFINE_TYPE_WITH_CODE (TestObject, test_object, G_TYPE_OBJECT,
+                         G_ADD_PRIVATE (TestObject))
+
+static void
+test_object_class_init (TestObjectClass *klass)
+{
+}
+
+static void
+test_object_init (TestObject *self)
+{
+  TestObjectPrivate *priv = test_object_get_private (self);
+
+  if (g_test_verbose ())
+    g_print ("Offset of %sPrivate for type '%s': %d\n",
+             G_OBJECT_TYPE_NAME (self),
+             G_OBJECT_TYPE_NAME (self),
+             TestObject_private_offset);
+
+  priv->dummy_0 = 42;
+  priv->dummy_1 = 3.14159f;
+}
+
+static int
+test_object_get_dummy_0 (TestObject *self)
+{
+  TestObjectPrivate *priv = test_object_get_private (self);
+
+  return priv->dummy_0;
+}
+
+static float
+test_object_get_dummy_1 (TestObject *self)
+{
+  TestObjectPrivate *priv = test_object_get_private (self);
+
+  return priv->dummy_1;
+}
+
+typedef struct {
+  TestObject parent_instance;
+} TestDerived;
+
+typedef struct {
+  char *dummy_2;
+} TestDerivedPrivate;
+
+typedef struct {
+  TestObjectClass parent_class;
+} TestDerivedClass;
+
+GType test_derived_get_type (void);
+
+G_DEFINE_TYPE_WITH_CODE (TestDerived, test_derived, test_object_get_type (),
+                         G_ADD_PRIVATE (TestDerived))
+
+static void
+test_derived_finalize (GObject *obj)
+{
+  TestDerivedPrivate *priv = test_derived_get_private ((TestDerived *) obj);
+
+  g_free (priv->dummy_2);
+
+  G_OBJECT_CLASS (test_derived_parent_class)->finalize (obj);
+}
+
+static void
+test_derived_class_init (TestDerivedClass *klass)
+{
+  G_OBJECT_CLASS (klass)->finalize = test_derived_finalize;
+}
+
+static void
+test_derived_init (TestDerived *self)
+{
+  TestDerivedPrivate *priv = test_derived_get_private (self);
+
+  if (g_test_verbose ())
+    g_print ("Offset of %sPrivate for type '%s': %d\n",
+             G_OBJECT_TYPE_NAME (self),
+             G_OBJECT_TYPE_NAME (self),
+             TestDerived_private_offset);
+
+  priv->dummy_2 = g_strdup ("Hello");
+}
+
+static const char *
+test_derived_get_dummy_2 (TestDerived *self)
+{
+  TestDerivedPrivate *priv = test_derived_get_private (self);
+
+  return priv->dummy_2;
+}
+
+typedef struct {
+  TestObject parent_instance;
+} TestMixed;
+
+typedef struct {
+  gint dummy_3;
+} TestMixedPrivate;
+
+typedef struct {
+  TestObjectClass parent_class;
+} TestMixedClass;
+
+GType test_mixed_get_type (void);
+
+G_DEFINE_TYPE (TestMixed, test_mixed, test_object_get_type ())
+
+static void
+test_mixed_class_init (TestMixedClass *klass)
+{
+  g_type_class_add_private (klass, sizeof (TestMixedPrivate));
+}
+
+static void
+test_mixed_init (TestMixed *self)
+{
+  TestMixedPrivate *priv = G_TYPE_INSTANCE_GET_PRIVATE (self, test_mixed_get_type (), TestMixedPrivate);
+
+  if (g_test_verbose ())
+    g_print ("Offset of %sPrivate for type '%s': %d\n",
+             G_OBJECT_TYPE_NAME (self),
+             G_OBJECT_TYPE_NAME (self),
+             TestMixed_private_offset);
+
+  priv->dummy_3 = 47;
+}
+
+static gint
+test_mixed_get_dummy_3 (TestMixed *self)
+{
+  TestMixedPrivate *priv = G_TYPE_INSTANCE_GET_PRIVATE (self, test_mixed_get_type (), TestMixedPrivate);
+
+  return priv->dummy_3;
+}
+
+typedef struct {
+  TestMixed parent_instance;
+} TestMixedDerived;
+
+typedef struct {
+  gint64 dummy_4;
+} TestMixedDerivedPrivate;
+
+typedef struct {
+  TestMixedClass parent_class;
+} TestMixedDerivedClass;
+
+GType test_mixed_derived_get_type (void);
+
+G_DEFINE_TYPE_WITH_CODE (TestMixedDerived, test_mixed_derived, test_mixed_get_type (),
+                         G_ADD_PRIVATE (TestMixedDerived))
+
+static void
+test_mixed_derived_class_init (TestMixedDerivedClass *klass)
+{
+}
+
+static void
+test_mixed_derived_init (TestMixedDerived *self)
+{
+  TestMixedDerivedPrivate *priv = test_mixed_derived_get_private (self);
+
+  if (g_test_verbose ())
+    g_print ("Offset of %sPrivate for type '%s': %d\n",
+             G_OBJECT_TYPE_NAME (self),
+             G_OBJECT_TYPE_NAME (self),
+             TestMixedDerived_private_offset);
+
+  priv->dummy_4 = g_get_monotonic_time ();
+}
+
+static gint64
+test_mixed_derived_get_dummy_4 (TestMixedDerived *self)
+{
+  TestMixedDerivedPrivate *priv = test_mixed_derived_get_private (self);
+
+  return priv->dummy_4;
+}
+
+static void
+private_instance (void)
+{
+  TestObject *obj = g_object_new (test_object_get_type (), NULL);
+
+  g_assert_cmpint (test_object_get_dummy_0 (obj), ==, 42);
+  g_assert_cmpfloat (test_object_get_dummy_1 (obj), ==, 3.14159f);
+
+  g_object_unref (obj);
+}
+
+static void
+private_derived_instance (void)
+{
+  TestDerived *obj = g_object_new (test_derived_get_type (), NULL);
+
+  g_assert_cmpstr (test_derived_get_dummy_2 (obj), ==, "Hello");
+  g_assert_cmpint (test_object_get_dummy_0 ((TestObject *) obj), ==, 42);
+
+  g_object_unref (obj);
+}
+
+static void
+private_mixed_derived_instance (void)
+{
+  TestMixedDerived *derived = g_object_new (test_mixed_derived_get_type (), NULL);
+  TestMixed *mixed = g_object_new (test_mixed_get_type (), NULL);
+
+  g_assert_cmpint (test_mixed_get_dummy_3 (mixed), ==, 47);
+  g_assert (test_mixed_derived_get_dummy_4 (derived) <= g_get_monotonic_time ());
+
+  g_object_unref (derived);
+  g_object_unref (mixed);
+}
+
+int
+main (int argc,
+      char *argv[])
+{
+  g_test_init (&argc, &argv, NULL);
+
+  g_test_add_func ("/private/instance", private_instance);
+  g_test_add_func ("/private/derived-instance", private_derived_instance);
+  g_test_add_func ("/private/mixed-derived-instance", private_mixed_derived_instance);
+
+  return g_test_run ();
+}