Cast trampolines before transmuting

[EPashkin]

Fix warning introduced rust-lang/rust#19925
See gtk-rs/gtk#278

[GuillaumeGomez]

Good for me. Thanks @EPashkin!

[gkoz]

We can do better. I consider as usize the last resort but here the fns can actually match the GSourceFunc signature.

[EPashkin]

Then we need at minimum these changes:

1. make trampolines unsafe,
2. change trampolines func to gpointer and transmute it back
3. change parameter on call ffi function to Some(trampoline)

unsafe extern "C" fn trampoline(func: gpointer) -> gboolean {
    let func: &RefCell<Box<FnMut() -> Continue + 'static>> = transmute(func);
    let _guard = CallbackGuard::new();
    (&mut *func.borrow_mut())().to_glib()
}

pub fn idle_add<F>(func: F) -> u32
where F: FnMut() -> Continue + Send + 'static {
    unsafe {
        glib_ffi::g_idle_add_full(glib_ffi::G_PRIORITY_DEFAULT_IDLE, Some(trampoline),
            into_raw(func), Some(destroy_closure))
    }
}

[EPashkin]

I don't like part about unsafe

[gkoz]

Yes, something along those lines. Typically the trampolines do something unsafe (at least transmute the argument) so there is some justification for their being unsafe overall.

Here had the function not been unsafe I'd probably still put the meat of it in an unsafe block because letting func escape that block doesn't make much sense to me.

    let _guard = CallbackGuard::new();
    unsafe {
        let func: &RefCell<Box<FnMut() -> Continue + 'static>> = transmute(func);
        (&mut *func.borrow_mut())().to_glib()
    }

Finally, keeping the &RefCell<Box<FnMut() -> Continue + 'static>> bit in the signature is just as unsafe but in a more silent way.

[EPashkin]

On nightly build safe trampolines don't convert automatically to unsafe:

source.rs:74:75: 74:85 error: mismatched types:
 expected `unsafe extern "C" fn(*mut libc::c_void) -> i32`,
    found `extern "C" fn(*mut libc::c_void) -> i32 {source::trampoline}`
(expected unsafe fn,
    found normal fn) [E0308]
source.rs:74         glib_ffi::g_idle_add_full(glib_ffi::G_PRIORITY_DEFAULT_IDLE, Some(trampoline),
                                                                                       ^~~~~~~~~~
source.rs:74:75: 74:85 help: run `rustc --explain E0308` to see a detailed explanation

[gkoz]

Yes, unfortunately rustc is not smart enough in this case: while extern fn() {source::trampoline} coerces to extern fn() and extern fn() coerces to unsafe extern fn(), it can't make the transitive jump. It might accept

let trampoline: extern fn(gpointer) = trampoline;
g_idle_add_full(..., Some(trampoline), ...)

[EPashkin]

Yes it accept it with let trampoline: extern fn(gpointer) -> gboolean = trampoline;

[EPashkin]

Some(trampoline as extern fn(gpointer) -> gboolean) will do too.
And Some(trampoline as extern fn (_) -> _). Not sure that works with multiple params.

[gkoz]

Yeah looks like as isn't a dangerous cast in this form, just invokes a coercion.

[EPashkin]

So what variant use in glib?
In gtk I still prefer use transform(trampoline as usize) until generated is done as too many functions here.

[EPashkin]

extern "C" fn trampoline(func: gpointer) -> gboolean {
    let _guard = CallbackGuard::new();
    unsafe {
        let func: &RefCell<Box<FnMut() -> Continue + 'static>> = transmute(func);
        (&mut *func.borrow_mut())().to_glib()
    }
}

pub fn idle_add<F>(func: F) -> u32
where F: FnMut() -> Continue + Send + 'static {
    unsafe {
        glib_ffi::g_idle_add_full(glib_ffi::G_PRIORITY_DEFAULT_IDLE, Some(trampoline as extern fn (_) -> _),
            into_raw(func), Some(destroy_closure))
    }
}

or

unsafe extern "C" fn trampoline(func: gpointer) -> gboolean {
    let _guard = CallbackGuard::new();
    let func: &RefCell<Box<FnMut() -> Continue + 'static>> = transmute(func);
    (&mut *func.borrow_mut())().to_glib()
}

pub fn idle_add<F>(func: F) -> u32
where F: FnMut() -> Continue + Send + 'static {
    unsafe {
        glib_ffi::g_idle_add_full(glib_ffi::G_PRIORITY_DEFAULT_IDLE, Some(trampoline),
            into_raw(func), Some(destroy_closure))
    }
}

or something other?

[gkoz]

I'm happy with both. Personally I'd go for the second style because it's shorter and I don't think having whole unsafe fns makes a difference here.

[EPashkin]

Updated.

[gkoz]

Thanks!