Generalize merge

[treeowl]

Generalize types of merge methods.

[Ericson2314]

It might still be possible to push the function push the application to the deeper so the parameter is not exposed in the class method? Then it could still have @saulzar's signature? I do think that is better for future compatibility.

[treeowl]

What does "push the function" mean?

[treeowl]

By the way: if the concern is the performance characteristics, I think we might be able to do something like

mergeG :: GCompare k => (forall a. Coercible (q a) (Event t (v a)))
         => DMap k q -> Event t (DMap k v)

It's not very pretty, but I think it'll probably let you use both Compose (Event t) Identity and Event t. I haven't tried it though.

[Ericson2314]

@treeowl sorry about my hastily-written comment. I edited it to be hopefully be clear. I am just concerned about not wanted/needing that parameter if we get something like my ghc-proposals/ghc-proposals#248.

[treeowl]

Sorry, "push the application to the deeper" doesn't mean anything to be either. Can you show what you mean?

[Ericson2314]

I started playing with the code, and then I realized I was being stupid; I was thinking "oh you use the same argument every time", but of course the general use is not coerceEvent.

Also playing I around I noticed damn this is way more annoying then even when I was making my examples in #233 days ago. I thought it was GHC 8.4 but 8.6 was no better. It seems quite separate from future proposals, the special Constraint solver stuff is downright buggy with local reasoning. I tried replacing Event t with an abstract e to get it to stop, but it was of no avail. I now see why gcoerceWith and pattern matching on Coercion has been useless so far. What a mess!

Given all that, great work! I like your

mergeG :: GCompare k => (forall a. Coercible (q a) (Event t (v a)))
         => DMap k q -> Event t (DMap k v)

perhaps that could be an immediately-deprecated mergeG0, than then @saulzar's mergeG is written in terms of it as is merge? Maybe I'm too bullish on something being better in 8.8 or 8.10, but this status quo is just so awful I can hardly stand coding around it.

[treeowl]

No, mergeG0 doesn't work either. We can't satisfy (forall a. Coercible (q a) (Event t (v a))). The best we can do in that regard is (forall a. (Coercion (q a) (Event t (v a)))), coercing with that everywhere the current PR calls for nt.

[treeowl]

I'm really not a fan of mergeG0 anyway. What makes passing functions un-reflexish?

[oliver-batchelor]

I can confirm performance is identical (when patched against the same version of reflex) with this (unsurprisingly perhaps!).

[Ericson2314]

For me it's not reflex specific, just question a question of 1) does the higher order function increase expressive power 2) allow some more performant hand-fusing. traverse and >>= don't pass 1) when there's sequence and join, for example, but do pass 2).

The naive reflex implementation's mergeG doesn't pass 2) since it itself maps the function making an intermediate DMap, but it looks like Spider's does pass? If so, I take back my criticism: the HOF is a good idea even were coercions not so underpowered.

[treeowl]

@Ericson2314 the Pure implementation does indeed map over the DMap, but it did so before too. Now it just maps something else over it at the same time. I have very little sense of the performance implications for Spider when the function is non-trivial, because I'm still just starting to learn about what Spider gets up to.

[Ericson2314]

+1 OK that is a good sign too wrt the manual fusion justification.