Re: [Owen Taylor <otaylor redhat com>] Re: .defs Issues From Java-Gnome (fwd)

[Tim Janik <timj gtk org>]

On 4 Jan 2001, Owen Taylor wrote:

> 
> Tim,
> 
> This is very urgent to get fixed. Do you think you'll be able to do it
> in the next day or two? If not, do you mind if someone else does it?

this is not a matter of me not having enough time for the task.

> Is the approach I suggest below OK?

unfortunately not, if it were that easy, we'd be having that already.


> Tim Janik <timj gtk org> writes:
> 
> > On 14 Dec 2000, Havoc Pennington wrote:
> > 
> > 
> > > For cheap value objects, including primitive C types such as "int" and
> > > higher-level types such as GtkTextIter and GdkRectangle, sometimes the
> > > object is an out or inout parameter; in those cases you have to pass
> > > the object in to a method by reference, so you can't just copy these
> > > things always.
> > 
> > basically you're right. except that, the signal system in its current
> > form (and the type system with all its readily-boxed-magic ;) don't
> > know about passing value types by reference, which gets us all that
> > demand-copying mess.
> > we had some proposals for using additional GtkType flags to indicate
> > value-type references, but basically they were put down because they
> > didn't scale too well. i'm still somewhat uncertain as to what the best
> > approach here is... ;(
> 
> The problem wasn't scaling but the fact that problem here can't
> be solved on a type-by-type basis, it needs to be solved on
> a use-by-use basis.

kinda right. what we do in C is sometimes pretty ugly to map,
e.g. we have G_TYPE_INT and
- put that as signal arg (..., int x, ...), that is pretty
  straight forward to marshal
- collect that generically, e.g. for g_object_set() and signal_emit()
  g_value_set_int (&v, va_arg (args, int));
- implicitely introduce pointer indirections for g_object_get():
  int my_int; g_object_get (o, "intval", &my_int, NULL);
  this, the value's lcopy variant knows about, i.e. it does:
  int *generic_int_p = va_arg (args, void*);
  *generic_int_p = g_value_get_int (&v);
this works for types that are passed by value in C and for types that
are passed by reference and are reference counted in C, such as GObject.

blatantly, that is it. if you have a something-value that you need to
pass in, you're fince with value types or reference counted reference types,
but if you need something that you pass in and need to get out again, only
reference counted reference types do suit our purposes. that's why a
signal argument such as (..., int *inout, ...) is still announced to the
type system as G_TYPE_POINTER, it can't sufficiently handle that.

so there's a broader problem behaind the issues that havoc and you are
experiencing, that is, the type signal system doesn't support all the types
that we actually use for signals, namely out values that arent't reference
counted reference types (to give another example, you can't create a
signal (... GObject **out_object, ...) to the sigal system either without
using G_TYPE_POINTER).
for a quick fix of the problem you are currently having, a special
"i'm static for this scope" flag to the signal types and the value
collectors is actually sufficient. basically it could work like:

 #define G_TYPE_STATIC_FLAG	(1 << 31)
 #define G_TYPE_STATIC(type)	((type) | G_TYPE_STATIC_FLAG)
-#define G_VALUE_COLLECT(value, var_args, __error)  { ...; \
-         g_value_set_boxed (&v, va_arg (args, void*)); ...}
+#define G_VALUE_COLLECT(value, var_args, bool_static, __error)  { ...; \
+         (bool_static ? g_value_set_static_boxed :
+          g_value_set_boxed) (&v, va_arg (args, void*)); ...}
then you do:
signal_new (..., 2, G_TYPE_COLOR, G_TYPE_STATIC (G_TYPE_COLOR), G_TYPE_NONE);

though G_TYPE_STATIC_FLAG wouldn't make sense for all types, we'd actually
need an extra member in GTypeValueTable to indicate whether this'd actually
make sense for this type.

however, that's still not adressing the more general problem of us not
correctly dealing with references to types, such as int* or GObject**.
that in itself wouldn't be too hard to fix either, basically we could
have a new fundamental type G_TYPE_REFERENCE, a helper macro to create/
objetain reference types and a couple of assorted functions:
#define	G_TYPE_MAKE_REFERENCE(type)	(g_type_reference_from_type (type))
GType g_type_from_reference_type (GType reference_type);
void g_value_move_reference_contents (const GValue *reference_type_value,
                                      GValue       *contents_value,
                                      gboolean      static_contents);
