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

GeneralizedNewtypeDeriving broken on GHC 7.8.2 #12

Open
tazjin opened this issue May 8, 2014 · 23 comments
Open

GeneralizedNewtypeDeriving broken on GHC 7.8.2 #12

tazjin opened this issue May 8, 2014 · 23 comments

Comments

@tazjin
Copy link

tazjin commented May 8, 2014

Hej,

I noticed this today when trying to build my blog with GHC 7.8.2

Minimal complete example:

{-# LANGUAGE GeneralizedNewtypeDeriving #-}

import Data.SafeCopy

newtype Foo = Foo Integer
            deriving (SafeCopy)

Output:

~/s/acid-test> ghc Broken.hs 
[1 of 1] Compiling Main             ( Broken.hs, Broken.o )

Broken.hs:6:23:
    Could not coerce from ‘Kind Integer’ to ‘Kind Foo’
      because the first type argument of ‘Kind’ has role Nominal,
      but the arguments ‘Integer’ and ‘Foo’ differ
      arising from the coercion of the method ‘kind’ from type
                   ‘Kind Integer’ to type ‘Kind Foo’
    Possible fix:
      use a standalone 'deriving instance' declaration,
        so you can specify the instance context yourself
    When deriving the instance for (SafeCopy Foo)

Using the TH call deriveSafeCopy works fine, but seems to generate instances incompatible with earlier instances derived automatically. Fails with an error: Could not parse saved checkpoint due to the following error: Failed reading: safecopy: Integer: Cannot find getter associated with this version number: Version {unVersion = 5131390}

Any ideas?

//V

@benma
Copy link

benma commented May 17, 2014

I am having issues with this too. Could a safecopy dev please take a look? Thanks a lot!

@jaccokrijnen
Copy link

Same problem here.

@tazjin
Copy link
Author

tazjin commented Sep 4, 2014

So I ended up doing an acid-state migration on an older GHC version to a setup where I call the TH function deriveSafeCopy for every newtype. This works but is unfortunately not as clean.

@dschalk
Copy link

dschalk commented Nov 8, 2014

The acid-state example in The Happstack Book and some of the acid-state repository examples don't compile with GHC-7.8.3. The error messages say coercion is impossible because Data.SafeCopy.SafeCopy.Kind is nominal while two arguments differ. Simon Peyton-Jones explains what changed with the GHC-7.8.x in a video presentation at "https://skillsmatter.com/skillscasts/5296-safe-zero-cost-coercions-in-haskell".
The compiler messages says"Possible fix: use a standalone 'deriving instance' declaration . . ." Has anyone had any success implementing this suggestion? I wanted to use acid-state but the example code doesn't compile with the new GHC and I don't want to use GHC 7.x because the problem is not a bug in GHC-7.8.3; the problem stems from a bug having been fixed (see the Jones presentation). How are people dealing with this situation? Is the solution so trivial that nobody has bothered to mention it here? Are people removing "newtype" from their code, or using "unsafeCoerce", or what?

@tazjin
Copy link
Author

tazjin commented Nov 8, 2014

@dschalk As I mentioned in my last comment I've been using the Template Haskell functions instead, so I still create newtype wrappers but then instead call deriveSafeCopy on the types. See this commit for a minor example.

@dschalk
Copy link

dschalk commented Nov 8, 2014

Thanks. This works for me too.

@sdroege
Copy link

sdroege commented Dec 24, 2014

The problem with this seems to be that it changes the serialization in an incompatible way. At least I can't load the on-disk state anymore after this change, and it's not clear how to write a migration path for that as the old code does not compile anymore.

@sdroege
Copy link

sdroege commented Dec 24, 2014

I solved it too now by migrating with an older version of GHC, and then removing the old types to make it compile again with newer GHC.

Maybe the docs should have at least a warning about using GeneralizedNewtypeDeriving, and the examples should stop using it.

@MichaelXavier
Copy link

@dschalk @tazjin I'm a bit confused. WIll switching to standalone deriving not work? I've got a lot of code that used GND and I'm trying to figure out if i should use standalone instance or TH without having to go through a major migration headache.

@sdroege
Copy link

sdroege commented Jan 16, 2015

@MichaelXavier during my testing standalone deriving did not work for all instances, and for those it did it changed the representation somehow to make a migration necessary nonetheless. In the end I just used deriveSafeCopy for every type and migrated them all to that then.

@MichaelXavier
Copy link

@sdroege since "migrated" is an overloaded term in this context I want to clarify: using standalone instances created in some cases byte-incompatible safecopy whereas code that was previously using GND that you changed to deriveSafeCopy always produced byte-compatible safecopy?

@sdroege
Copy link

sdroege commented Jan 16, 2015

@MichaelXavier No, sorry. Both deriveSafeCopy and standalone instances created byte-incompatible instances, but standalone instances did not work for all types for me (got compiler errors for some). So I settled with using deriveSafeCopy for everything and migrated the old GND instances to the new byte-incompatible deriveSafeCopy instances.

@MichaelXavier
Copy link

Jeeze. We could really use some comment from the package authors. This can cause a whole lot of damage. I'm in the process of adding safecopy tests to all of my projects that use it and I guess hand-writing dozens of safecopy instances.

@ozataman
Copy link

ozataman commented Feb 4, 2015

Would really love to hear comments from the package authors on this. This is holding up our migration of a large code base from 7.6 to 7.8. We have dozens of newtypes with GND and it would be a major headache to manually migrate them. Even if we put in the work, what's even scarier is the potential for mistakes and data corruption in production.

@lemmih
Copy link
Member

lemmih commented Feb 5, 2015

I'm not sure what kind of comment you want. Either someone writes a bit of TH code to mimic GND or you'll have to write the instances by hand. It sucks that you can no longer use GND but it's not the end of the world.

@MichaelXavier
Copy link

Can the library provide a migration path for this? Thankfully 7.8 raises a compile time error when this is encountered, so the user has to stop and think. It would be much better if the docs for this library had a section on this and provided a function that mimicked the GND. The alternative is the user guesses the right thing to do (standalone instance derive according to GHC's suggestion) and breaks safecopy on all their newtypes silently.

@lemmih
Copy link
Member

lemmih commented Feb 5, 2015

Yes, I would like that very much. Since several people (including you) are affected by this, surely we won't have to wait too long for a pull request. :)

@lemmih
Copy link
Member

lemmih commented Feb 5, 2015

I quite interested in when standalone deriving isn't the same as ordinary deriving. It would be helpful if @ozataman added a test case.

Edit: Oops, meant @sdroege, not @ozataman.

@MichaelXavier
Copy link

@ozataman and I work together. Part of our upgrade involves getting a test suite around safecopy instances to test this. We haven't had the time yet to get back to that project the last week or so. @sdroege since you have experienced this incompatibility firsthand, do you think you could produce an example of standalone producing different output? Maybe its confined to certain types that were GNDed.

@sdroege
Copy link

sdroege commented Feb 6, 2015

See this commit for the case where it broke for me (just a little toy project in case someone is wondering):
sdroege/duckduckbot@d5a2f79
deriveSafeCopy worked on both but broke the serialization format, standalone deriving caused a compiler error (with ghc 7.8.3)

I won't have time to look into this in the next days, so if it's urgent for anybody please take those information and try to reproduce it. Otherwise I'll try to have some time for this in the following weeks.

@lemmih
Copy link
Member

lemmih commented Feb 8, 2015

That is expected. The TH code is very different from GND. The interesting case is when standalone deriving is different from ordinary deriving.

@sdroege
Copy link

sdroege commented Feb 8, 2015

Sure, the problem for me was more that standalone deriving did not compile at all 😄

@MichaelXavier
Copy link

@lemmih I've finally gotten back to this and I can confirm in my case that standalone deriving produces the exact same error:

    Could not coerce from Kind Text to Kind TextWrapper
      because the first type argument of Kind has role Nominal,
      but the arguments Text and TextWrapper differ
      arising from a use of GHC.Prim.coerce
    In the expression: GHC.Prim.coerce (kind :: Kind Text) :: Kind TextWrapper
    In an equation for kind’:
        kind = GHC.Prim.coerce (kind :: Kind Text) :: Kind TextWrapper
    When typechecking the code for  kind
      in a standalone derived instance for SafeCopy TextWrapper’:
      To see the code I am typechecking, use -ddump-deriv
    In the instance declaration for SafeCopy TextWrapper

I've been playing around with some ways to generically implement these safecopy instances to delegate to the wrapped type's implementation but because Contained does not export anything and is not a Functor, I can't see a way I could wrap the existing implementation. Instead I have to look up the implementation and then re-implement, inserting the newtype Constructor as needed.

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

8 participants