darcs

Patch 1729 clean up partitioning functions and expo... (and 3 more)

Title clean up partitioning functions and expo... (and 3 more)
Superseder Nosy List bfrk
Related Issues
Status accepted Assigned To
Milestone

Created on 2018-09-15.09:17:41 by bfrk, last changed 2018-10-17.06:47:29 by ganesh.

Files
File name Status Uploaded Type Edit Remove
clean-up-imports-in-darcs_patch_ident-to-avoid-warnings.dpatch bfrk, 2018-09-17.12:01:31 application/x-darcs-patch
clean-up-partitioning-functions-and-export-primed-versions.dpatch bfrk, 2018-09-15.09:17:40 application/x-darcs-patch
patch-preview.txt bfrk, 2018-09-15.09:17:40 text/x-darcs-patch
patch-preview.txt bfrk, 2018-09-17.12:01:31 text/x-darcs-patch
unnamed bfrk, 2018-09-15.09:17:40 text/plain
unnamed bfrk, 2018-09-17.12:01:31 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg20311 (view) Author: bfrk Date: 2018-09-15.09:17:40
Patches I extracted from my V3 branch, cleaned up and tested.

4 patches for repository http://darcs.net/screened:

patch 47da99770d4be412f7d9baed75eb0573a5b18d54
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Sep 14 09:00:22 CEST 2018
  * clean up partitioning functions and export primed versions
  
  The primed versions have been slightly modified in that they do not reverse
  the result, leaving that to the caller, if needed. Also, partitionRL' now
  returns a middle part similar to partitionFL'.
  
  The implementation is now slightly optimized: instead of trying commute
  first and only commuteWhatWeCan if that fails, we can use commuteWhatWeCan
  right away and pattern match on the dependencies.

patch 52eed7f3297d547768ff1775ff88a758f5af75e6
Author: Ben Franksen <ben.franksen@online.de>
Date:   Wed Nov  1 01:42:39 CET 2017
  * added foldRL_M, foldwFL, and foldwRL, renamed foldlFL/RL to foldFL/RL
  
  First, the renamings (removing the 'l') have been made because (a) the name
  is wrong for RL, which is actually right associative, and (b) because these
  are both the "natural" folds, i.e. the ones that replace the cons operator
  with a function.
  
  The other functions aren't used, yet, but will be used in the upcoming
  addition of RepoPatchV3. The 'w' variants allow the transformation of
  witnessed values, they are similar to foldRL/FL_M just not monadic.

patch 8c7276d1b1ab1b1c214b9bfb7acbb38b753f4ad8
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu May 24 17:53:30 CEST 2018
  * document D.P.V2.RepoPatch.allConflictsWith

patch 4dd74fd8ea2b0880f4ab614202d637580b324548
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Oct 29 23:14:12 CET 2017
  * add class Ident to abstract over patches with an identity
  
  This change is a preparation for the addition of identifiers to prim
  patches, so we can use the same algorithms for them.
Attachments
msg20314 (view) Author: bfrk Date: 2018-09-17.12:01:31
Hm, tested, yes, but did not test that it compiles w/o warnings.

1 patch for repository http://darcs.net/screened:

patch d005c1b29d2babad9fe81e55863f814533f27888
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Sep 17 13:58:41 CEST 2018
  * clean up imports in Darcs.Patch.Ident to avoid warnings
Attachments
msg20315 (view) Author: bfrk Date: 2018-09-18.20:20:14
>   * added foldRL_M, foldwFL, and foldwRL, renamed foldlFL/RL to foldFL/RL
>   
>   First, the renamings (removing the 'l') have been made because (a) the name
>   is wrong for RL, which is actually right associative, and (b) because these
>   are both the "natural" folds, i.e. the ones that replace the cons operator
>   with a function.
>   
>   The other functions aren't used, yet, but will be used in the upcoming
>   addition of RepoPatchV3. The 'w' variants allow the transformation of
>   witnessed values, they are similar to foldRL/FL_M just not monadic.

The patch name and long comment on this patch are unfortunately full of
bugs. First of all, I did /not/ rename foldlFL/RL to foldFL/RL. They are
/not/ the natural folds but instead the memory leaking left-associative
non-strict variant. I guess I became aware of that at some point and
forgot to edit the patch description. Furthermore, none of the functions
I added are any longer used by RepoPatchV3. I wish I could just
obliterate this patch but unfortunately I already screened it. We could
make an exception, as it seems the mirrors at hub.darcs.net are out of
date by a few months anyway.

Sorry for the mess.

In the long run we should strive to detect and eliminate FL and RL
traversals that build up long chains of thunks like the foldl from the
Prelude. It is not that hard: re-write the definition so that the
function parameter is applied in infix form: this tells you whether this
is a natural fold (which is okay, like foldr the input is consumed
lazily as a stream). If not, check whether it uses an accumulator for
the result (okay, too, this is the strict version, but should be
documented as such). Otherwise kill it or re-write with an accumulator.
msg20373 (view) Author: ganesh Date: 2018-10-06.01:53:11
>  * clean up partitioning functions and export primed versions

Fine

>  * added foldRL_M, foldwFL, and foldwRL, renamed foldlFL/RL to 
foldFL/RL

Fine

>  * document D.P.V2.RepoPatch.allConflictsWith

Fine

(one more to go, but I need a bit more time to go through
the last one about introducing Ident)
msg20374 (view) Author: ganesh Date: 2018-10-06.13:06:49
These two are fine too. Darcs.Patch.Ident is a bit of a
mishmash of different things, including properties
which we haven't traditionally put in the main code (but
I don't really mind if we start doing so). But I'm sure
we can give it more structure as this matures, and maybe
you already have in the V3 patches, I haven't looked at
those yet.

>  * clean up imports in Darcs.Patch.Ident to avoid warnings
>  * add class Ident to abstract over patches with an identity

I also previously missed your message about the patch
comment for fold etc. I tend to have multiple local copies
of screened so would rather not obliterate things. Wrong
patch comments are an annoyance but not the end of the
world, especially if the latest code is in good shape.
History
Date User Action Args
2018-09-15 09:17:41bfrkcreate
2018-09-15 09:22:24bfrksetstatus: needs-screening -> needs-review
2018-09-17 12:01:33bfrksetfiles: + patch-preview.txt, clean-up-imports-in-darcs_patch_ident-to-avoid-warnings.dpatch, unnamed
messages: + msg20314
2018-09-18 20:20:15bfrksetmessages: + msg20315
2018-10-06 01:53:11ganeshsetstatus: needs-review -> review-in-progress
messages: + msg20373
2018-10-06 13:06:49ganeshsetstatus: review-in-progress -> accepted-pending-tests
messages: + msg20374
2018-10-17 06:47:29ganeshsetstatus: accepted-pending-tests -> accepted