Skip to content

Conversation

pbor
Copy link
Contributor

@pbor pbor commented Jun 13, 2024

No description provided.

bilelmoussaoui
bilelmoussaoui previously approved these changes Jun 13, 2024
@sdroege
Copy link
Member

sdroege commented Jun 14, 2024

I don't think this is correct, at least not for the shared case. from_raw() and into_raw() are not just returning &Arc<T> as *const T or so but do some offsetting.

Can you add tests for both cases somewhere (using a pointer slice or something else that requires the traits) to ensure this is actually working correctly? Also if these traits are implemented you can implement FromValue for references like glib::wrapper! does.

@pbor
Copy link
Contributor Author

pbor commented Jun 14, 2024

Thanks for catching that SharedBoxed was a bad idea... indeed I did it by cut&paste without thinking :(

Can you add tests for both cases somewhere (using a pointer slice or something else that requires the traits) to ensure this is actually working correctly?

I added a small test for the boxed case, but please check if you think it is enough to exercise this.

you can implement FromValue for references like glib::wrapper! does

As far as I can see that is already there: https://github.com/gtk-rs/gtk-rs-core/blob/master/glib-macros/src/boxed_derive.rs#L56

@pbor pbor changed the title Derive TransparentPtrType trait for Boxed and SharedBoxed Derive TransparentPtrType trait for Boxed Jun 14, 2024
@sdroege sdroege merged commit 73e8cd7 into gtk-rs:master Jun 17, 2024
@sdroege sdroege added the needs-backport PR needs backporting to the current stable branch label Jun 17, 2024
@pbor pbor added backported PR was backported to the current stable branch and removed needs-backport PR needs backporting to the current stable branch labels Jun 18, 2024
@sdroege
Copy link
Member

sdroege commented Oct 29, 2024

See #1559. This is going to be changed to TransparentType as TransparentPtrType was simply wrong.

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

Labels

backported PR was backported to the current stable branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants