lv_binding_rust icon indicating copy to clipboard operation
lv_binding_rust copied to clipboard

`Obj::create`, `<widget>::create` and `<widget>::add_style` have wrong lifetimes and mutability

Open Ddystopia opened this issue 1 year ago • 0 comments

Hello, widgets have lifetime

impl<'a> Bar<'a>
pub fn create(parent: &mut impl NativeObject) -> LvResult<Self>

Which takes reference to some NativeObject and returns Bar with unbounded lifetime - it can be freely set to 'static, for example. In that case it would not ub though, because Obj has no free on drop:

pub fn example() -> Result<Label<'static>, LvError> {
    let mut parent = Obj::new()?;
    Label::create(&mut parent)
}

Obj has slightly different lifetime requirements, taking reference to some NativeObject and returns Obj with lifetime of that reference

impl<'a> Obj<'a>
    pub fn create(parent: &'a mut impl NativeObject) -> LvResult<Self>

A better approach would be to describes that lifetime like that:

impl<'a> Bar<'a> // And `Obj<'a`> too
pub fn create(parent: &(impl NativeObject + 'a)) -> LvResult<Self>

First change: we are not exclusively borrowing parent, so many children can exists. There are not real rust data to be mutated in parent, so we are allowed to get mutable pointers and mutate by them - all memory is owned by C code. If lvgl was written in Rust, it would've been required to have interior mutability and proof it is Sync via unsafe impl. &/&mut are about shared/exclusive access, not mutability.

Second change: Bar now is not dependent on lifetime of a reference, but on the lifetime of the parent itself. Consider use case: you create some tree of objects - all data are allocated on the lvgl heap. The lifetime 'a in &(impl NativeObject + 'a) represent region where parent would be alive. Then implicit lifetime of borrow and independence of the result from it tells that parent is still allowed to move freely, it is not pinned to the position it has - and it is perfectly fine, because it only has a box inside, no pointers or references are stored directly into the parent.

It will also allow stop leaking Obj because there will be a guarantee free is okay with lifetimes.

Those two changes in Obj and <widget> would allow following code to exist:

struct Tree<'a> {
    left: Obj<'a>,
    right: Obj<'a>,
    left_label: Label<'a>,
    right_label: Label<'a>,
}

pub fn example<'a>(parent: &Obj<'a>) -> Result<Tree<'a>, LvError> {
    let left = Obj::create(parent)?;
    let right = Obj::create(parent)?;
    let left_label = Label::create(&left)?;
    let right_label = Label::create(&right)?;
    Ok(Tree {
        left,
        right,
        left_label,
        right_label,
    })
}

The save logic applies to Style in it's current form, because it also has box inside. It will allow to give the same style to multiple elements - now it is impossible because of &mut.

Ddystopia avatar Apr 23 '24 08:04 Ddystopia