lvgl / lv_binding_rust

LVGL bindings for Rust. A powerful and easy-to-use embedded GUI with many widgets, advanced visual effects (opacity, antialiasing, animations) and low memory requirements (16K RAM, 64K Flash).
MIT License
687 stars 71 forks source link

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

Open Ddystopia opened 7 months ago

Ddystopia commented 7 months ago

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.