Skip to content
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

No more error: division by zero when trying to print or operate on empty storable vectors #168

Merged
merged 2 commits into from
May 3, 2017
Merged

Conversation

mihaimaruseac
Copy link
Contributor

No description provided.

No more `error: division by zero` when trying to print or operate on empty storable vectors
@dolio
Copy link
Contributor

dolio commented May 3, 2017

Looks good. At first I was thinking we could do better, because you need no storage for () and presumably anything else of size 0. But then I realized that mallocVector will take care of this as well as can be done, so it's fine.

@cartazio
Copy link
Contributor

cartazio commented May 3, 2017

@dolio :
reasoning being that the code is now


  basicUnsafeNew n
    | n < 0 = error $ "Storable.basicUnsafeNew: negative length: " ++ show n
    | n > mx = error $ "Storable.basicUnsafeNew: length too large: " ++ show n
    | otherwise = unsafePrimToPrim $ do
        fp <- mallocVector n
        return $ MVector n fp
    where
      size = sizeOf (undefined :: a) `max` 1
      mx = maxBound `quot` size :: Int

and mallocVector uses the storable width times the number of elements to determine size of allocation?

@dolio
Copy link
Contributor

dolio commented May 4, 2017

Yeah mallocVector will allocate 0 bytes for a size 0 type. I suppose in principle we could do something a little dicey, like put a null pointer in there, and trust that the peek/poke in the Storable instance don't do anything with the pointer. But that's a pretty marginal win.

@lehins lehins mentioned this pull request Jan 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants