darcs

Patch 1934 add -fno-cse to D.P.Prim.Named given the unsafePerformIO

Title add -fno-cse to D.P.Prim.Named given the unsafePerformIO
Superseder Nosy List ganesh
Related Issues
Status in-discussion Assigned To
Milestone

Created on 2019-09-22.22:33:29 by ganesh, last changed 2019-09-23.21:55:14 by bf.

Files
File name Status Uploaded Type Edit Remove
add-_fno_cse-to-d_p_prim_named-given-the-unsafeperformio.dpatch ganesh, 2019-09-22.22:33:29 application/x-darcs-patch
patch-preview.txt ganesh, 2019-09-22.22:33:29 text/x-darcs-patch
unnamed ganesh, 2019-09-22.22:33:29 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg21538 (view) Author: ganesh Date: 2019-09-22.22:33:29
I won't screen this immediately. It's technically
correct, but maybe not worth the noise given we hope
to drop fromAnonymousPrim completely. And none of our
other unsafePerformIOs have it.

1 patch for repository darcs-unstable@darcs.net:screened:

patch 957497857baceec7e129ac4cff61eedc27dca863
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sun Sep 22 21:51:31 BST 2019
  * add -fno-cse to D.P.Prim.Named given the unsafePerformIO
Attachments
msg21546 (view) Author: bf Date: 2019-09-23.12:14:09
> I won't screen this immediately. It's technically
> correct, but maybe not worth the noise given we hope
> to drop fromAnonymousPrim completely. And none of our
> other unsafePerformIOs have it.

I was wondering about that. The docs also say something about disabling
full laziness optimization...
msg21550 (view) Author: ganesh Date: 2019-09-23.13:06:34
-no-full-laziness isn't needed in this particular case
because the unsafePerformIO can't float outside the lambda -
its body uses a variable bound by the function.
msg21553 (view) Author: bf Date: 2019-09-23.16:38:38
> -no-full-laziness isn't needed in this particular case
> because the unsafePerformIO can't float outside the lambda -
> its body uses a variable bound by the function.

I am glad you feel confident to say that. I have read the paragraph in
the docs and didn't really understand what that one was about...
msg21554 (view) Author: ganesh Date: 2019-09-23.16:52:29
I think the example helps.

  f x = unsafePerformIO (newIORef [])

needs the flag because otherwise the compiler might turn it into something
like

  f = let v = unsafePerformIO (newIORef []) in \x -> v

But if you have

  f x = unsafePerformIO (newIORef [x])

it's ok because the x on the right hand side makes that transformation 
invalid.
msg21563 (view) Author: bf Date: 2019-09-23.21:55:13
> I think the example helps.
> 
>   f x = unsafePerformIO (newIORef [])
> 
> needs the flag because otherwise the compiler might turn it into something
> like
> 
>   f = let v = unsafePerformIO (newIORef []) in \x -> v
> 
> But if you have
> 
>   f x = unsafePerformIO (newIORef [x])
> 
> it's ok because the x on the right hand side makes that transformation 
> invalid.

Okay, thanks, I think I get it now. Still, quite subtle. I wouldn't want
to apply that kind of reasoning to code I write if it can be helped.
Concentrating global mutable variables in Darcs.Util.Global may have
been a good idea to start with, so we can just disable all possibly
disastrous optimizations in that one module. However it seems this
hasn't really worked out to contain use of unsafePerformIO:

> grep -rwl unsafePerformIO src
src/Darcs/Patch/Match.hs
src/Darcs/Patch/Info.hs
src/Darcs/Patch/Progress.hs
src/Darcs/Patch/Prim/Named.hs
src/Darcs/Util/IsoDate.hs
src/Darcs/Util/Global.hs
src/Darcs/Util/AtExit.hs
src/Darcs/Util/Download.hs
src/Darcs/Util/Printer/Color.hs
src/Darcs/Util/ByteString.hs
src/Darcs/Util/Ssh.hs
src/Darcs/Util/Progress.hs
src/Darcs/Repository/Hashed.hs
src/Darcs/Repository/Cache.hs
src/Darcs/UI/Email.hs
History
Date User Action Args
2019-09-22 22:33:29ganeshcreate
2019-09-22 22:34:36ganeshsetstatus: needs-screening -> in-discussion
2019-09-23 12:14:09bfsetmessages: + msg21546
2019-09-23 13:06:34ganeshsetmessages: + msg21550
2019-09-23 16:38:38bfsetmessages: + msg21553
2019-09-23 16:52:30ganeshsetmessages: + msg21554
2019-09-23 21:55:14bfsetmessages: + msg21563