alphaHeavy / protobuf

An implementation of Google's Protocol Buffers in Haskell.
http://hackage.haskell.org/package/protobuf
BSD 3-Clause "New" or "Revised" License
96 stars 18 forks source link

Optional Enumeration causes missing instance error on Encode #3

Closed nilscc closed 10 years ago

nilscc commented 10 years ago

The following code sample

{-# LANGUAGE DeriveGeneric #-}

import Data.ProtocolBuffers
import Data.TypeLevel.Num
import GHC.Generics (Generic)

data MyEnum = A | B
  deriving (Enum, Eq, Show)

data MyMessage = MyMessage { enum :: Optional D1 (Enumeration MyEnum) }
  deriving (Generic, Show)

instance Encode MyMessage

leads to the error:

No instance for (Data.Foldable.Foldable Data.Monoid.Last)
  arising from a use of `protobuf-0.1.2:Data.ProtocolBuffers.Encode.$gdmencode'
Possible fix:
  add an instance declaration for
  (Data.Foldable.Foldable Data.Monoid.Last)
In the expression:
  (protobuf-0.1.2:Data.ProtocolBuffers.Encode.$gdmencode)
In an equation for `encode':
    encode = (protobuf-0.1.2:Data.ProtocolBuffers.Encode.$gdmencode)
In the instance declaration for `Encode MyMessage'
nilscc commented 10 years ago

A simple fix would be:

deriving instance Foldable Last

This would be an orphan instance for a base-datatype though, which I don't really like.

NathanHowell commented 10 years ago

Another other option would be to replace Last with an in-package equivalent... I could also see about adding Foldable and Traversable instances to the newtypes in Data.Monoid and provide orphans for older versions of base.. Any preference?

nilscc commented 10 years ago

If you could get those instances into base that'd be probably be the cleanest solution. An in-package equivalent to Last would work, but it'd be kind of redundant… However, if you decide to add orphan instances to your package, please make sure to put them in an empty module (e.g. Data.ProtocolBuffers.Instances, and put an explanation to the top why it's there), since there is currently (afaik) no way to exclude instance definitions from an imported module and you could end up with unavoidable duplicate instance definitions errors otherwise. That would also make it unnecessary to check the current base version and you might just declare that module deprecated once base comes out with those instances.

NathanHowell commented 10 years ago

I'll see what I can do, for 7.8 it's probably too late. I'll definitely add the orphan... we apparently ran across this last year, see ef819f5b6a559f01685f00808dfe2fa01443f3e4, but never developed a proper fix.

NathanHowell commented 10 years ago

Does change 2056f20e1697b40dc3a769cf8357bcd3ef2bbe5f meet your needs?

NathanHowell commented 10 years ago

I've pushed 0.1.3 to Hackage, please reopen this ticket if you run into any issues.

nilscc commented 10 years ago

Looks good!