jeremyletang / rgtk

GTK+ bindings and wrappers for Rust (DEPRECATED SEE https://github.com/rust-gnome )
GNU Lesser General Public License v3.0
121 stars 22 forks source link

Started to update to last rustc version (removing box closure) #177

Closed GuillaumeGomez closed 9 years ago

GuillaumeGomez commented 9 years ago

Can you take a look at it @jeremyletang ?

gsingh93 commented 9 years ago

Bumping this. The alpha is out, so now is a good time to fix these build errors.

GuillaumeGomez commented 9 years ago

It's kinda difficult to update a code which I didn't write. That's why it's taking so much time.

gsingh93 commented 9 years ago

No problem. I'll try to help if I get time and can figure anything out.

GuillaumeGomez commented 9 years ago

It would be very appreciated ! Thanks !

gsingh93 commented 9 years ago

I commented out that macro error because I couldn't figure it out, and I continued to fix the other errors (we can come back to it later). My progress is tracked here: https://github.com/gsingh93/rgtk/tree/fix-build

One significant change was that ToCStr was removed, so we have to use CString::from_slice() now (this migration isn't complete yet). Another change is that CVec is gone. I'm not familiar with doing FFI, so I can't really figure out what that needs to be changed to.

GuillaumeGomez commented 9 years ago

For the ToCStr and CVec stuff, I made bindings here and here. The functions are just the same. For an example, look my rust-fmod repo or my rust-GSL repo.

jeremyletang commented 9 years ago

@gsingh93, you should prefer the new way provide by the std to handle the c types, as they should be maintained forever in the standard distribution of Rust.

I will check the macro / closure stuff if you can't @gsingh93, as soon as I have time.

gsingh93 commented 9 years ago

My plan is to convert all the ToCStr to use CString, use @GuillaumeGomez's CVec bindings for now, fix all the errors I can, then figure out the CVec stuff later (I think I'll have to wrap those types in my own structs that have destructors).

After fixing a bunch more errors, I'm left with a lot of these errors:

src/gtk/macros.rs:37:18: 37:22 error: the trait `Copy` may not be implemented for this type; the type has a destructor [E0184]
src/gtk/macros.rs:37         #[derive(Copy)]

Not sure what the right fix is. Removing the Copy trait might break more things. I don't even know what breaking change this is related to.

gsingh93 commented 9 years ago

What is the best way to fix the Copy/Drop issues? Specifically what's going on is that we have this macro:

macro_rules! struct_Widget(
    ($gtk_struct:ident) => (
        #[derive(Copy)]
        pub struct $gtk_struct {
            pointer: *mut ffi::C_GtkWidget
        }
    );
);

Which creates a widget struct that derives copy, but some widgets also call the impl_drop!() macro. Copy and Drop can't be implemented on the same object for safety reasons.

jeremyletang commented 9 years ago

hi @gsingh93, we can change the macro and remove the #[derive(Copy)] then implement manually impl Copy for YourWidget {} or a Clone impl for the struct which implements Drop too.

gsingh93 commented 9 years ago

I think removing the Copy impl is probably the best thing to do, because we shouldn't be copying the pointer without adding to the reference count like we do in Clone. I've removed it for now.

My current progress is in this pull request: https://github.com/jeremyletang/rgtk/pull/180. I think I'm done working on this for a little while, so if either of you want to continue, you can consider starting from there.

GuillaumeGomez commented 9 years ago

And it's now finished ! @jeremyletang, you can check !

gsingh93 commented 9 years ago

Nice! There was a macro in signals.rs that I had commented out. Don't forget to comment it back in and fix it before merging.

GuillaumeGomez commented 9 years ago

For the macro, it'll wait. I'm way too tired for that. Just waiting to get travis answer.