gtk-rs / gir

Tool to generate rust bindings and user API for glib-based libraries
https://gtk-rs.org/gir/book/
MIT License
230 stars 102 forks source link

codegen: only generate trait signatures once #1461

Closed jf2048 closed 1 year ago

jf2048 commented 1 year ago

Is there some reason this is not done already? It seems to reduce code size in gtk4-rs by about 5-6%.

sdroege commented 1 year ago

I don't know why it is done this way. It was like that when I first looked at gtk-rs :) @GuillaumeGomez ?

GuillaumeGomez commented 1 year ago

I don't remember at all. It looks like a failed rebase. If it doesn't change anything, then it's a great improvement!

sdroege commented 1 year ago

What do you mean with failed rebase?

Also when regenerating, this will require changes to all the manual traits too.

GuillaumeGomez commented 1 year ago

Then I clearly misunderstood what it was doing. Oh well.

sdroege commented 1 year ago

It removes the ext trait function implementations and moves them into the trait definition as default implementations for the functions. So the ext trait implementation is now only a single line, and the signatures for the functions have to provided only a single time.

The main difference here is that if it's possible to also implement the trait on other types then they would automatically get the default implementations of the functions now instead of having to provide some. The default implementations would likely be wrong for any other implementation. But can there be other implementations of those traits or would all possible implementations overlap with the blanket implementation?

GuillaumeGomez commented 1 year ago

So the show method for WidgetExt default impl would be working only for Widget and not for any other types implementing WidgetExt is what you meant?

sdroege commented 1 year ago

It would work for T: IsA<Widget> but not for i32. But can someone implement the trait for i32?

jf2048 commented 1 year ago

I think it makes sense to say we don't want anybody implementing WidgetExt etc for random types, maybe even seal those traits