Re: .defs Issues From Java-Gnome

[Owen Taylor <otaylor redhat com>]

Tim Janik <timj gtk org> writes:

> > > > 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.

OK,

In conventional terms, what you are talking about is the "direction",
or "parameter passing mode" for arguments. In, for instance, CORBA,
you have three directions:

 in
 out
 inout

And, yes, we have all of those directions in G* currently.  And, yes,
it would be nice to solve this problem, though I'm rather dubious we
can do it in the 2.0 timescale. But, as I'll argue below, I think the
result of fixing it for boxed types, would end up looking like my
solution - don't copy boxed types when passing them to signals.


If you really want to pursue this issue, I think you'd find looking
the C language mapping for CORBA useful:

 ftp://ftp.omg.org/pub/docs/formal/99-07-35.{ps,pdf}

[ Just the C language mapping, PDF is ~420k, 58 pages, pages
  20-35 are relevant, in particular section 1-19 on p. 31  ]

I'm not suggesting we should copy these rules, but they should
give you some idea of the complexity that could be involved
here.


But, the fundemental difference between CORBA and our situation
is that CORBA actually has full knowledge of every type
other than Object *, unlike our highly generic type system
where we've tried to virtualize everything. 

As an example of what actually knowing what types are allows,
according to CORBA rules, a fixed length struct (say GdkRectangle)
passed out is mapped similar to:

 GdkRectangle rect;

 foo_get_rect (foo, &rect);
 [ use rect ]

While a variable length struct (say, GdkEvent) is mapped 
similar to:

 GdkEvent *event;

 foo_get_event (foo, &event);
 [ use event ]
 gdk_event_free (event);

But can we do this? No - if both GdkRectangle and GdkEvent stay as
GDK_TYPE_BOXED, then we cannot treat the GdkRectangle argument to
::size_request() as an OUT parameter, it is simply an IN parameter
that points to memory that is modified in the call - similar to the
way that GObject parameters are IN parameters.

> > 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.

We use 'static' for strings, because the most common case involves:

 const static char *my_key = "foo";
 
 quark = g_quark_from_static_string (&foo);

And, especially for the quark case, because the string will be
kept around forever by GLib, so it needs to be static.

For string signal parameters, the use of 'static' becomes distinctly
odd. For boxed types, it's just bizarre.
 
> 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.

I don't think there is ever a reason to ever copy boxed values when
passing them into signals. It's slow, and it isn't necessary,
since the caller is responsible for maintaining in parameters
for the duration of the call.

Yes, its possible to come up with examples where the boxed type
passed in is freed during the call - but I just don't think
that we can or should try to protect against all possible
problems at that level.

> 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.

Let stop a second and say something about the concept here.
If you want to pack a direction flag into the same guint32 as
the type, that's OK with me:

 G_DIRECTION_IN | G_TYPE_INT

If I thought a "static" flag, was OK, then you'd have:

 G_TYPE_FLAG_STATIC | G_TYPE_INT

But please don't talk about this as creating a new type. The type
is the same. Keep this in mind:

 Creating reference types and passing them in is not the
 best way to map out parameters in most languages:

For example, in Python, you should return a tuple. A reference type
in Python would have to be mapped as an array of length
1 or something, so compare:

     tmp = [None]
     int_return = get_rect (foo, tmp)
     rect_return = tmp[0]

    With:

     (int_return, rect_rect) = get_rect (foo)

Most languages don't have reference types - introducing them
into the GObject type system will only produce ugliness.

> 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 ;)

Well, that's a first shot at comments - certainly I haven't covered
every possible related issue, but my general opinion is:

 - Adding argument directions isn't crazy, but, it, by itself
   doesn't help us here.

 - A static flag should not be necessary for signal arguments,
   because copying is almost always harmful.

 - Don't think ofany of this as creating new types, think of it as
   adding extra flags to signal arguments indicating the 
   parameter passing conventions for the type of the parameter.

Regards,
                                        Owen