Less restrictive closure lifetime

[GuillaumeGomez]

I think we can now make the lifetime less strict on widgets' signals. Don't you think @EPashkin?

cc @antoyo

[EPashkin]

You mean + 'static part of closure? Related issues: https://github.com/gtk-rs/gtk/issues/16, https://github.com/gtk-rs/gtk/issues/263, https://github.com/gtk-rs/gtk/issues/390 What make you think that it can be removed or lessened? How?

[GuillaumeGomez]

You mean + 'static part of closure?

Yes

What make you think that it can be removed or lessened? How?

I think this solution for signals could be viable (to be tested and confirmed):

fn connect_clicked<'a, F: Fn(&Self) + 'a>(&'a self, f: F) -> u64 {
     unsafe {
         let f: Box_<Box_<Fn(&Self) + 'a>> = Box_::new(Box_::new(f));
         connect(self.to_glib_none().0, "clicked",
             transmute(clicked_trampoline::<Self> as usize), Box_::into_raw(f) as *mut _)
     }
}

[EPashkin]

IMHO it good idea if works.

[GuillaumeGomez]

That's the whole problem: testing. Once I'm done with the Nullable API (I swear: I'll try to finish it before the end of the week!) I'll try to find out how to be sure we won't break everything with it.

[antoyo]

I updated gir in this branch to test that. Here is an example of using the generated code:

extern crate gtk;

use std::mem;

use gtk::{Button, ButtonExt, ContainerExt, Inhibit, Label, WidgetExt, Window, WindowType};
use gtk::Orientation::Vertical;

struct Model {
    counter: i32,
}

struct Widget {
    button: Button,
    label: Label,
    model: Model,
}

impl Widget {
    fn clicked(&self) {
        self.button.set_sensitive(false);
        self.label.set_text(&format!("Clicked: {}", self.model.counter));
    }
}

fn main() {
    gtk::init().unwrap();
    let window = Window::new(WindowType::Toplevel);
    let vbox = gtk::Box::new(Vertical, 0);
    window.add(&vbox);

    let button = Button::new_with_label("Button");
    vbox.add(&button);

    let button2 = Button::new_with_label("Button");
    vbox.add(&button2);

    let label = Label::new("Label");
    vbox.add(&label);

    {
        let widget = Widget {
            button,
            label,
            model: Model {
                counter: 42,
            },
        };

        button2.connect_clicked(|_| {
            widget.clicked();
        });

        widget.button.connect_clicked(|_| {
            widget.clicked();
        });
        mem::forget(widget);
    }

    window.show_all();
    window.connect_delete_event(|_, _| {
        gtk::main_quit();
        Inhibit(false)
    });
    gtk::main();
}

I'm not sure if it is sound (doesn't look it), but valgrind doesn't complain.

(And to be honest, I'm not sure how useful it will be since the closure is Fn and not FnMut.)

[EPashkin]

@antoyo Seems this really worked. Thanks P.S. Currently we use Rc<RefCell<T>> to use mutable variables in closures. See https://github.com/gtk-rs/examples/blob/pending/src/bin/multi_windows.rs

[antoyo]

Ah, using a Box shows the issue:

extern crate gtk;

use std::mem;

use gtk::{Button, ButtonExt, ContainerExt, Inhibit, Label, WidgetExt, Window, WindowType};
use gtk::Orientation::Vertical;

struct Model {
    counter: i32,
}

struct Widget {
    button: Button,
    label: Label,
    model: Model,
}

impl Widget {
    fn clicked(&self) {
        self.button.set_sensitive(false);
        self.label.set_text(&format!("Clicked: {}", self.model.counter));
    }
}

fn main() {
    gtk::init().unwrap();
    let window = Window::new(WindowType::Toplevel);
    let vbox = gtk::Box::new(Vertical, 0);
    window.add(&vbox);

    let button = Button::new_with_label("Button");
    vbox.add(&button);

    let button2 = Button::new_with_label("Button");
    vbox.add(&button2);

    let label = Label::new("Label");
    vbox.add(&label);

    {
        let widget = Box::new(Widget {
            button,
            label,
            model: Model {
                counter: 42,
            },
        });

        button2.connect_clicked(|_| {
            widget.clicked();
        });

        widget.button.connect_clicked(|_| {
            widget.clicked();
        });
    }

    window.show_all();
    window.connect_delete_event(|_, _| {
        gtk::main_quit();
        Inhibit(false)
    });
    gtk::main();
}

This program segfaults. I'll investigate to see why the lifetime is not taken into account.

[antoyo]

I think I now understand why it compiles: the lifetime 'object_lifetime that is now generated is not the lifetime of the struct, but the lifetime of the borrow.

To make this work, I believe we would need to add a lifetime parameter to the widgets, so gtk::Button will become gtk::Button<'a, T: 'a>, with a paramater like marker: PhantomData<&'a F> and the connect methods will be like:

fn connect_clicked<F: Fn(&Self) + 'a>(&self, f: F) -> u64;

I don't think it is worth adding this complexity since the benefits seem minimal (i.e. still need to use Rc<RefCell<T>> to mutate data inside the callback).

[LoveIsGrief]

I'm currently running into the issue of mutating data in a callback and as a rust beginner, being forced to use RefCell is not only verbose and "boilerplatey" (can be wrapped in a macro of course) but also not immediately apparent. Is there a reason FnMut cannot be used? Would existing code break?

[sdroege]

Your signal handler can cause the signal to be emitted again, which would call your signal handler from itself. That breaks the requirements of FnMut (and in case of RefCell can lead to a panic if you don't release the borrow early enough again inside your signal handler).

[LoveIsGrief]

Would that stop FnMut from compiling?If it compiles and  RefCell can lead to a panic, maybe using FnMut might be more desirable. There is no boilerplate code necessary and it's more accessible to users new to Rust or Gtk.

Or can arbitrary data be stored in the properties of Gtk components? Then the current signatures could stay and only a hint +example in the documentation would be necessary.

[sdroege]

Would that stop FnMut from compiling?

No, it would compile and would be unsound then, leading to undefined behaviour.

Or can arbitrary data be stored in the properties of Gtk components? Then the current signatures could stay and only a hint +example in the documentation would be necessary.

If you create a subclass of a GTK widget then you can store whatever data you want together with the widget.