Add NFData instance for ByteArray - #65
Conversation
| -- modules when there is no safe alternative | ||
|
|
||
| #if MIN_VERSION_base(4,9,0) || (MIN_VERSION_base(4,6,0) && !MIN_VERSION_base(4,7,0)) | ||
| #if (MIN_VERSION_base(4,9,0) && !MIN_VERSION_base(4,17,0)) || (MIN_VERSION_base(4,6,0) && !MIN_VERSION_base(4,7,0)) |
There was a problem hiding this comment.
Why Data.ByteArray is not Safe Haskell? Is this an oversight?
, because it uses unsafeCoerce# and manipulates state tokens manually. How do you feel about it?
it's not a reason. As long as exported API is safe, it can be marked (and should be) safe. error is Safe. e.g. though its implementation is "GHC specific".
There was a problem hiding this comment.
The Data.ByteArray module exports indexing functions that segfault if you provide an out-of-bounds index.
There was a problem hiding this comment.
I’m kinda ambivalent here. My personal belief is that Trustworthy is suitable only for code, whose correctness was automatically verified. But if there is a strong desire to mark Data.ByteArray as such, I would not oppose. Note that there is not prior art: neither Data.Primitive.ByteArray, nor Data.Text.Array, nor Data.ByteString.Short.Internal are marked as safe, so I would not say that this breaks user expectations.
There was a problem hiding this comment.
@andrewthad which indexing functions: https://gitlab.haskell.org/ghc/ghc/-/blob/100ffe75f509a73f1b26e768237888646f522b6c/libraries/base/Data/ByteArray.hs#L16-18
If the no-checks segfaulting functions are exported, then it's a good question whether API should be split into Safe and .Unsafe part, as it is with ByteString e.g. ByteArray is very safe. At the very least the segfault trap should be documented!
@Bodigrim Data.ByteString and Data.ByteString.Short are trustworthy, Data.ByteString.Unsafe and Data.ByteString.Short.Internal aren't.
There was a problem hiding this comment.
@andrewthad none of the functions is exported:
module Data.ByteArray (
ByteArray(..)
) where
|
@cigsender this is ready for review now. |
5 participants
ByteArrayhas just landed into GHC HEAD: https://gitlab.haskell.org/ghc/ghc/-/commit/1f8090933268b1ca071bc4a8a35b0f1828a76fceUp to you whether to merge it now or warehouse until GHC 9.4 RC.
I'm not particularly proud of reviving
Control.DeepSeq.BackDoor, but it feels wrong to markData.ByteArrayitself asSafeor evenTrustworthy, because it usesunsafeCoerce#and manipulates state tokens manually. How do you feel about it?