I finished off my experiment to replace the Apply
type class with PrimApply/Effect, following on from the
discussion about Invertible here:

https://lists.osuosl.org/pipermail/darcs-devel/2019-September/020262.html

Apply stays as an empty class to avoid lots of UndecidableInstances.

As I mentioned in the previous mail the difficulty is code that
is overloaded on both prims and non-prims, that wants to call apply.
The solution is to make sure that prims continue to be instances
of Apply, so each needs a trivial instance of Effect, as well as
PrimOf prim ~ prim.

Overall I'm not really convinced it's an improvement, as we just
seem to be trading off messiness in Apply with messiness in
Effect/PrimOf. But perhaps it's at least moving it down a layer.
And of course we now have your 'unapply' idea which provides
a less invasive solution for Apply (Invertible p).

An alternative idea was to make a wrapper type 'ApplyablePrim'
so that we could have

instance PrimApply prim => Apply (ApplyablePrim prim)

and then change all the high-level places that call code that's
overloaded on prim/non-prim to add the wrapper when it is
passing a prim. But that turns out to be a lot of annoying
wrapping and I gave up.

2 patches for repository darcs-unstable@darcs.net:screened:

patch c86f011113412fc60b579238d19f8508ba5d4a19
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Tue Sep 17 17:56:29 BST 2019
  * Move PrimOf out of PrimPatchBase
  
  Quite a few PrimPatchBase instances then turn out to be
  unnecessary.
  

patch ff23d0ace2cba00e29053a3c060f0b677ada8af3
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Tue Sep 17 22:37:53 BST 2019
  * replace the apply method with applyPrimFL . effect

Attachments

• move-primof-out-of-primpatchbase.dpatch
• patch-preview.txt
• unnamed

You have accepted the addition of unapply.

I guess this can now be marked this as rejected?

It's up to you. I'm certainly not pushing this change, but it still
makes some sense as an alternative solution. It would simplify
the range of Apply instances we have and enforce by construction
the concept of ApplyState p ~ ApplyState (PrimOf p).

> It's up to you. I'm certainly not pushing this change, but it still
> makes some sense as an alternative solution. It would simplify
> the range of Apply instances we have and enforce by construction
> the concept of ApplyState p ~ ApplyState (PrimOf p).

Hmm, yes. Makes sense. I'll leave it alone until I find the time to
compare the two approaches in detail.