Switch `SideEffect` to GAT lifetime

[GregoryConrad]

Side effects, when expressed correctly, should just be functions that work for any given SideEffectRegistrar<'a> and should themselves not be dependent upon any lifetime (i.e., the SideEffectRegistrar itself). However, due to limitations in Rust's current implementation of higher-ranked lifetimes and some other related weird type-related bugs I've found along the way, this is not possible at the moment and the current (technically incorrect) implementation of side effects was born (they work, just not optimally). Rust just isn't quite ready for some of the odd, edge-case stuff I am trying to put it through.

Here's some sample code I was working on that properly expresses side effects, for when the time comes that it is possible:

pub trait SideEffect {
    type Api<'a>;
    fn build<'a>(self, registrar: SideEffectRegistrar<'a>) -> Self::Api<'a>;
}
impl<T, F: FnOnce(SideEffectRegistrar) -> T> SideEffect for F {
    type Api<'a> = T;
    fn build<'a>(self, registrar: SideEffectRegistrar<'a>) -> Self::Api<'a> {
        self(registrar)
    }
}

And for future reference, if I need it, here's some side-effect code I was experimenting with using the correct approach.

// pub fn raw<T: Send + 'static>(
//     initial: T,
// ) -> impl for<'a> SideEffect<
//     Api<'a> = (
//         &'a mut T,
//         impl Fn(Box<dyn FnOnce(&mut T)>) + Clone + Send + Sync,
//     ),
// > {
//     fix_lifetime(move |register: SideEffectRegistrar| register.raw(initial))
// }
// pub fn raw<T: Send + 'static>(
//     initial: T,
// ) -> impl for<'a> FnOnce(
//     SideEffectRegistrar<'a>,
// ) -> (
//     &'a mut T,
//     impl Fn(Box<dyn FnOnce(&mut T)>) + Clone + Send + Sync,
// ) {
//     fix_lifetime(move |register: SideEffectRegistrar| register.raw(initial))
// }

fn fix_lifetime<F, T, R>(f: F) -> F
where
    F: for<'a> FnOnce(SideEffectRegistrar<'a>) -> (&'a mut T, R),
{
    f
}

pub fn as_listener() -> impl for<'a> SideEffect<Api<'a> = ()> {
    |_: SideEffectRegistrar| {}
}

// TODO THis works because it's not a closure and the lifetime elision rules work correctly
pub fn zero_raw(register: SideEffectRegistrar) -> (&mut u8, impl Fn(u8) + Clone + Send + Sync) {
    let (state, rebuild) = register.raw(0);
    let set_state = move |new_state| {
        rebuild(Box::new(move |state| *state = new_state));
    };
    (state, set_state)
}

[GregoryConrad]

And in case I forget: this issue is what is blocking the improved Container::listen() API. I was unable to implement it correctly using the current SideEffect trait (perhaps I need to just try again but have been unsuccessful so far), so this new approach to side effects was needed.

[GregoryConrad]

TODO: try getting around this with a manual trait impl of SideEffect instead of relying on closure/fn higher ranked lifetimes

[GregoryConrad]

Tried the following as a workaround:

pub struct Raw<T>(T);
impl<T: Send + 'static> SideEffect for Raw<T> {
    type Api<'a> = (&'a mut T, impl CData + Fn(Box<dyn FnOnce(&mut T)>));
    fn build(self, registrar: SideEffectRegistrar<'_>) -> Self::Api<'_> {
        registrar.raw(self.0)
    }
}

pub fn as_listener() -> impl for<'a> SideEffect<Api<'a> = ()> {
    |_: SideEffectRegistrar| {}
}

pub struct State<T>(pub T);
impl<T: Send + 'static> SideEffect for State<T> {
    type Api<'a> = (&'a mut T, impl CData + Fn(T));
    fn build(self, registrar: SideEffectRegistrar<'_>) -> Self::Api<'_> {
        let (state, rebuild) = registrar.raw(self.0);
        let set_state = move |new_state| {
            rebuild(Box::new(|state| *state = new_state));
        };
        (state, set_state)
    }
}

Didn't work because:

• A slew of compiler bugs. Don't even know where to begin
  • Big one is several error[E0700]: hidden type for `impl (Fn(u8)) + CData` captures lifetime that does not appear in bounds and related messages, but that makes no sense because the impl Trait is 'static and doesn't depend on the lifetime 'a from the higher ranked lifetime (it does a .clone() of the needed elements to be 'static).
• Requires nightly for impl_trait_in_assoc_type, which has been attempted to be pushed to stabilization, but is currently stalled.

So, really looks like this issue is impossible to achieve until Rust has some substantial compiler bug fixes.

[0e4ef622]

This seems to compile. It's a newtype around the closure instead of just the initial state.

fn fix_lifetime<F, T, R1, R2>(f: F) -> F
where
    F: for<'a> FnOnce(SideEffectRegistrar<'a>) -> (&'a mut T, R1, R2),
{
    f
}

pub struct Raw<F>(F);
impl<T, F, R1, R2> SideEffect for Raw<F>
where
    T: Send + 'static,
    F: FnOnce(SideEffectRegistrar<'_>) -> (&mut T, R1, R2),
{
    type Api<'a> = (&'a mut T, R1, R2);
    fn build<'a>(self, registrar: SideEffectRegistrar<'a>) -> Self::Api<'a> {
        self.0(registrar)
    }
}

pub fn raw<T: Send + 'static>(
    initial: T,
) -> impl for<'a> SideEffect<
    Api<'a> = (
        &'a mut T,
        impl CData + Fn(Box<dyn FnOnce(&mut T)>),
        Arc<dyn Send + Sync + Fn(Box<dyn FnOnce()>)>,
    ),
> {
    Raw(
        fix_lifetime(move |register: SideEffectRegistrar<'_>| register.raw(initial))
    )
}

[GregoryConrad]

@0e4ef622 Huh! I am super impressed that (1) you were able to adapt the code samples I gave above to accommodate the changes I've made to side effects since then and (2) modify those changes into something that actually compiles. Thanks for sharing that snippet!

I'll take a closer look into this workaround... SideEffect GATs were the last "big thing" preventing ReArch's 1.0, so this would be great to have resolved. (The only other thing separating ReArch-rs from a 1.0 is the desire to get a few experienced Rust devs out there to do a code review to make sure I followed best-practices and exposed a forward-thinking API; I did the best I could but a more seasoned set of eyes is always a plus.)