gtk-rs / gtk-rs-core

Rust bindings for GNOME libraries
https://gtk-rs.org/gtk-rs-core
MIT License
281 stars 113 forks source link

Constructing immutable Object #32

Open kornelski opened 4 years ago

kornelski commented 4 years ago

It doesn't seem possible to create an Object subclass that is immutable, i.e. doesn't have setters.

glib::Object::new insists on allocating a default object and using setters to initialize.

I would like to start with already-initialized data, "wrap" it to be an Object, and implement only getters on it, so that I don't need to support interior mutability or default/empty object state.

BoxedType seems close in terms of using an existing Rust object, but I don't know if it can have properties: gtk-rs/glib#629

sdroege commented 4 years ago

I'm not sure I understand what you're looking for

Is your problem that your struct needs to be initialized from GObject property values or (even worse) from something that requires calling API on the GObject instance that is being created?

Can you outline in more detail what you're trying to do / what you're trying to get around? Is it the same case as the subclassing example in the gtk-rs examples where you can only create the values for inside your struct in ObjectImpl::constructed?

kornelski commented 4 years ago

That "out of thin air" part is the problem.

I have Rc<MyExistingRustStruct> which does not support interior mutability for all fields. Some fields are immutable and constant, so I can't have setters for them.

On a high level, I'm trying to make a sortable TreeView. My initial approach was updating it "manually" row by row, column by column, but it's been messy and totally buggy with sortable columns, so I'm looking for a better approach. I hoped I could simplify this by using an actual model and binding to it directly.

glib's construct-only and read-only properties are AFAIK just a runtime check, so don't free me from still having to change fields in my model to RefCell<Option>, and don't free me from having to implement setters.

I'm looking for something like glib::Object::from(MyExistingRustStruct{…}) instead of glib::Object::new(&[set all values one by one via setters on a blank object]).

sdroege commented 4 years ago

I see, yes that's the main issue currently. There are a couple of problems here

  1. in new() you could have access to the instance already but the problem is that it's "incomplete" at this point and thus unsafe to use. It would also not solve your specific problem (but related ones others had)
  2. You need to have an instance of your struct before GObject::constructed(), because construction properties (the ones you pass to Object::new()) will go through set_property().
    1. also means that you somehow need to have an instance of your struct before the first property is actually handled.

We might be able to do something around GObject::constructor() here (you have all construct property values in an array there), but it's not clear to me how that could look like in a safe way. You have all property values at that point, and calling the parent implementation of that virtual method is what is actually allocating the object. However as part of the parent implementation it will go into set_property().

I can only think of two solutions right now, but maybe you have other ideas:

kornelski commented 4 years ago

Thanks! So "cheating" with TLS seems fine:

thread_local! {
    pub static INIT_HACK: RefCell<Option<imp::MyType>> = RefCell::new(None);
}

mod imp {   
    impl ObjectSubclass for MyType {
        // …

        fn new() -> Self {
            INIT_HACK.with(|h| h.borrow_mut().take()).expect("init hack")
        }
    }
}

impl MyType {
    pub fn new(/* args */) -> MyType {
        INIT_HACK.with(|h| {
            *h.borrow_mut() = Some(imp::MyType {
                // args
            });
        });
        glib::Object::new(Self::static_type(), &[]).unwrap().downcast().unwrap()
    }
}

It'd be cool if gtk-rs could do something like this behind the scenes, so that it could implement Rust's From.

sdroege commented 4 years ago

Let's keep it open, I'm not happy with this solution and would like to find something better :)

ids1024 commented 3 years ago

It seems to me this is a broader issue that really applies to any subclass struct with at least one immutable member. It's rather a pain to have to use something like RefCell<Option<T>> for a member that should be provided by a CONSTRUCT_ONLY parameter (or a nice type-safe rust equivalent), and will remain unchanged for the rest of the object's lifetime.

This imposes a memory and runtime overhead, though that's probably insignificant. But it does make code uglier, and mean the fact it's not later mutated isn't statically enforced.

Edit: I guess once_cell::unsync::OnceCell would partially reduce the overhead, partially enforce the behavior, and make it more obvious to someone reading the code, but it still seems suboptimal.