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

Empty storable vectors in GHCi error with divide by zero. #167

Closed
mihaimaruseac opened this issue Apr 30, 2017 · 7 comments
Closed

Empty storable vectors in GHCi error with divide by zero. #167

mihaimaruseac opened this issue Apr 30, 2017 · 7 comments

Comments

@mihaimaruseac
Copy link
Contributor

mihaimaruseac commented Apr 30, 2017

In GHCi:

=> import Data.Vector.Storable
=> empty
*** Exception: divide by zero

Enabling -XNoMonomorphismRestriction, giving it an explicit type or having it compiled by ghc from a file makes the error go away.

This doesn't happen for either boxed (Data.Vector) or unboxed (Data.Vector.Unboxed) vectors.

I consider it a vector bug since it's an inconsistency between the unboxed and the storable case. But, if it is not, feel free to close this/raise it to the proper bug tracker.

@RyanGlScott
Copy link
Member

RyanGlScott commented May 1, 2017

Well, this issue didn't exist in GHC 7.10.3 or earlier by sheer coincidence. The reason it changed in 8.0 is that a new Storable () instance was added:

instance Storable () where
  sizeOf _ = 0
  alignment _ = 1
  peek _ = return ()
  poke _ _ = return ()

And luck just has it that if you type empty at the REPL with no type annotations, its type will default to Vector (). It seems that having sizeOf _ = 0 causes the divide by zero error. I'm not familiar enough with the codebase to know where the faulty division is happening, though.

@RyanGlScott
Copy link
Member

A quick search reveals this as a likely culprit:

instance Storable a => G.MVector MVector a where
  ...
  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)
      mx = maxBound `quot` size :: Int

@mihaimaruseac
Copy link
Contributor Author

Makes sense and indeed that seems to be the culprit.

Now, for solution, the simplest way would be to disallow making a storable vector of (). At least this will give educative error messages. I made #168 for this, as a starting point.

@cartazio
Copy link
Contributor

cartazio commented May 1, 2017 via email

@mihaimaruseac
Copy link
Contributor Author

Yeah, it's issue #105 (I only searched for it now). I see that no progress has been made on it :(

I changed my PR so now it divides by 1 if the size of the elements is 0.

@mihaimaruseac
Copy link
Contributor Author

Compiled the following with profiling

import qualified Data.Vector.Storable as VS

main = print (VS.empty :: VS.Vector ())

and indeed got the expected error

*** Exception (reporting due to +RTS -xc): (THUNK_STATIC), stack trace:
  GHC.Real.CAF
  --> evaluated by: Data.Vector.Storable.Mutable.basicUnsafeNew,
  called from Main.main,
  called from Main.CAF:main3
  --> evaluated by: Main.main,
  called from Main.CAF:main2
  --> evaluated by: Main.main
mm: divide by zero

The error goes away with fix I proposed in #168

@dolio
Copy link
Contributor

dolio commented May 3, 2017

Fix merged.

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

No branches or pull requests

4 participants