Opened 19 months ago

Closed 19 months ago

Last modified 18 months ago

#8813 closed task (fixed)

further support deriving instances Typeable1, Typeable2, etc

Reported by: maeder Owned by: dreixel
Priority: normal Milestone: 7.8.1
Component: libraries/base Version: 7.8.1-rc1
Keywords: Cc: simonpj
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC rejects valid program Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description

ghc-7.8.20140130 complains with

    Not in scope: type constructor or class ‛Typeable2’
    Perhaps you meant ‛Typeable’ (imported from Data.Typeable)

changing Typeable2 to Typeable fixes it but is incompatible with ghc-7.6:

    Expecting two more arguments to `Gr'
    In the stand-alone deriving instance for `Typeable Gr'

Attachments (2)

TypeableN.hs (850 bytes) - added by maeder 19 months ago.
proof of concept
Typeable.hs (4.4 KB) - added by maeder 19 months ago.

Download all attachments as: .zip

Change History (43)

comment:1 Changed 19 months ago by dreixel

  • Resolution set to wontfix
  • Status changed from new to closed

Typeable2 no longer exists. If you need your code to work with both 7.8 and 7.6, you can use CPP. There's also Data.OldTypeable in 7.8. Some more information:

https://ghc.haskell.org/trac/ghc/wiki/GhcKinds/PolyTypeable

comment:2 Changed 19 months ago by maeder

An (derived) instance of Data.OldTypeable.Typeable cannot be used for a context based on Data.Typeable. Therefore Data.OldTypeable is pretty useless.

Is it no option (i.e. better) to make Typeable{1,..,7} behave like Typeable (with a warning)?

comment:3 Changed 19 months ago by carter

@maeder, nope, please use CPP.

CPP allows us to better support evolving / improving GHC while still ensuring that code can work across multiple GHC versions. Please use it. :)

comment:4 follow-up: Changed 19 months ago by maeder

What is the problem to add (renamed) copies of Typeable (named TypeableN) and add generic instances "TypeableN k => Typeable k" and re-export those via Data.Typeable? This would allow to test unmodified existing code!!!

(These intermediate TypeableN classes may be marked to be deprecated later)

comment:5 follow-up: Changed 19 months ago by simonpj

The biggest single problem is that RC1 has been out some weeks; RC2 is due today; full release next week. Moreover the person (Pedro) who was working on this area is probably heads-down on his ICFP submission (deadline 1 March). There was quite a bit of email traffic on this at the time all this was nailed down (months ago) and no objections.

I'm honestly not sure of details of your proposal. Do you want to modify libraries only, or the compiler as well? Would you like to make a patch?

Simon

comment:6 in reply to: ↑ 4 Changed 19 months ago by dreixel

Replying to maeder:

What is the problem to add (renamed) copies of Typeable (named TypeableN) and add generic instances "TypeableN k => Typeable k" and re-export those via Data.Typeable? This would allow to test unmodified existing code!!!

(These intermediate TypeableN classes may be marked to be deprecated later)

I thought about ways to do this before, and never came to a reasonable solution. If you can find one, please let us know.

For example, instances like Typeable1 t => Typeable t are impossible, as all ts match this instance head. I also didn't see how to reconcile the type of the old typeOF with the new poly-kinded typeRep.

comment:7 follow-up: Changed 19 months ago by maeder

The other simple alternative is to patch the renamer and rename the class Data.Typeable.TypeableN to Data.Typeable.Typeable.

This surely does not reconcile typeOF and typeRep, but hand-written instances have been (sort of) deprecated long before.

