Re: 2.4 schedule

[Tim Janik <timj gtk org>]

On Tue, 23 Sep 2003, Owen Taylor wrote:

> Aside from commit approval, I'm waiting on several other things from
> Tim:

hm, meant to discuss remainings with you on irc, but
you seem to be absent, so:

>  A) An answer to my question from September 10:
>
>   What do you think, then, of making g_param_spec_set_redirect_target()
>   not public? What about making g_param_spec_get_redirect_target() just
>   expect a GParamSpecOverride rather than using qdata?

looking at your patch, g_object_class_list_properties(),
g_object_class_find_property() and g_object_interface_list_properties()
don't use get_redirect target, while g_object_interface_find_property()
does. is this a left over from moving the redirection lookup into
the pool?
provided we can make all lookups private, we probably don't
need public API for redirections.

i find the pspec-override implementation a bit suspicious though.
if we resolve redirections only inside the pool (and return NULL
in case of no redirect target), methods like:
param_override_set_default, param_override_validate,
param_override_values_cmp, and the get_nick, get_blurb variants
should never be called for a redirect pspec, unless you create
a g_param_spec_override() purposely, don't add it to a class and
just use it.
that's quite pointless without using at least set_redirect_target(),
so we should do either of:
- provide public set/get redirect functions, in which case your
  param_override_*() methods need to handle a NULL target more gracefully
- disallow GParamSpecOverride uses alltogether (leaving its constructor
  as an implementation detail of object/iface property systems).
  here, the param_override_*() methods need to error out more clearly,
  since their invocation can only happen if something went wrong earlier.

i'd opt for the second, but for that your implementation needs to be
tweaked to also allow automated setting of the redirect target *after*
calling g_param_spec_override ("name", NULL). that's basically attempting
a second lookup from within the pool if you encounter an override pspec
with redirect target of NULL, and in case of success doing
set_redirect_target() on that pspec.

there's a left over ptototype g_object_property_get_redirect_target()
in your patch.

>  B) A decision on making g_type_class_ref(iface_type) return the
>     "default vtable".

i don't like the idea of g_type_class_ref() covering additional types
for which G_TYPE_IS_CLASSED()==FALSE. since interfaces can't trivially
be turned into classed types (in the sense that G_TYPE_IS_CLASSED() would
return TRUE for them) which would also be an invalid API break btw,
the accessors here need to be seperated:

gpointer              g_type_default_interface_ref   (GType            type);
gpointer              g_type_default_interface_peek  (GType            type);
void                  g_type_default_interface_unref (gpointer         g_iface);


>  C) A decision on whether the 'postcheck' function we were discussing
>     should be called:
>
>     - immediately after the iface_init function gets called for any
>       interface on any type deriving from the specified type.
>
>     - After class_init and every iface_init subsequent to that.

recently, you said you'd rather stick to solving only the problem at hand,
which is an extra hook after iface_init, and on second thought i agree with
you here. "the function" here needs to be a hook list though, similar to
g_type_add_class_cache_func(), we get:

typedef gboolean (*GTypeInterfaceCheckFunc)  (gpointer         func_data,
                                              GtypeClass      *g_class,
                                              GTypeInterface  *g_iface);
void             g_type_add_interface_check  (gpointer                cache_data,
                                              GTypeInterfaceCheckFunc cache_func);
void             g_type_remove_interface_check (gpointer                cache_data,
                                                GTypeInterfaceCheckFunc cache_func);
/* simplest variant, the hooks could also be added _per_ (fundamental)
 * class type, but i don't expect much use of this, so this is most
 * likely good enough
 */

and the hooks are to be called whenever type_iface_vtable_iface_init_Wm()
is called with entry->init_state!=INITIALIZED. to properly guard the code
here (from gtype.c):

      entry->init_state = INITIALIZED;

      type_iface_vtable_iface_init_Wm (lookup_type_node_I (entry->iface_type), node);
[...]
 if (class_state >= IFACE_INIT)
    {
      type_iface_vtable_iface_init_Wm (iface, node);
      new_state = INITIALIZED;
    }

  if (class_state != UNINITIALIZED && class_state != INITIALIZED)
    {
      /* The interface was added while we were initializing the class
       */
      IFaceEntry *entry = type_lookup_iface_entry_L (node, iface);
      g_assert (entry);

      entry->init_state = new_state;


the entry->init_state = INITIALIZED; assignment needs to be moved
*inside* type_lookup_iface_entry_L(), and needs to occour before
iholder->info->interface_init() is being called.

> Given decisions on those answers, I think I could make up a good
> approximation to a final patch.

great, i still have to learn for an exam, so i tried to make comments
in a way that you can turn most of this into code pretty straight.

>
> Regards,
> 						Owen
>

---
ciaoTJ