gtk-rs-core icon indicating copy to clipboard operation
gtk-rs-core copied to clipboard

Discussion on automatically chaining up vfuncs

Open A6GibKm opened this issue 3 years ago • 4 comments

The vfuncs constructed and dispose work a little different, the bindings will automatically chain up dispose at the end while the user has to manually chain up constructed.

It would be a bug if neither the bindings or the user chained up dispose, specifically at the end of the vfunc, automatically chaining it makes sense, the problem is that it leaves us in an awkward situation where half of the vfuncs are automatically chained for you and the other require manual chaining. In most cases no visible problem will appear if the user forgets to do it, they will go on their way for a long time until they find some weird and subtle behavior and have a very hard time debugging it.

It is also problematic that the documentation for this behavior is not very discoverable.

Some possible actions include:

  • Never automatically chain up vfuncs
  • Automatically chain up whenever it makes sense. constructed is a good example, I struggle to think a reason why the user specifically needs to chain it at the end of the vfunc, but in gtk4-rs there are many examples where it would be problematic, e.g. gtk4::Window::close_request since the user might want to do something with the bool returned
  • Document it better, maybe add a debug statement if you forget to manually chain up
  • Do nothing, it can be argued that it is clear from the docs which vfuncs have (or don't have) a parent_vfuncname

Since I have sent more than one MR chaining multiple instances of constructed it is not far fetched to think this will lead to subtle app bugs in the future and that not being aware of this somewhat common.

A6GibKm avatar Aug 11 '21 17:08 A6GibKm

where half of the vfuncs are automatically chained for you and the other require manual chaining

The only ones that should be automatic are ObjectImpl::{dispose,set_property,get_property}, the latter happening already in C directly. Having these as exceptions doesn't seem like much of a problem to me.

dispose is chaining up automatically because doing it at the end of your own dispose is the only valid place to do that, and not doing that causes problems including memory safety issues.

Document it better

I think this would be the solution here but I don't know where/how to add it to the docs.

maybe add a debug statement if you forget to manually chain up

It's most of the time not a bug to not chain up. It completely depends on whether you want to extend or replace the base class behaviour.

Not chaining up constructed seems like a bug though so maybe we should try to detect that case indeed. However chaining up before or after your own code seems both valid to me.

sdroege avatar Aug 12 '21 09:08 sdroege

What should we do here then? As it is, this issue is not really actionable :)

sdroege avatar Aug 31 '21 07:08 sdroege

I think the sanest would be to do nothing on gtk-rs itself, and to document this better, probably in the examples, subclass docs and gtk-rust-template.

A6GibKm avatar Aug 31 '21 16:08 A6GibKm

I don't think that makes sense, each vfunc should be handled separately and there is no way to know whether you are supposed to chain up or not without having to read the C code. Hopefully https://gitlab.gnome.org/GNOME/gobject-introspection/-/issues/411 will help for that

bilelmoussaoui avatar Feb 11 '22 09:02 bilelmoussaoui

Closing as such

sdroege avatar Oct 10 '22 12:10 sdroege