mgsloan / store

Fast binary serialization in Haskell
MIT License
109 stars 35 forks source link

store fails to build with vector-0.13.0.0 #174

Closed facundominguez closed 2 years ago

facundominguez commented 2 years ago

The cause of the breakage seems to be that vector-0.13.0.0 is adding some new types UnboxViaPrim a and As a b, for which store can't generate instances automatically.

For instance, one of the errors is

src/Data/Store/Internal.hs:824:3: error:
    • No instance for (Store (Data.Vector.Unboxed.Base.Vector b))
        arising from a use of ‘peek’
      There are instances for similar types:
...
    • In a stmt of a 'do' block: c0f0 <- peek
      In the expression:
        do c0f0 <- peek
           return (Data.Vector.Unboxed.Base.V_UnboxAs c0f0)
      In an equation for ‘peek’:
          peek
            = do c0f0 <- peek
                 return (Data.Vector.Unboxed.Base.V_UnboxAs c0f0)

The offending generated instance is

    instance Store (Data.Vector.Unboxed.Base.Vector (Data.Vector.Unboxed.Base.As a b)) where
      size
        = case Unit size :: Size (Data.Vector.Unboxed.Base.Vector b) of
            Unit(ConstSize c0f0)
              | True -> ConstSize (0 + sz0)
              where
                  sz0 = c0f0
            Unitc0f0
              -> (VarSize
                    $ (\ x_ajmY
                         -> (0 + (case x_ajmY of {
                                    Data.Vector.Unboxed.Base.V_UnboxAs f0
                                      -> (getSizeWith c0f0) f0 }))))
      peek
        = do c0f0 <- peek
             return (Data.Vector.Unboxed.Base.V_UnboxAs c0f0)
      poke
        = \ val
            -> case val of {
                 Data.Vector.Unboxed.Base.V_UnboxAs c0f0 -> do poke c0f0 }

It looks like store would need to be smarter to add to the context the necessary constraints. Or (question 1) is store supposed to skip generating these instances?

One side question that strives from this is (question 2) whether maintenance could be made simpler by fixing the set of type class instances that are generated. That is store-0.70 could generate Store instances for types A, B, and C, no matter which version of vector is used. If one wants to use Store instances for newer types D and E, then one has to upgrade to store-0.71, and so on whenever newer types appear in dependencies.

mgsloan commented 2 years ago

A fix for this will be included in 0.7.16!

Or (question 1) is store supposed to skip generating these instances?

IIUC generating instances here would only be useful to derive Store instances via the As newtype added in 0.13.0.0. While theoretically useful, other mechanisms should work too and it would be weird to use Vector's As to derive store instances. So, I've decided to omit these.

One side question that strives from this is (question 2) whether maintenance could be made simpler by fixing the set of type class instances that are generated.

I believe overall maintenance is reduced by automatically discovering which instances should be generated based on which types are provided by dependencies. This avoids a lot of boilerplate and tedious / error prone CPP. True, in some rare cases like this one, it causes trouble.

Also as a result of this, store's current code can also build with ghc from 6 years ago (7.10), and probably earlier.