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

Add WeakRef support to glib::Object #204

Merged
merged 1 commit into from Jul 26, 2017

Conversation

Projects
None yet
3 participants
@sdroege
Member

sdroege commented Jul 23, 2017

Another thing for which I had custom bindings in my GStreamer code.

}
}
pub struct WeakRef<T: IsA<Object> + ?Sized>(Box<gobject_ffi::GWeakRef>, PhantomData<*const T>);

This comment has been minimized.

@sdroege

sdroege Jul 23, 2017

Member

Has to be a Box because GWeakRef remembers the address of this, and otherwise it would remember an "unstable" address (it is changing when moving the WeakRef around) and use it later (when the real object is destroyed).

@sdroege

sdroege Jul 23, 2017

Member

Has to be a Box because GWeakRef remembers the address of this, and otherwise it would remember an "unstable" address (it is changing when moving the WeakRef around) and use it later (when the real object is destroyed).

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 23, 2017

Member

Naming of the functions comes from Arc

Member

sdroege commented Jul 23, 2017

Naming of the functions comes from Arc

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 25, 2017

Member

@GuillaumeGomez Do you have any suggestions for the function names, or does it sound good to you like this?

Member

sdroege commented Jul 25, 2017

@GuillaumeGomez Do you have any suggestions for the function names, or does it sound good to you like this?

Show outdated Hide outdated src/object.rs
}
unsafe impl<T: IsA<Object> + Send + Sync + ?Sized> Send for WeakRef<T> {}
unsafe impl<T: IsA<Object> + Send + Sync + ?Sized> Sync for WeakRef<T> {}

This comment has been minimized.

@EPashkin

EPashkin Jul 25, 2017

Member

Maybe +Send => Send, +Sync => Sync

@EPashkin

EPashkin Jul 25, 2017

Member

Maybe +Send => Send, +Sync => Sync

This comment has been minimized.

@EPashkin

EPashkin Jul 25, 2017

Member

Also T: IsA<Object> can be unsized?

@EPashkin

EPashkin Jul 25, 2017

Member

Also T: IsA<Object> can be unsized?

This comment has been minimized.

@sdroege

sdroege Jul 25, 2017

Member

You need both to have each when reference counting is involved. We ensure that for Object anyway in gir, but better play safe here too.

For unsized, as a trait object maybe? Otherwise not really I guess. But doesn't hurt. I can remove that if you prefer that.

@sdroege

sdroege Jul 25, 2017

Member

You need both to have each when reference counting is involved. We ensure that for Object anyway in gir, but better play safe here too.

For unsized, as a trait object maybe? Otherwise not really I guess. But doesn't hurt. I can remove that if you prefer that.

This comment has been minimized.

@EPashkin

EPashkin Jul 25, 2017

Member

Ok. It was just note about unsized.

@EPashkin

EPashkin Jul 25, 2017

Member

Ok. It was just note about unsized.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jul 25, 2017

Member

I'll take a look in ~1 hour.

Member

GuillaumeGomez commented Jul 25, 2017

I'll take a look in ~1 hour.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jul 25, 2017

Member

What's wrong with upgrade and downgrade?

Member

GuillaumeGomez commented Jul 25, 2017

What's wrong with upgrade and downgrade?

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 25, 2017

Member

I thought they might be confusing with upcast and downcast. If you're fine with the names, great :) will fix it up later then. Thanks for looking

Member

sdroege commented Jul 25, 2017

I thought they might be confusing with upcast and downcast. If you're fine with the names, great :) will fix it up later then. Thanks for looking

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 25, 2017

Member

Should be good to go now

Member

sdroege commented Jul 25, 2017

Should be good to go now

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jul 25, 2017

Member

I'll look at it tomorrow. Please ping me if I forget. ;)

Member

GuillaumeGomez commented Jul 25, 2017

I'll look at it tomorrow. Please ping me if I forget. ;)

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege
Member

sdroege commented Jul 26, 2017

@GuillaumeGomez ping :)

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jul 26, 2017

Member

And as promised it got out of my mind. Since I have nothing to say on this, I merge. Thanks!

Member

GuillaumeGomez commented Jul 26, 2017

And as promised it got out of my mind. Since I have nothing to say on this, I merge. Thanks!

@GuillaumeGomez GuillaumeGomez merged commit dfddcd5 into gtk-rs:master Jul 26, 2017

2 checks passed

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

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 26, 2017

Member

Thanks!

Member

sdroege commented Jul 26, 2017

Thanks!

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