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 complete pointer type for trampoline return #769

Merged
merged 1 commit into from May 16, 2019

Conversation

Projects
None yet
3 participants
@sfanxiang
Copy link
Contributor

commented May 16, 2019

gpointer mismatches with callback return type generated here.

Example: GtkMapListModelMapFunc in gtk4.

cc @GuillaumeGomez @sdroege

@EPashkin

This comment has been minimized.

Copy link
Member

commented May 16, 2019

@sfanxiang Can you provide code diff after this PR?

No changes in gtk-rs or https://github.com/sdroege/gstreamer-rs

@sfanxiang

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

@EPashkin There you go, in auto-generated gtk4:

--- a/src/auto/map_list_model.rs
+++ b/src/auto/map_list_model.rs
@@ -30,7 +30,7 @@ impl MapListModel {
     pub fn new<P: IsA<gio::ListModel>>(item_type: glib::types::Type, model: Option<&P>, map_func: Option<Box<dyn Fn(&glib::Object) -> glib::Object + 'static>>) -> MapListModel {
         assert_initialized_main_thread!();
         let map_func_data: Box_<Option<Box<dyn Fn(&glib::Object) -> glib::Object + 'static>>> = Box::new(map_func);
-        unsafe extern "C" fn map_func_func<P: IsA<gio::ListModel>>(item: *mut gobject_sys::GObject, user_data: glib_sys::gpointer) -> glib_sys::gpointer {
+        unsafe extern "C" fn map_func_func<P: IsA<gio::ListModel>>(item: *mut gobject_sys::GObject, user_data: glib_sys::gpointer) -> *mut gobject_sys::GObject {
             let item = from_glib_full(item);
             let callback: &Option<Box<dyn Fn(&glib::Object) -> glib::Object + 'static>> = &*(user_data as *mut _);
             let res = if let Some(ref callback) = *callback {
@@ -85,7 +85,7 @@ impl<O: IsA<MapListModel>> MapListModelExt for O {
 
     fn set_map_func(&self, map_func: Option<Box<dyn Fn(&glib::Object) -> glib::Object + 'static>>) {
         let map_func_data: Box_<Option<Box<dyn Fn(&glib::Object) -> glib::Object + 'static>>> = Box::new(map_func);
-        unsafe extern "C" fn map_func_func(item: *mut gobject_sys::GObject, user_data: glib_sys::gpointer) -> glib_sys::gpointer {
+        unsafe extern "C" fn map_func_func(item: *mut gobject_sys::GObject, user_data: glib_sys::gpointer) -> *mut gobject_sys::GObject {
             let item = from_glib_full(item);
             let callback: &Option<Box<dyn Fn(&glib::Object) -> glib::Object + 'static>> = &*(user_data as *mut _);
             let res = if let Some(ref callback) = *callback {

Not really much atm, but I think we should avoid the type error either here or in -sys unless there's a reason not to.

@sdroege

This comment has been minimized.

Copy link
Member

commented May 16, 2019

Where does MapListModel come from exactly? I don't have it in GTK 4 3.91.2 here. The resulting change makes sense though if I guess correctly how that API works

@sfanxiang

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

@sdroege

Where does MapListModel come from exactly?

https://gitlab.gnome.org/GNOME/gtk/commits/master/gtk/gtkmaplistmodel.c, added in Sep 2018.

@sdroege

This comment has been minimized.

Copy link
Member

commented May 16, 2019

Curious why it's not appearing in my docs :) Apparently GTK has grown yet another balanced binary tree implementation.

Looks all good to me, thanks!

@EPashkin

This comment has been minimized.

Copy link
Member

commented May 16, 2019

@sfanxiang Thanks

@EPashkin EPashkin merged commit 579d4ed into gtk-rs:master May 16, 2019

2 checks passed

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

@sfanxiang sfanxiang deleted the sfanxiang:cb-ret-full-ptr branch May 26, 2019

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.