Docs for unsafeThaw should warn about the same things that unsafePerformIO

[bitonic]

Everything that the docs for unsafePerformIO say regarding NOINLINE, -fno-cse, and -fno-full-laziness applies to unsafeThaw. In fact, I think it's even easier to fall into weird behavior with unsafeThaw -- for example this program is unsafe:

main :: IO ()
main = foo >> foo

foo :: IO ()
foo = do
  indexVector <- V.unsafeThaw $ V.generate n id
  x <- VM.read indexVector 5
  VM.write indexVector 5 (x * x)
  print x

{-
Will probably print
5
5
5
25
-}

(See also https://ghc.haskell.org/trac/ghc/ticket/12656#comment:9 for related woes)

[bitonic]

Proposed new haddocks for unsafeThaw:

-- | /O(1)/ Unsafely convert an immutable vector to a mutable one without
-- copying. The immutable vector may not be used after this operation.
--
-- Moreover, @NOINLINE@, @-fno-cse@ and @-fno-full-laziness@ should be used to
-- ensure safety. See the documentation for 'unsafePerformIO' for more
-- information on this subject.

I'm currently unclear on whether unsafeFreeze needs the same warnings.

[cartazio]

Hrmm. This is a good start!
I'm not sure if you need all three at the same time.
Or at least there's some style tricks to make it better.
Do we want to have the full exposition somewhere ?

What are the analogous docs / exposition for unsafe perform IO

aren't the safety requirements more like unsafe dupable perform IO for
unsafe thaw and unsafe freeze? Either way it gives you the same value wrt
array read and writes . Just maybe a different heap allocated wrapper
constructor.

On Monday, October 3, 2016, Francesco Mazzoli [email protected]
wrote:

Proposed new haddocks for unsafeThaw:

-- | /O(1)/ Unsafely convert an immutable vector to a mutable one without

-- copying. The immutable vector may not be used after this operation.

-- Moreover, @noinline@, @-fno-cse@ and @-fno-full-laziness@ should be used to
-- ensure safety. See the documentation for 'unsafePerformIO' for more
-- information on this subject.

I'm currently unclear on whether unsafeFreeze needs the same warnings.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#139 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAQwvyXHSGpeDA6GUiay6TNTEPcJWoRks5qwSAWgaJpZM4KMtUs
.

[mgsloan]

I'm currently unclear on whether unsafeFreeze needs the same warnings.

I don't think it does. If you are sure that you do no further mutation of the mutable buffer, it doesn't matter if freezing it gets inlined.

[dolio]

We definitely need a more specific warning, if one is deemed necessary, because I don't think this case is particularly similar to unsafePerformIO. For instance, the 5 5 5 5 behavior in the linked ticket is caused by the vector being passed into unsafeThaw not being shared. Inlining the unsafeThaw part doesn't really make a difference. And turning on -fno-cse and -fno-full-laziness only affect unsafeThaw in that they might affect how shared the vectors you pass in are, and aren't needed because we're worried about the result of unsafeThaw.

[cartazio]

Yeah. This stuff is really subtle.

On Wednesday, October 5, 2016, dolio [email protected] wrote:

We definitely need a more specific warning, if one is deemed necessary,
because I don't think this case is particularly similar to unsafePerformIO.
For instance, the 5 5 5 5 behavior in the linked ticket is caused by the
vector being passed into unsafeThaw not being shared. Inlining the
unsafeThaw part doesn't really make a difference. And turning on -fno-cse
and -fno-full-laziness only affect unsafeThaw in that they might affect
how shared the vectors you pass in are, and aren't needed because we're
worried about the result of unsafeThaw.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#139 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAQwoKbxGS-IUyjq3j5ifEj-ZTBhSnyks5qxEAvgaJpZM4KMtUs
.

[lehins]

unsafePerformIO and unsafeThaw are not unsafe for same reasons and same recommendations do not apply. This piece of code:

  replicateM_ 5 $ do
    vm <- noinline V.unsafeThaw (V.generate 1 id)
    x <- VM.read vm 0
    print x
    VM.write vm 0 (100 + x)

will produce:

0
100
200
300
400

regardless of any ghc flags that I've tried. See my comment for more info on the ghc ticket: https://gitlab.haskell.org/ghc/ghc/-/issues/12656#note_347996

If we are to improve the documentation we need to figure out the exact guidelines for users, instead of just "try sprinkle some ghc flags and see if it helps"

[Shimuuar]

I don't think so. It's GHC optimizer creating/destroying sharing which is generally safe for pure code but readily apparent once you start mutating things in place. Bug happens and disappears at whim of optimizer so it's expected that seemingly equivalent programs will have different behaviors. Here's one that does have such behavior:

import qualified Data.Vector as V
import qualified Data.Vector.Mutable as VM

main :: IO ()
main = foo >> foo

foo :: IO ()
foo = do
  indexVector <- V.unsafeThaw $ V.generate 10 id
  x <- VM.read indexVector 5
  VM.write indexVector 5 (x * x)
  print x

$ ghc -fforce-recomp -O0 unsafe-thaw.hs && ./unsafe-thaw
[1 of 1] Compiling Main             ( unsafe-thaw.hs, unsafe-thaw.o )
Linking unsafe-thaw ...
5
25

$ ghc -fforce-recomp -O1 unsafe-thaw.hs && ./unsafe-thaw
[1 of 1] Compiling Main             ( unsafe-thaw.hs, unsafe-thaw.o )
Linking unsafe-thaw ...
5
5

After some thinking I come to conclusion that mutating vector after unsafeThaw is fundamentally unsafe. Safety depends on whether pure value is same of different (but equal) at different points of program execution. But haskell doesn't have notion of pointer equality so it's impossible reason about it without reasoning about GHC optimizer and how does it transforms programs.

All in all attempts to modify vector after unsafeThaw falls out of domain of software engineering and into realm of black magic, dark rituals, and unspeakable horrors. I think only advice I can give is: "don't attempt to mutate vector after unsafeThaw unless you know how to prevent GHC from aliasing buffers accidentally. We don't"

[lehins]

All in all attempts to modify vector after unsafeThaw falls out of domain of software engineering and into realm of black magic, dark rituals, and unspeakable horrors. I think only advice I can give is: "don't attempt to mutate vector after unsafeThaw unless you know how to prevent GHC from aliasing buffers accidentally. We don't"

🤣 I think we should include this sentence verbatim into haddock.