GregoryConrad / rearch-rs

Re-imagined approach to application design and architecture
https://crates.io/crates/rearch
MIT License
81 stars 4 forks source link

Aggregate the various effect types (`&mut`, `Lazy`, `Cloned`) via new trait (`StateTransformer`) #30

Closed GregoryConrad closed 8 months ago

GregoryConrad commented 8 months ago

I started noticing a ton of redundancy when writing regular side effects, lazy side effects, and cloneable side effects. Thus, we can make one generic side effect of each type (state, reducer, value, etc.) that takes in a generic input in order to produce different results. We can call these generic inputs StateTransformers or something similar. A trait for this could look something like:

pub trait StateTransformer: Send + 'static {
    type TransformedOutput<'a>;
    fn transform(&mut self) -> Self::TransformedOutput<'_>;
}

I’m thinking we can ship MutRef, Cloned, LazyMutRef, LazyCloned, Ref, and LazyRef state transformers out of the box. MutRef will just return the data with &mut, LazyMutRef will do the same but initializes with a oncecell Lazy, Cloned returns a clone of the value, etc. it’d be even cooler if we can compose the state transformers; i.e., make lazy a transformer that takes another transformer as an input, so we can write:

effects::state(Lazy(Cloned(|| 1234)))

or maybe:

effects::state(Cloned(Lazy(|| 1234)))

GregoryConrad commented 8 months ago

I think an issue with Cloned(Lazy(T)) is that we would need a counterpart NonLazy because we can’t make Cloned work for both T and StateTransformer because negative trait impls don’t exist. Thus, we would need to do Lazy(Cloned(T)), which I think looks better anyways. However, I also don’t know if that’s possible, because the clone would be for cloning the function passed into Cloned and not the value itself…

Thus, we probably would need to expose an api like:

or, what will probably happen, is just expose LazyFooBar et al.

GregoryConrad commented 8 months ago

I think I'm running into a compiler bug, because this looks correct to me:

trait StateTransformer: Send + 'static {
    type TransformedOutput<'a>;
    fn transform(&mut self) -> Self::TransformedOutput<'_>;
}

fn dumbed_down_raw<ST: StateTransformer>(
    st: ST,
) -> impl for<'a> SideEffect<Api<'a> = ST::TransformedOutput<'a>> {
    EffectLifetimeFixer0::new(move |register: SideEffectRegistrar| {
        let (transformer, _, _) = register.raw(st);
        transformer.transform()
    })
}

// Not needed to reproduce bug, but helpful for understanding:
struct Ref<T>(pub T);
impl<T: Send + 'static> StateTransformer for Ref<T> {
    type TransformedOutput<'a> = &'a T;
    fn transform(&mut self) -> Self::TransformedOutput<'_> {
        &self.0
    }
}

But produces this compilation error:

error[E0308]: mismatched types
  --> rearch-effects/src/lib.rs:91:9
   |
91 |         transformer.transform()
   |         ^^^^^^^^^^^^^^^^^^^^^^^ expected `&mut _`, found associated type
   |
   = note: expected mutable reference `&mut _`
                found associated type `<ST as StateTransformer>::TransformedOutput<'_>`
   = help: consider constraining the associated type `<ST as StateTransformer>::TransformedOutput<'_>` to `&mut _`
   = note: for more information, visit https://doc.rust-lang.org/book/ch19-03-advanced-traits.html
help: try removing the method call
   |
91 -         transformer.transform()
91 +         transformer
   |

error[E0271]: type mismatch resolving `<EffectLifetimeFixer0<{closure@lib.rs:89:31}> as SideEffect>::Api<'a> == <ST as StateTransformer>::TransformedOutput<'a>`
  --> rearch-effects/src/lib.rs:88:6
   |
88 | ) -> impl for<'a> SideEffect<Api<'a> = ST::TransformedOutput<'a>...
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&mut _`, found associated type
   |
   = note: expected mutable reference `&mut _`
                found associated type `<ST as StateTransformer>::TransformedOutput<'_>`
help: consider constraining the associated type `<ST as StateTransformer>::TransformedOutput<'_>` to `&mut _`
   |
86 | pub fn ez_raw<ST: StateTransformer<TransformedOutput<'_> = &mut _>>(
   |                                   ++++++++++++++++++++++++++++++++

Some errors have detailed explanations: E0271, E0308.
GregoryConrad commented 8 months ago

