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

Remove callback guard #667

Merged
merged 3 commits into from Jun 23, 2018

Conversation

Projects
None yet
3 participants
@EPashkin
Member

EPashkin commented Jun 23, 2018

@@ -10,8 +10,7 @@ while (/^(\N*)\V*fn\s+(\w+)\s*(<[^(]+>)?\s*(\([^{;]+)\{\N*\n^(\N*)$/gms) {
$first_line =~ /^\s*(
assert_initialized_main_thread |
assert_not_initialized |
skip_assert_initialized |
callback_guard
skip_assert_initialized

This comment has been minimized.

@EPashkin

EPashkin Jun 23, 2018

Member

unsafe functions just not checked, so this line not worked anyway

@EPashkin

EPashkin Jun 23, 2018

Member

unsafe functions just not checked, so this line not worked anyway

let _guard = ::glib::CallbackGuard::new();
if cfg!(debug_assertions) {
assert_initialized_main_thread!();
}

This comment has been minimized.

@EPashkin

EPashkin Jun 23, 2018

Member

I slightly worry about this check, maybe it really needed

@EPashkin

EPashkin Jun 23, 2018

Member

I slightly worry about this check, maybe it really needed

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jun 23, 2018

Member

Yes but it should not be in the callback guard macro. I thought there was another macro for that?

Member

sdroege commented Jun 23, 2018

Yes but it should not be in the callback guard macro. I thought there was another macro for that?

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jun 23, 2018

Member

This macro named assert_initialized_main_thread ;)
I just not sure that we need check that all callback called from main thread in gtk.

Member

EPashkin commented Jun 23, 2018

This macro named assert_initialized_main_thread ;)
I just not sure that we need check that all callback called from main thread in gtk.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jun 23, 2018

Member

They are and it makes no sense to check that in the callback. It would be a gtk bug and not ours, not something we should try to detect

Member

sdroege commented Jun 23, 2018

They are and it makes no sense to check that in the callback. It would be a gtk bug and not ours, not something we should try to detect

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jun 23, 2018

Member

Ok, then I think that PR is ready

Member

EPashkin commented Jun 23, 2018

Ok, then I think that PR is ready

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jun 23, 2018

Member

I just hope gtk doesn't have another bug. :-/

Thanks @EPashkin and @sdroege!

Member

GuillaumeGomez commented Jun 23, 2018

I just hope gtk doesn't have another bug. :-/

Thanks @EPashkin and @sdroege!

@GuillaumeGomez GuillaumeGomez merged commit e599669 into gtk-rs:master Jun 23, 2018

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 EPashkin deleted the EPashkin:remove_callback_guard branch Jun 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment