Deriving Clone on ObjectSubclass is unsound

[sdroege]

It would create a clone of just the instance private struct. Calling ObjectSubclass::get_instance() on it would then get a reference to some random memory.

This needs to be prevented somehow. Maybe via a cleverly placed assert_not_impl_all in the glib_object_subclass! macro.

[jplatte]

So really anything implementing ObjectSubclass must not be constructed without going through some glib functionality? That seems like it would be impossible to enforce, since Clone is just one of an endless amount of ways of instantiating a type.

[sdroege]

On 16 October 2020 00:33:30 EEST, Jonas Platte notifications@github.com wrote:

So really anything implementing ObjectSubclass must not be constructed without going through some glib functionality? That seems like it would be impossible to enforce, since Clone is just one of an endless amount of ways of instantiating a type.

We could require a non cloneable ZST to be stored inside the struct, and make that only constructable inside glib.

All not beautiful

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[jplatte]

Okay, so I am right? Ouch 😅

I guess a good solution API-wise would be having a custom method receiver wrapper type (still unstable unfortunately).

Could a wrapper type maybe work without it being the receiver in an impl MySubclass block, so users have to instead write impl Obj<MySubclass> / impl Whatever for Obj<MySubclass>?

[sdroege]

That could also work indeed, so we already have two potential solutions :) I'd prefer yours if custom receiver types were stable but like this both are a bit suboptimal. Will think about it a bit more :)

[YaLTeR]

Even something like this is UB I guess:

impl ObjectSubclass for FooPrivate {
    const NAME: &'static str = "Foo";
    type Type = Foo;
    type ParentType = glib::Object;
    type Instance = glib::subclass::simple::InstanceStruct<Self>;
    type Class = glib::subclass::simple::ClassStruct<Self>;

    glib_object_subclass!();

    fn new() -> Self {
        let x = Self {};
        let _boom = x.get_instance();
        x
    }
}

[sdroege]

Yes, same thing really. Fortunately this will immediately crash when you try to make use of the instance instead of causing hard to debug issues :)

[sdroege]

I think the solution here is waiting for arbitrary Self types to become stable. Then we could have a Pin-like wrapper around the references of the impl struct wherever it is used that guarantees that it's allocated correctly, and then use fn foo(self: PinLikeThing<&Self>, ...) for all the functions.

Without that it's going to be very unergonomic to implement.

[sdroege]

Not that it solves the general problem, but https://github.com/gtk-rs/gtk-rs-core/pull/783 implements ToOwned for ObjectSubclass and then you can't derive/implement Clone anymore.