Re: 2.4 schedule

[Tim Janik <timj gtk org>]

On Thu, 25 Sep 2003, Owen Taylor wrote:

> On Wed, 2003-09-24 at 19:18, Tim Janik wrote:
>
> > >  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?
>
> It's a bug - it should have been in g_object_class_find_property(), but
> there's it's necessary.
>
> g_param_spec_pool_lookup() has to return the GParamSpecOverride,
> not the redirect target, other otherwise we won't have the
> correct owner type and property ID to actually call the
> implementation.

ah, but then all combinations of object/interface list/find need to
do the lookup as an override pspec shouldn't be user visible.

> > 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.
>
> Well, for get_nick/blurb/default() that's mostly true. But for
> set_default() / validate() / values_cmp(), etc, is that the case?
> As above g_param_spec_pool_lookup() can't do redirection, so
> inside GObject we'll mostly be dealing with the GParamSpecOverride.

yeah, and if we use it for anything else than figuring the property ID
and owner type, we're doing something wrong, so it's best to have its
methods check/catch that.

> What about construct() and GObject::notify? What GParamSpecs get
> passed there?

those are user visible, so they shouldn't be GParamSpecOverride either.

> It's certainly going to be easier and faster if
> we pass in the GParamSpecOverride. g_object_constructor() would
> have to look up every property again by name.

it would be faster yes, but it's quite possible the user actually
messes around with the GObjectConstructParam array, i.e. it's
pspecs or values (that's why it's exposed to him). so it's part
of the user visible API and needs to be redirected already.

> > 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
>
> I certainly didn't imagine anyone calling
> g_param_spec_set_redirect_target() to *unset* the paramspec
> for an already created GParamSpecOverride. GParamSpecOverride
> takes a 'GParamSpec *overriden' property that may not be NULL.

> > - disallow GParamSpecOverride uses alltogether (leaving its constructor
> >   as an implementation detail of object/iface property systems).
>
> I don't think 'private to object property systems' is a coherent
> API policy.

we have that policy already. fundamental types and iface-check functions
fall under that very same category.

> Either we:
>
>  - Don't export
>  - Export but with #define guards becuase we have reduced stability
>    guarantees.
>  - Export
>
> I'm happy to remove the "generally" from the current documentation
>
>  "... and generally will not be directly useful unless you are
>   implementing a new base type similar to GObject"

yes, that's as far as the above "disallow" was meant to go.
i.e. just conceptual (not #defined) deprecation for things other than
proeprty system implementations.

> >   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).
>
> Why can't we just require the param spec to be provided at creation
> time?

basically, to make adding the interface with the implementing
property work after class_init. we just added the necessary gtype
logic to check that case.

> > 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.
>
> That's conceptually really messy, since it leaves the GParamSpecOverride
> without a value type until it first gets looked up.

that's just an implementation detail though.

> I don't think you really answered my question:
>
>  Should we *remove* the public g_param_spec_set_redirect_target()
>  and make GParamOverride the only way to create a redirected
>  GParamSpec.
>
> That would allow considerable simplification of the code and some
> increase in efficiency. (No need to use qdata for the redirect
> target)

i listed two paths we could persue, and expressed bias towards
the second, which was getting rid of a public set_redirect_target
and implementing automated pool-internal set_redirect_target.

i now think we should definitely implement the second option,
eventhough it has a conceptually messy implementation detail ;)

> Regards,
> 						Owen
>

---
ciaoTJ