statig icon indicating copy to clipboard operation
statig copied to clipboard

Issue with context lifetimes

Open DylanRJohnston opened this issue 1 year ago • 5 comments

Hey all, I've been trying to integrate this library into Bevy for helping managing entity states. However I've encountered some issues with the borrow checker and passing in mutable references via Context to the state machine. I believe the root of the issue is that in IntoStateMachine type Ctx<'ctx> only has a single lifetime, which is not expressive enough to pass around mutable references to structs that contain mutable borrows themselves.

I've tried to reduce the problem to some minimal examples

mod example_broken {
    struct Foo;

    struct Event<'a> {
        value: &'a mut Foo,
    }

    struct Context<'a, 'b> {
        event: &'a mut Event<'b>,
    }

    type Ctx<'a> = Context<'a, 'a>;

    fn takes_context(context: &mut Ctx<'_>) {
        unimplemented!()
    }

    fn example(mut event: Event) {
        // Error: Event does not live long enough
        takes_context(&mut Context { event: &mut event })
    }
}

This example I think shows the root of the problem, the type alias type Ctx<'a> = Context<'a, 'a> forces the lifetime of the borrow 'a in event: &'a mut Event<'b> to be the same as the data borrowed by Event 'b. In this case, Event contains data borrowed from the ECS system, whereas 'a only lives for the lifetime of example. Hence the problem, it wants event in example to life as long as the data borrowed from the ECS system.

If we allow type Ctx<'a> to have two lifetime parameters type Ctx<'a, 'b> this is enough to allow the borrow checker to track the lifetimes independently.

mod example_working {
    struct Foo;

    struct Event<'a> {
        value: &'a mut Foo,
    }

    struct Context<'a, 'b> {
        event: &'a mut Event<'b>,
    }

    type Ctx<'a, 'b> = Context<'a, 'b>;

    fn takes_context(context: &mut Ctx<'_, '_>) {
        unimplemented!()
    }

    fn example(mut event: Event) {
        takes_context(&mut Context { event: &mut event })
    }
}

Another solution I explored was adding lifetime constraints to the function itself, however this makes the function longer a valid system for bevy.

mod example_lifetime_constraints {
    struct Foo;

    struct Event<'a> {
        value: &'a mut Foo,
    }

    struct Context<'a, 'b> {
        event: &'a mut Event<'b>,
    }

    type Ctx<'a> = Context<'a, 'a>;

    fn takes_context(context: &mut Ctx<'_>) {
        unimplemented!()
    }

    fn example<'event, 'value>(event: &'event mut Event<'value>)
    where
        'event: 'value,
    {
        takes_context(&mut Context { event: event })
    }
}

Another approach is to use Rc<RefCell<Event>> which lets us eliminate the compile checking of the problematic borrow of event so that Context only has a single lifetime and is thus compatible with the type Ctx<'ctx> definition.

mod example_ref_cell {
    use std::{cell::RefCell, rc::Rc};

    struct Foo;

    struct Event<'a> {
        value: &'a mut Foo,
    }

    impl<'a> Event<'a> {
        fn mutate(&mut self) {
            unimplemented!()
        }
    }

    struct Context<'a> {
        event: Rc<RefCell<Event<'a>>>,
    }

    type Ctx<'a> = Context<'a>;

    fn takes_context(context: &mut Ctx<'_>) {
        context.event.borrow_mut().mutate()
    }

    fn example(mut event: Event) {
        let rc = Rc::new(RefCell::new(event));

        for i in 1..3 {
            takes_context(&mut Context { event: rc.clone() })
        }
    }
}

Curiously, if you pass the &mut event directly i.e. Context is just a type alias, it works?

mod example_bare {
    struct Foo;

    struct Event<'a> {
        value: &'a mut Foo,
    }

    impl<'a> Event<'a> {
        fn mutate(&mut self) {
            unimplemented!()
        }
    }

    type Context<'a> = Event<'a>;

    type Ctx<'a> = Context<'a>;

    fn takes_context(context: &mut Ctx<'_>) {
        context.mutate()
    }

    fn example(mut event: Event) {
        for i in 1..3 {
            takes_context(&mut event);
        }
    }
}```

DylanRJohnston avatar Dec 22 '23 01:12 DylanRJohnston