darcs

Patch 1919 replace Apply with PrimApply/Effect

Title replace Apply with PrimApply/Effect
Superseder Nosy List ganesh
Related Issues
Status in-discussion Assigned To
Milestone

Created on 2019-09-17.21:53:24 by ganesh, last changed 2019-09-27.09:36:47 by bf.

Files
File name Status Uploaded Type Edit Remove
move-primof-out-of-primpatchbase.dpatch ganesh, 2019-09-17.21:53:24 application/x-darcs-patch
patch-preview.txt ganesh, 2019-09-17.21:53:24 text/x-darcs-patch
unnamed ganesh, 2019-09-17.21:53:24 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg21441 (view) Author: ganesh Date: 2019-09-17.21:53:24
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
msg21627 (view) Author: bf Date: 2019-09-27.09:11:50
You have accepted the addition of unapply.

I guess this can now be marked this as rejected?
msg21628 (view) Author: ganesh Date: 2019-09-27.09:27:15
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).
msg21629 (view) Author: bf Date: 2019-09-27.09:36:47
> 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.
History
Date User Action Args
2019-09-17 21:53:24ganeshcreate
2019-09-17 21:53:47ganeshsetstatus: needs-screening -> in-discussion
title: Move PrimOf out of PrimPatchBase (and 1 more) -> replace Apply with PrimApply/Effect
2019-09-27 09:11:50bfsetmessages: + msg21627
2019-09-27 09:27:15ganeshsetmessages: + msg21628
2019-09-27 09:36:47bfsetmessages: + msg21629