Skip to content
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

Use rust 1.79 implied supertraits to simplify trait bounds #1529

Merged
merged 2 commits into from
Oct 9, 2024

Conversation

felinira
Copy link
Contributor

@felinira felinira commented Oct 5, 2024

This removes a lot of the boilerplate introduced by #1519 by using supertraits instead of where clauses. Supertraits are allowed to imply their trait bounds since rust 1.79.

This bumps MSRV to 1.80.

IMO it's easier to review this against d8dfa3a than master, as it reverts most of what that's doing.

@sdroege
Copy link
Member

sdroege commented Oct 5, 2024

IMO it's easier to review this against d8dfa3a than master, as it reverts most of what that's doing.

felinira/gtk-rs-core@d8dfa3a...wip/implied-supertraits

where
<Self as ObjectSubclass>::Type: IsA<glib::Object>,
<Self as ObjectSubclass>::Type: IsA<super::MyPluginInterface>,
pub trait MyPluginInterfaceImpl:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should do the same for all the other extension traits I guess. You look into this afterwards?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't an extension trait though? Not sure what you mean here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added the comment at a random place. I mean for example pub trait FileExt: IsA<File> + sealed::Sealed + 'static could lose the sealing AFAICS, what we talked about on matrix 2 weeks or so ago

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but those are auto traits. I'm not sure how involved it is to change gir here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That kind of change should be easy. But I can also take a look at it one of these days :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there is something that needs to be done on gir side, just let me know what is the change you want to have. I am a bit familiar with the codebase already, no need for extra-suffering ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bilelmoussaoui Instead of having the sealing it is sufficient for the extension traits to require IsA<TheType>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And when regenerating, the sealing also should be dropped from the manual extension traits like that. I hope that slightly improves compile times too :)

@sdroege sdroege merged commit f69594c into gtk-rs:master Oct 9, 2024
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants