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

• add-_fno_cse-to-d_p_prim_named-given-the-unsafeperformio.dpatch
• patch-preview.txt
• unnamed

> 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...

-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.

> -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...

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.

> 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

Updated version with more explanation. I'm going to screen this now as
I think it makes sense while we still have the unsafePerformIO.

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

patch 6fccddb0ab22715c8ff345f4032303941cfd647d
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sat Jan 25 17:22:00 GMT 2020
  * add -fno-cse to D.P.Prim.Named given the unsafePerformIO

Attachments

• add-_fno_cse-to-d_p_prim_named-given-the-unsafeperformio.dpatch
• patch-preview.txt
• unnamed

> I think it makes sense while we still have the unsafePerformIO.

Yup.

________________________________

Helmholtz-Zentrum Berlin für Materialien und Energie GmbH

Mitglied der Hermann von Helmholtz-Gemeinschaft Deutscher Forschungszentren e.V.

Aufsichtsrat: Vorsitzender Dr. Volkmar Dietz, stv. Vorsitzende Dr. Jutta Koch-Unterseher
Geschäftsführung: Prof. Dr. Bernd Rech (Sprecher), Prof. Dr. Jan Lüning, Thomas Frederking

Sitz Berlin, AG Charlottenburg, 89 HRB 5583

Postadresse:
Hahn-Meitner-Platz 1
D-14109 Berlin

Looks good.

Looks good.