I'm not familiar with the internals of Data.Typeable but the following is not impossible:

 {-# Language FlexibleInstances, UndecidableInstances #-}
class Pretty a where
  pretty :: a -> String
instance Show a => Pretty a where
  pretty = show

Furthermore, Data.OldTypeable should be removed. It's more harmful to provide a second incompatible Typeable class.

comment:8 in reply to: ↑ 7 Changed 19 months ago by dreixel

Replying to maeder:

The other simple alternative is to patch the renamer and rename the class Data.Typeable.TypeableN to Data.Typeable.Typeable.

This surely does not reconcile typeOF and typeRep, but hand-written instances have been (sort of) deprecated long before.

I'm not familiar with the internals of Data.Typeable

Well, I am, and I couldn't find an acceptable solution. But, as I said before, I would very much welcome a better solution. It does have to be fully worked out so that we can judge whether it actually works or not. So please provide a replacement Data.Typeable that is better than the current one.

Furthermore, Data.OldTypeable should be removed. It's more harmful to provide a second incompatible Typeable class.

This was provided as a quick way to make old code compile by simply changing the import. It's marked as deprecated, and while it may not be entirely useful because OldTypeable and (New)Typeable instances are distinct, it can still make some old code work. I don't see it as harmful, but, again, I'm willing to be convinced otherwise.

comment:9 in reply to: ↑ 5 Changed 19 months ago by maeder

I quick test of adding a Typeable2 class ended with:

libraries/base/Data/Typeable/TypeableN.hs:1:1:
    Typeable instances can only be derived; replace the following instance:
      instance [overlap ok] Typeable2 a => Typeable a
        -- Defined at libraries/base/Data/Typeable/TypeableN.hs:32:10

So, at least, we do not need to worry about hand-written instances.

comment:10 Changed 19 months ago by maeder

http://hackage.haskell.org/package/uni-graphs-2.2.1.0/docs/Graphs-GraphDisp.html contains an example where replacing Typeable1 by Typeable was not enough, because Typeable1 used to fix the kind. I had to add

+{-# LANGUAGE KindSignatures #-}

and replace the empty class as follows:

-class Typeable1 nodeType => NodeTypeClass nodeType
+class Typeable (nodeType :: * -> *) => NodeTypeClass nodeType

So it would make sense to further supply Typeable1, Typeable2, etc. classes with matching kinds.

Is there a way to circumvent the above "instances can only be derived" error? Is there a different way to supply TypeableN classes that imply/derive Typeable instances?

comment:11 Changed 19 months ago by rwbarton

Well, I think this is a bit silly—you should just write the kind signature and forget that Typeable1 ever existed.

However, this does appear to work (with ConstraintKinds):

type Typeable1 (t :: * -> *) = Typeable t
class Typeable1 t => C t
-- *Main> :k C
-- C :: (* -> *) -> Constraint

so I suppose Data.Typeable could export such type synonyms Typeable1, etc.

comment:12 Changed 19 months ago by maeder

I'm confused. Can _class_ synonyms be introduced using the keyword "type"?

comment:13 Changed 19 months ago by carter

@maeder

1) this ticket is closed

2) if you're having trouble understanding how to work around this, please email haskell-cafe with questions, or ask for help on #haskell on freenode

3) you can use CPP + Reid's suggestion of constraint kinds (http://blog.omega-prime.co.uk/?p=127 is a blog post on them) to emulate having Typeable1 ... N conditionally defined as non polykinded alternatives using constraint kinds. This seems to be more work than just fixing the code mind you :)

4) if you're confused about constraint kinds, ask on haskell cafe mailing list or on #haskell on freenode

comment:14 Changed 19 months ago by maeder

  • Resolution wontfix deleted
  • Status changed from closed to new

If you have such a simple workaround based on ConstraintKinds why do you break backwards-compatibility nilly-willy at let me waste my time by asking for a patch?

comment:15 follow-up: Changed 19 months ago by simonpj

Guys, lets all be polite!

Christian, yes indeed one of the primary motivations for -XConstraintKinds was to allow "type" synonyms to include class constraints (and indeed all sorts of constraints).

Pedro said "I couldn't find an acceptable solution. But, as I said before, I would very much welcome a better solution." Reid suggested one, perhaps one that Pedro thought of and discarded, perhaps not. It has the advantage that it might be a simple change to Data.Typeable making (deprecated) exports of Typeable1, Typeable2 etc.

Pedro: can you think of any problems with this? Thanks Reid.

If someone would like to try it out and send a patch the validates, it might just make it into 7.8.

Simon

Changed 19 months ago by maeder

proof of concept

Changed 19 months ago by maeder

comment:16 Changed 19 months ago by maeder

I've only added Typeable1 and Typeable2 and did not try to reduce the language extensions required in the attached modules Data.Typeable and Data.Typeable.TypeableN.

I even think, these synonyms should not be deprecated since they help useres to avoid kind signatures!

comment:17 Changed 19 months ago by maeder

These constraint kinds will probably not help for i.e.

deriving instance Typeable2 Gr

Sigh.

comment:18 Changed 19 months ago by kosmikus

I think that even if TypeableN would be added as type synonyms using -XConstraintKinds, they should be deprecated. They're ugly and don't add anything except (a limited amount of) backwards compatibility. Adding a kind-signature in a case that'd otherwise be "too" polymorphic is much clearer IMHO.

comment:19 Changed 19 months ago by rwbarton

Christian: actually that does work.

