New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve deriving of trait impls #627

Merged
merged 5 commits into from Aug 17, 2018

Conversation

Projects
None yet
2 participants
@sdroege
Member

sdroege commented Aug 2, 2018

See gtk-rs/glib#367 for context

This is WIP

Add support for generating Hash impls
And also unhide Hash/Equal functions for objects as those are generally
implemented already and the specific ones might provide additional
functionality
RefMode::ByRef => if explicit_target_type.is_empty() {
("".into(), ".to_glib_none().0")
} else {
(format!("ToGlibPtr::<{}>::to_glib_none(", explicit_target_type), ").0")

This comment has been minimized.

@EPashkin

EPashkin Aug 3, 2018

Member

Thanks for finding oneliner.
I always forget about this form of call

@EPashkin

EPashkin Aug 3, 2018

Member

Thanks for finding oneliner.
I always forget about this form of call

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 3, 2018

Member

Looks very good.
My only worry, that I don't understand why impl hash::Hash and other don't generated for traited Gio.File, Gio.Icon

Member

EPashkin commented Aug 3, 2018

Looks very good.
My only worry, that I don't understand why impl hash::Hash and other don't generated for traited Gio.File, Gio.Icon

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Aug 3, 2018

Member

My only worry, that I don't understand why impl hash::Hash and other don't generated for traited Gio.File, Gio.Icon

See gtk-rs/glib#367 (comment) . It's intentional and expected

Member

sdroege commented Aug 3, 2018

My only worry, that I don't understand why impl hash::Hash and other don't generated for traited Gio.File, Gio.Icon

See gtk-rs/glib#367 (comment) . It's intentional and expected

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 3, 2018

Member

@sdroege Thanks for reminder

Member

EPashkin commented Aug 3, 2018

@sdroege Thanks for reminder

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Aug 3, 2018

Member

@sdroege Thanks for reminder

Basically trait based, non-dynamic-dispatch Eq etc mechanism are conceptually incompatible with subclassing

Member

sdroege commented Aug 3, 2018

@sdroege Thanks for reminder

Basically trait based, non-dynamic-dispatch Eq etc mechanism are conceptually incompatible with subclassing

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Aug 16, 2018

Member

Ok fixed all the comments, now just have to bring it to the final decision about which traits to implement where. Will do so tomorrow

Member

sdroege commented Aug 16, 2018

Ok fixed all the comments, now just have to bring it to the final decision about which traits to implement where. Will do so tomorrow

sdroege added some commits Aug 17, 2018

Unhide ToString/Compare special functions for objects and don't impl …
…the traits

These are generically implemented and specific impls don't make much
sense due to subtyping causing incoherent results otherwise.
Derive various traits for Boxed/Shared types unless overridden by config
And don't derive any of the trait impls if they would be provided by
actual functions.
@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Aug 17, 2018

Member

This should be all good now, and should be merged together with gtk-rs/glib#367 and then regens everywhere

Member

sdroege commented Aug 17, 2018

This should be all good now, and should be merged together with gtk-rs/glib#367 and then regens everywhere

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 17, 2018

Member

@sdroege Thanks, code looks good.
I'm a little worried about let instance_parameter = idx == 0; but IMHO this can be fixed if caused problem.

After this merged gtk-rs/glib#367 need update.

Member

EPashkin commented Aug 17, 2018

@sdroege Thanks, code looks good.
I'm a little worried about let instance_parameter = idx == 0; but IMHO this can be fixed if caused problem.

After this merged gtk-rs/glib#367 need update.

@EPashkin EPashkin merged commit 04c26a8 into gtk-rs:master Aug 17, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment