Remove OldTypeable
ClosedPublic

Authored by mgmeier on Oct 4 2014, 6:12 PM.

Details

Summary

Remove OldTypeable, which has been deprecated in 7.8, from base, compiler and testsuite.

Test Plan

Diff validated fine in my tree, but please review carefully, I'm a first-time ghc contributor.
(@dreixel: we met at Haskell hackathon in Berlin, thx again for your code walkthru!)

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mgmeier updated this revision to Diff 783.Oct 4 2014, 6:12 PM
mgmeier retitled this revision from to Remove OldTypeable.
mgmeier updated this object.
mgmeier edited the test plan for this revision. (Show Details)
mgmeier added a reviewer: dreixel.
mgmeier updated the Trac tickets for this revision.
mgmeier added a subscriber: dreixel.
mgmeier set the repository for this revision to rGHC Glasgow Haskell Compiler.Oct 4 2014, 6:39 PM
hvr added a reviewer: ekmett.EditedOct 5 2014, 4:22 AM
hvr added a subscriber: hvr.

adding Edward, as iirc he may want to keep it around for yet another GHC cycle if it doesn't cause actual pain

(I talked to him because I was about to remove OldTypeable myself already a few weeks ago...)

hvr added a comment.Oct 5 2014, 4:24 AM

Fwiw, if this is going to be done for GHC 7.10.1, I'm missing a libraries/base/changelog.md entry

mgmeier updated this revision to Diff 785.Oct 5 2014, 8:12 AM
mgmeier updated this object.
  • remove OldTypeable (fixes Trac #9639)
  • add changelog entry

Ok, done, thx for pointing that out.
Is there a discussion thread (on removing OldTypeable now vs. later) to follow somewhere? (have not seen that on ghc-devs IIRC)

hvr added a comment.Oct 5 2014, 8:42 AM
In D311#7091, @mgmeier wrote:

Is there a discussion thread (on removing OldTypeable now vs. later) to follow somewhere? (have not seen that on ghc-devs IIRC)

No, that short passing-by "oh and btw, can we remove OldTypeable" conversation occurred off-list, so there's no public record of it... But I'm sure @ekmett will soon express his current opinion here :-)

mgmeier updated this revision to Diff 787.Oct 5 2014, 11:10 AM
mgmeier edited edge metadata.
  • remove deprecated types Typeable1 .. Typeable7 from base as well
hvr requested changes to this revision.Oct 8 2014, 4:45 PM
hvr added a reviewer: hvr.
hvr added inline comments.
compiler/prelude/PrelNames.lhs
1282–1283

I see no reason to keep this around as comment; we have a CVS for that... :-)

libraries/base/changelog.md
84

it should be mentioned that Data.Typeable.Typeable{1..7} are gone as well

This revision now requires changes to proceed.Oct 8 2014, 4:45 PM
mgmeier updated this revision to Diff 834.Oct 9 2014, 7:49 AM
mgmeier edited edge metadata.
  • add changes requested by @hvr
hvr accepted this revision.Oct 9 2014, 7:55 AM
hvr edited edge metadata.

LGTM

ekmett edited edge metadata.Oct 11 2014, 1:40 PM

I'm 100% behind removing OldTypeable at this stage.

However, it appears to me that removing the TypeableN aliases will break a lot more existing code than just removing OldTypeable.

I hadn't realized that was included in the desired scope.

On one hand they've gone through a deprecation cycle. On the other they are still much more widely used.

I look in just my own codebases and I spot almost 100 uses of them, and they are almost all in places where for support reasons I _cannot_ remove them yet. This means that this change would result in the placement of dozens of little local transient definitions like:

#if defined(__GLASGOW_HASKELL__) && __GLASGOW_HASKELL__ >= 709
{-# LANGUAGE ConstraintKinds #-}
#endif
...
#if defined(__GLASGOW_HASKELL__) && __GLASGOW_HASKELL__ >= 709
type Typeable2 = Typeable
#endif

This would create a large burden on anyone who has heavily used Typeable for types that include higher-kinded arguments, and who needs to support versions of GHC prior to 7.8.x, whereas simply deferring that particular removal to a later date greatly reduces the scope of that impact.

dreixel requested changes to this revision.Oct 11 2014, 1:53 PM
dreixel edited edge metadata.

Yes, I suggest we defer the removal of those classes until a later date.

This revision now requires changes to proceed.Oct 11 2014, 1:53 PM
mgmeier updated this revision to Diff 888.Oct 16 2014, 6:08 PM
mgmeier edited edge metadata.
  • revert deletion of Typeable{1..7}
dreixel accepted this revision.Oct 17 2014, 2:08 AM
dreixel edited edge metadata.

Thanks!

This revision was automatically updated to reflect the committed changes.
hvr added a comment.Oct 18 2014, 9:27 AM

@mgmeier thanks for helping get rid of old bitrotting code :-)