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

Remove ListBoxExtManual and autogenerate all remaining functions from it #770

Merged
merged 3 commits into from Jan 31, 2019

Conversation

Projects
None yet
3 participants
@sdroege
Copy link
Member

sdroege commented Jan 29, 2019

And also regenerate everything with the latest gir instead of some other version.

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 29, 2019

There are also many other functions that are now autogenerated and can be removed.

@GuillaumeGomez is going to check all the crates, and also if they have the correct gir version.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Jan 29, 2019

They have the correct gir version. Checking for the duplicates.

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 29, 2019

There's a bug in gir here

error[E0605]: non-primitive cast: `unsafe extern "C" fn(*mut libc::c_void, *mut libc::c_void) -> *mut ffi::GtkWidget {<O as auto::flow_box::FlowBoxExt>::bind_model::create_widget_func_func::<'a, P, Q, R>}` as `unsafe extern "C" fn(*mut gobject_ffi::GObject, *mut libc::c_void) -> *mut ffi::GtkWidget`
   --> src/auto/flow_box.rs:228:39
    |
228 |         let create_widget_func = Some(create_widget_func_func::<'a, P, Q, R> as _);
    |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: an `as` expression can only be used to convert between primitive types. Consider using the `From` trait

error[E0057]: this function takes 1 parameter but 0 parameters were supplied
   --> src/auto/flow_box.rs:225:23
    |
225 |             let res = (*callback)();
    |                       ^^^^^^^^^^^^^ expected 1 parameter

error[E0599]: no method named `to_glib` found for type `auto::widget::Widget` in the current scope
   --> src/auto/flow_box.rs:226:17
    |
226 |               res.to_glib()
    |                   ^^^^^^^
    | 
   ::: src/auto/widget.rs:55:1
    |
55  | / glib_wrapper! {
56  | |     pub struct Widget(Object<ffi::GtkWidget, ffi::GtkWidgetClass, WidgetClass>) @implements Buildable;
57  | |
58  | |     match fn {
59  | |         get_type => || ffi::gtk_widget_get_type(),
60  | |     }
61  | | }
    | |_- method `to_glib` not found for this
    |
    = help: items from traits can only be used if the trait is implemented and in scope
    = note: the following trait defines an item `to_glib`, perhaps you need to implement it:
            candidate #1: `glib::translate::ToGlib`
    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

error[E0605]: non-primitive cast: `unsafe extern "C" fn(*mut libc::c_void, *mut libc::c_void) -> *mut ffi::GtkWidget {<O as auto::list_box::ListBoxExt>::bind_model::create_widget_func_func::<'a, P, Q, R>}` as `unsafe extern "C" fn(*mut gobject_ffi::GObject, *mut libc::c_void) -> *mut ffi::GtkWidget`
   --> src/auto/list_box.rs:202:78
    |
202 |         let create_widget_func = if create_widget_func_data.is_some() { Some(create_widget_func_func::<'a, P, Q, R> as _) } else { None };
    |                                                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: an `as` expression can only be used to convert between primitive types. Consider using the `From` trait

error[E0057]: this function takes 1 parameter but 0 parameters were supplied
   --> src/auto/list_box.rs:196:17
    |
196 |                 callback()
    |                 ^^^^^^^^^^ expected 1 parameter

error[E0599]: no method named `to_glib` found for type `auto::widget::Widget` in the current scope
   --> src/auto/list_box.rs:200:17
    |
200 |               res.to_glib()
    |                   ^^^^^^^
    | 
   ::: src/auto/widget.rs:55:1
    |
55  | / glib_wrapper! {
56  | |     pub struct Widget(Object<ffi::GtkWidget, ffi::GtkWidgetClass, WidgetClass>) @implements Buildable;
57  | |
58  | |     match fn {
59  | |         get_type => || ffi::gtk_widget_get_type(),
60  | |     }
61  | | }
    | |_- method `to_glib` not found for this
    |
    = help: items from traits can only be used if the trait is implemented and in scope
    = note: the following trait defines an item `to_glib`, perhaps you need to implement it:
            candidate #1: `glib::translate::ToGlib`
    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

    Building [=======================================================> ] 65/66: gtk                                                      
error: aborting due to 6 previous errors

Some errors occurred: E0057, E0599, E0605.
For more information about an error, try `rustc --explain E0057`.
error: Could not compile `gtk`.
} else {
panic!("cannot get closure...")
};
res.to_glib()

This comment has been minimized.

@sdroege

sdroege Jan 29, 2019

Author Member

This needs to be to_glib_full()

unsafe extern "C" fn create_widget_func_func<'a, P: IsA<gio::ListModel> + 'a, Q: Into<Option<&'a P>>, R: Fn(&glib::Object) -> Widget + 'static>(item: glib_ffi::gpointer, user_data: glib_ffi::gpointer) -> *mut ffi::GtkWidget {
let callback: &Option<R> = &*(user_data as *mut _);
let res = if let Some(ref callback) = *callback {
callback()

This comment has been minimized.

@sdroege

sdroege Jan 29, 2019

Author Member

The callback should get the first parameter of the FFI function passed as &Object

};
res.to_glib()
}
let create_widget_func = if create_widget_func_data.is_some() { Some(create_widget_func_func::<'a, P, Q, R> as _) } else { None };

This comment has been minimized.

@sdroege

sdroege Jan 29, 2019

Author Member

There's a mismatch with the pointer type of the first argument here. *mut libc::c_void vs. *mut gobject_ffi::GObject. The sys crate is correct, the code generated here is wrong.

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 29, 2019

Now only waiting for gtk-rs/gir#713

@EPashkin EPashkin referenced this pull request Jan 29, 2019

Merged

Define manual traits #766

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 31, 2019

Once we can generate the bind_model() functions, I'm not sure if we should use the generated ones here or still keep manual implementations around.

The generated one would require the user to call Some(Box::new(|foo, bar| {....})) which is not very user-friendly but it works. A nicer, more rusty API would be to have a bind_model() like the existing one (not accepting None!) and in addition a unbind_model() that only does that, but that's not something we can autogenerate as it depends on the semantics of the function.

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 31, 2019

About ListBox::bind_model:
First thing can be done by declare parameter nullable=false,
and agree that unbind_model need be in manual trait,
at minimum until we discover way to generate it.

Also we have problem with return type: it changed to base type,
maybe we need teach gir replace these return types in future.
Until then I don't see big problem in use generated bind_model and manual unbind_model.

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 31, 2019

About ListBox::bind_model:
First thing can be done by declare parameter nullable=false,

Indeed, forgot that we can simply override nullability :)

and agree that unbind_model need be in manual trait,
at minimum until we discover way to generate it.

This would need a new configuration or gobject-introspection annotation. There's simply no information for that anywhere currently :)

Also we have problem with return type: it changed to base type,
maybe we need teach gir replace these return types in future.

That was actually a mistake in the manual code. It can be any kind of widget that is returned there. If it's a ListBoxRow then it's taken as-is, if it's another kind of widget then it's first placed into a ListBoxRow and only then added to the list box.

So the autogenerated version is fine.

@sdroege sdroege force-pushed the sdroege:listbox-closures branch from c1ebb5e to ea749f8 Jan 31, 2019

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 31, 2019

This should be good to go now

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 31, 2019

error[E0583]: file not found for module `flow_box`
   --> D:/eap/rust/0/gtk\src\lib.rs:238:5
    |
238 | mod flow_box;
    |     ^^^^^^^^
    |
    = help: name the file either flow_box.rs or flow_box\mod.rs inside the directory "D:/eap/rust/0/gtk\src"
@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 31, 2019

@sdroege Can you add fix to examples before this?

Remove various duplicated functions
And generate a few more functions that we can.

@sdroege sdroege force-pushed the sdroege:listbox-closures branch from ea749f8 to 7730207 Jan 31, 2019

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 31, 2019

error[E0583]: file not found for module `flow_box`

Oops, forgot to add the new file.

@sdroege Can you add fix to examples before this?

What fix do they need? Should work as before.

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 31, 2019

What fix do they need? Should work as before.

Ah I see: gtk-rs/examples#224

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jan 31, 2019

👍

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Jan 31, 2019

@GuillaumeGomez please merge :) Failures are fixed by the examples PR.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Jan 31, 2019

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit b87812d into gtk-rs:master Jan 31, 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
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.