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

Create fewer boxes for closures #710

Merged
merged 2 commits into from Jan 28, 2019

Conversation

Projects
None yet
3 participants
@sdroege
Copy link
Member

sdroege commented Jan 28, 2019

See the individual commits for explanations.

Needs some manual code updated too, which I'll do after the regens are all through.

Don't needlessly double-box closures for GIO async functions
The trampoline takes the concrete type of the closure as type parameter
already and we don't pass around trait objects but boxes of the concrete
type. Double-boxing is unnecessary in this case.
@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Jan 28, 2019

👍

Don't box signal handler closures twice
By making the trampolines generic in the concrete type of the closure.

@sdroege sdroege force-pushed the sdroege:fewer-boxes branch from 782f263 to 752c826 Jan 28, 2019

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 28, 2019

Let's get this in together with the callback generation fixes. It's small enough and we can regenerate all things at once.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Jan 28, 2019

@EPashkin Can you look at this one please?

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 28, 2019

@sdroege Thanks, I merge it.
Agree, that result better included in closures regen.
Note: don't forget to fix signal::destroy_closure

@EPashkin EPashkin merged commit b5a197a into gtk-rs:master Jan 28, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 28, 2019

listbox_model example now fails on close edit dialog

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 28, 2019

Note: don't forget to fix signal::destroy_closure

Ah that sucks. We need to make that one generic too. Let me fix all that, sorry

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Jan 28, 2019

I wait. But I hope you can feel the pressure now. 😛

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 28, 2019

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 28, 2019

@sdroege Thanks, now listbox_model works fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.