-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use unsafeWithForeignPtr in Data.ByteString.Unsafe #401
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These instances do seem good to convert to the unsafe version. There's still a bunch of withForeignPtr
s in the library though, e.g. in BS.split
. Did you or @bgamari make a complete pass over these?
In functions like findIndex
and filter
, I suspect we have to stick with withForeignPtr
because the user-defined predicates can't be assumed to be safe?!
As far as I understand from https://gitlab.haskell.org/ghc/homepage/-/merge_requests/84/diffs, since @bgamari in b82da95 does not hesitate to use |
Sure. Let's look at the case of foldl' :: (a -> Word8 -> a) -> a -> ByteString -> a
foldl' f v (BS fp len) =
accursedUnutterablePerformIO $ unsafeWithForeignPtr fp g
where
g ptr = go v ptr
where
end = ptr `plusPtr` len
-- tail recursive; traverses array left to right
go !z !p | p == end = return z
| otherwise = do x <- peek p
go (f z x) (p `plusPtr` 1) Note how In general, the question to ask yourself in these situations is:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small suggestion. LGTM apart from that.
* Use unsafeWithForeignPtr in Data.ByteString.Unsafe * Review remaining cases of withForeignPtr * Review suggestions
* Use unsafeWithForeignPtr in Data.ByteString.Unsafe * Review remaining cases of withForeignPtr * Review suggestions
As pointed out by @bgamari at https://gitlab.haskell.org/ghc/ghc/-/snippets/2920 and https://gitlab.haskell.org/ghc/ghc/-/issues/19557#note_357737