void g_value_set_reference (GValue  *reference_value,
                            gpointer location);
i.e.:
g_type_from_reference_type (G_TYPE_MAKE_REFERENCE (G_TYPE_FOO)) == G_TYPE_FOO
and G_TYPE_MAKE_REFERENCE (G_TYPE_FOO) simply returns a new/cached type id.
GValue rvalue, cvalue; int foo = 5;
g_value_init (&rvalue, G_TYPE_MAKE_REFERENCE (G_TYPE_INT));
g_value_init (&cvalue, G_TYPE_INT);
g_value_set_reference (&rvalue, &foo);
g_value_move_reference_contents (&rvalue, &cvalue,
                                 TRUE /* true/false here would actually matter
                                 for boxed types or strings */);
g_value_get_int (&cvalue) == 5

(yeah, we'd also need a variant for cvalue->*rvalue movements)

basically, that gives us the required support for all the things that we
actually use as pure C prototypes for signals, and it scales, e.g.
(..., int **foo, ...) is simply
G_TYPE_MAKE_REFERENCE (G_TYPE_MAKE_REFERENCE (G_TYPE_INT))

but to be techinically correct, boxed types, since they are by defintion
not reference counted but copiable types, would have to be handled differently
at least as far as collections/lcopies are concerned.
say "color" is of type GTK_TYPE_GDK_COLOR, and "ref_color" is of type
G_TYPE_MAKE_REFERENCE (GTK_TYPE_GDK_COLOR), and "requisition" is of type
GTK_TYPE_REQUISITION, then collection would work like:

g_object_set (o,
              "color", 0.1 /*red*/, 0.1/*green*/, 0.1/*blue*/,
              "ref_color", &widget->style->white,
              "requisition", width, height,
              NULL);

that'll also work for the non-NULL-terminated strings that havoc requests,
i.e. G_TYPE_LENGTH_STRING (or watchacallit) would be collected as an
(int length, char *contents) pair, however, a reference to it will be mapped
into a C type as: struct{int;char*;}*

while this approach with scalable reference types works up to the actuall
needs we have, it does involve significant changes to all code that currently
uses boxed types (the libgobject changes aren't that significant actually),
mainly s/GTK_TYPE_GDK_*/G_TYPE_MAKE_REFERENCE (GTK_TYPE_GDK_*)/ in the
signal creation functions and further changes to property handling functions.

> The only real solution here is:
> 
>  g_value_set_boxed_nocopy (&value, boxed_type);
> 
> Handling of freeing the value can be done in two ways:
> 
>  - Convention. If you called g_value_set_*_nocopy(), then you
>    should not call g_value_unset().
> 
>  - Keep a flag. You can do the same trick as you did for
>    g_value_set_static_string() by storing this flag in data[1].
> 
>    [ Repeat once again the comment about data[4] being useless ]
> 
> Since you have previously expressed strong opposition to the
> first option, it seems like the second option is the way
> we should handle this.
> 
> g_value_set_static_string() needs to be renamed to
> g_value_set_string_nocopy() to go along with this, since the concept
> of "not copying" is considerably more generic than "static".

apart from _nocopy being a pretty odd prefix, the concept of naming
functions that avoid an extra copy _stytic_ is already established
(e.g. g_quark_from_static_string) and does fit into the picture for
boxed types as well, i don't see why we'd want to call it _nocopy
for boxed types when _static_ was completely valid for strings.

but anyways, that's not at all the issue, what has to be solved is
basically special flagging of out values for signals, so the
value collection code knows what to copy and what not.
G_TYPE_STATIC() looks like a viable short-term solution, but it doesn't
really solve all of our problems and might get us a pretty odd compat
define #define G_TYPE_STATIC G_TYPE_MAKE_REFERENCE in 2.2 when we
decide to really go for reference types.
it also only cures one symptom we currently have and defers neccessary
API changes for reference types which we probably will want to have at
some point for proper LB mapping.

that being said, i'm still not too sure we should go for the reference
types right now, i'm not exactly sure why.
i'd really like to hear comments from you on the above, i guess that'll
help me making up my mind ;)

> 
> Regards,
>                                         Owen
> 


---
ciaoTJ