Just tried this, which would enable something like effects::state<Cloned<_>>(1234) (which I think I like more, since we can maybe then use default generics to specify MutRef as the default):

pub trait StateTransformer: Send + 'static {
    type Input;
    fn from_input(input: Self::Input) -> Self;
    type Output<'a>;
    fn as_output(&mut self) -> Self::Output<'_>;
}

pub fn new_raw<ST: StateTransformer /* TODO = MutRef */>(
    initial: ST::Input,
) -> impl for<'a> SideEffect<Api<'a> = <ST as StateTransformer>::Output<'a>> {
    EffectLifetimeFixer0::new(move |register: SideEffectRegistrar| {
        let (transformer, _, _) = register.raw(ST::from_input(initial));
        transformer.as_output()
    })
}

Same set of compilation errors as before.

GregoryConrad commented 8 months ago

This seems to be some weird error where the compiler for some reason thinks the first part of the API should always be getting a &mut T or T: 'static, but never anything else (like a &T or StateTransformer::Output<'a>). ~This is definitely some sort of compiler bug but I have too much going on right now to look into it further.~ Edit: I just didn't remember that the bounds in the EffectLifetimeFixer required &mut; see https://github.com/GregoryConrad/rearch-rs/issues/30#issuecomment-1890839087

0e4ef622 commented 8 months ago

I think that's just because the lifetime fixer wants &mut as a return value. It can compile with another fixer.

pub trait StateTransformer: Send + 'static {
    type Input;
    fn from_input(input: Self::Input) -> Self;
    type Output<'a>;
    fn as_output(&mut self) -> Self::Output<'_>;
}

struct LifetimeFixer<F, ST>(F, PhantomData<ST>);
impl<F, ST: StateTransformer> SideEffect for LifetimeFixer<F, ST>
where
    F: FnOnce(SideEffectRegistrar<'_>) -> ST::Output<'_>,
{
    type Api<'a> = ST::Output<'a>;
    fn build<'a>(self, registrar: SideEffectRegistrar<'a>) -> Self::Api<'a> {
        self.0(registrar)
    }
}

impl<F, ST> LifetimeFixer<F, ST>
where
    F: FnOnce(SideEffectRegistrar<'_>) -> ST::Output<'_>,
    ST: StateTransformer,
{
    fn new(f: F) -> Self {
        LifetimeFixer(f, PhantomData)
    }
}

pub fn new_raw<ST: StateTransformer /* TODO = MutRef */>(
    initial: ST::Input,
) -> impl for<'a> SideEffect<Api<'a> = ST::Output<'a>> {
    LifetimeFixer::<_, ST>::new(move |register: SideEffectRegistrar| {
        let (transformer, _, _) = register.raw(ST::from_input(initial));
        transformer.as_output()
    })
}
GregoryConrad commented 8 months ago

because the lifetime fixer wants &mut as a return value

Duh, can’t believe I didn’t think of that 😂

Thanks, I’ll take another crack at this today, or at least within the next couple of days

GregoryConrad commented 8 months ago

I would want the API to look like this I think:

effects::raw::<_, Cloned>(1234)

But that requires non_lifetime_binders, which, based on the little I know, looks a hell of lot like higher-kinded types. The implementation would look something like:

trait A<T> {}
fn b<R, S: for<T> A<T>>() {}

So then I tried:

trait A<T> {}
fn b<T, S: A<T>>(t: T) {}

Which yields an API like:

b::<_, B<_>>(0);

// based on:
struct B<T>(T);
impl<T> A<T> for B<T> {}

Issue is I would want a default generic, probably MutRef. But that isn't possible because of https://github.com/rust-lang/rust/issues/36887

Granted, the alternative API:

effects::raw(MutRef::from(1234))

Would never have a default either. So I just really need to decide between:

effects::raw::<_, MutRef<_>>(123)
// and:
effects::raw(MutRef::from(123))

Leaning more toward the first option at the moment, despite the few extra keystrokes, in the hope non_lifetime_binders eventually lands.

GregoryConrad commented 8 months ago

Oh, I think I was really overcomplicating it. We don't need a T type since we have StateTransformer::Input. So the API is one of:

effects::raw::<Ref<_>>(123)
// or
effects::raw(Ref::new(123))

I much prefer the first option. It's possible some state transformers could be like Ref(123) instead of Ref::new(123), which would save keystrokes, but not all of them would support that, and I'd rather keep API consistency.

All in all, I think the generic approach is better because it more clearly just defines the behavior of what is going on rather than the input data itself.