That seems like a decent argument to include these type synonyms for backwards-compatibility purposes. (As kosmikus notes, they're still not 100% backwards compatible. For example, Typeable still doesn't fix the kind of its argument, which could matter in a module with PolyKinds; and since TypeableN is now a type synonym it can't be the argument to another type constructor (with ConstraintKinds). But those are both fairly obscure cases.)

I fully agree though that the type synonyms should be marked as deprecated. TypeableN is a hack that served its purpose; now we can express our programs better using orthogonal features (polykinded types and kind signatures).

comment:20 Changed 19 months ago by maeder

Ok, deprecate them. When they are finally gone users could add such synonyms later themselves to easily port their old code. (The release notes should refer to solutions using KindSignatures and ConstraintKinds.)

comment:21 Changed 19 months ago by maeder

The new module Data.Typeable.TypeableN must also be added to base.cabal, as I just realised.

comment:22 Changed 19 months ago by rwbarton

I would suggest just adding them to Data.Typeable; wouldn't that allow most uses of Data.Typeable to remain unchanged in 7.8?

comment:23 Changed 19 months ago by maeder

I don't mind. I just thought it is easier to simply remove the import (and export entries) later on, but ignored the burden of adding and removing files.

comment:24 follow-up: Changed 19 months ago by simonpj

Pedro, any comments about this?

Simon

comment:25 in reply to: ↑ 24 Changed 19 months ago by dreixel

Yes, I'm working on it, will comment later today.

comment:26 Changed 19 months ago by dreixel

  • Owner set to dreixel

comment:27 follow-up: Changed 19 months ago by Jose Pedro Magalhaes <jpm@…>

In a5383f7400d05a758094578da28d958fb694b726/base:

Provide Typeable1..7 as type synonyms (see #8813)

comment:28 in reply to: ↑ 15 Changed 19 months ago by dreixel

  • Status changed from new to merge

Replying to simonpj:

Pedro said "I couldn't find an acceptable solution. But, as I said before, I would very much welcome a better solution." Reid suggested one, perhaps one that Pedro thought of and discarded, perhaps not. It has the advantage that it might be a simple change to Data.Typeable making (deprecated) exports of Typeable1, Typeable2 etc.

Pedro: can you think of any problems with this? Thanks Reid.

No, I hadn't thought of this before, and no, I don't see any problems with it. As has been pointed out already, it won't work for all old code. But it might still make some more code work, and I don't see a problem with including it. So I submitted a patch to base adding those. In particular, the original problem code described in this ticket should now compile (along with a deprecation warning).

I've also updated the wiki page at https://ghc.haskell.org/trac/ghc/wiki/GhcKinds/PolyTypeable to refer to 7.8 as the present, not the future, and I've added a section on how to make your code compile again. I've also added a link to this wiki page from the Data.Typeable haddock.

comment:29 follow-up: Changed 19 months ago by simonpj

Pedro: Fantastic, thank you. Did you run validate?

Austin: please merge to the stable branch.

Reid: thank you for the idea!

Christian: I hope this makes things better for you.

Simon

comment:30 in reply to: ↑ 29 Changed 19 months ago by dreixel

Replying to simonpj:

Pedro: Fantastic, thank you. Did you run validate?

Yes, it passed.

comment:31 Changed 19 months ago by maeder

Thanks, guys, I hope this makes things better also for other (less language extensions aware) users.

Christian

comment:32 Changed 19 months ago by maeder

looking at Data/Typeable/Internal.hs in the above patch I see that there are two blocks of language extension pragmas, before and after the haddock header, which is a bit confusing as I expected ConstraintKinds to be near KindSignatures.

comment:33 Changed 19 months ago by Jose Pedro Magalhaes <jpm@…>

In eea1b6f5fe254b249acc618ef5a82c3e52a27f8c/base:

Language extension wibble (see #8813)

comment:34 Changed 19 months ago by dreixel

Oh, yes. Fixed, thanks.

comment:35 Changed 19 months ago by maeder

Thanks and sorry again, but should not all language pragmas be on top of the file (before the haddock header)?

comment:36 Changed 19 months ago by dreixel

I'm not sure why the Unsafe is separate. I guess that was added during some SafeHaskell sweep. I'd prefer if they are all together, yes, but that should probably be done in one go to all the modules in base, at least (I suspect Data.Typeable.Internal might not be the only module with this problem...)

comment:37 Changed 19 months ago by maeder

Unsafe being separate at the top of the file actually suggests that all language pragmas should be on top of the file. (But that's a subject of a style guide.)

comment:38 Changed 19 months ago by dreixel

Yes. That should be a separate ticket, though.

comment:39 in reply to: ↑ 27 Changed 19 months ago by hvr

  • Resolution set to fixed
  • Status changed from merge to closed

Replying to Jose Pedro Magalhaes <jpm@…>:

In a5383f7400d05a758094578da28d958fb694b726/base:

Provide Typeable1..7 as type synonyms (see #8813)

This has been merged in the ghc-7.8 branch via [84aad16f054fe8c7b/base]

Is there anything left to be done for the release notes?

comment:40 Changed 19 months ago by hvr

  • Component changed from Compiler to libraries/base
  • Milestone set to 7.8.1

comment:41 Changed 18 months ago by George

It seems this fix might allow quickspec to compile unmodified in RC2. In RC1 it fails with

[ 3 of 20] Compiling Test.QuickSpec.Utils.Typeable ( src/Test/QuickSpec/Utils/Typeable.hs, dist/build/Test/QuickSpec/Utils/Typeable.o )

src/Test/QuickSpec/Utils/Typeable.hs:9:59:
    Not in scope: type constructor or class ‛T.Typeable1’
    Perhaps you meant ‛T.Typeable’ (imported from Data.Typeable)

src/Test/QuickSpec/Utils/Typeable.hs:9:72:
    Not in scope: type constructor or class ‛T.Typeable2’
    Perhaps you meant ‛T.Typeable’ (imported from Data.Typeable)
Failed to install quickspec-0.9.2

Just thought you might want to know of examples where this fix is indeed helpful

Note: See TracTickets for help on using tickets.