Added (more) missing instances for Identity and Const
ClosedPublic

Authored by duairc on Apr 2 2016, 1:23 PM.

Details

Summary
  • Identity and Const now have Num, Real, Integral, Fractional, Floating, RealFrac and RealFloat instances
  • Identity and Const now have Bits and FiniteBits instances
  • Identity and Const now have IsString instances

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Branch
master
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 9036
Build 11081: arc lint + arc unit
duairc updated this revision to Diff 7171.Apr 2 2016, 1:23 PM
duairc retitled this revision from to Added (more) missing instances for Identity and Const.
duairc updated this object.
duairc edited the test plan for this revision. (Show Details)
duairc updated this revision to Diff 7175.Apr 3 2016, 6:07 AM
duairc edited edge metadata.

Fixed previous diff where I accidentally enabled DeriveDataTypeable unnecessarily in Data.Functor.Const

RyanGlScott requested changes to this revision.Apr 3 2016, 8:21 PM
RyanGlScott added a reviewer: RyanGlScott.
RyanGlScott added a subscriber: RyanGlScott.

Is there a Trac ticket for this? I personally don't have any objection to adding these instances, but given that this changes base's API (albeit very slightly), it might be best to have a Trac page for it (if nothing else, to notify the CLC).

In any case, there's some other things that need to be addressed:

libraries/base/Data/String.hs
83

Where is the IsString instance for Identity you claimed to add?

libraries/base/changelog.md
144

Is there a reason that this is chunked into three different parts? There are quite a few lines beginning with the phrase "Identity and Const now have"... I think it would be best to lump them together to make it clear they were all added in one go.

This revision now requires changes to proceed.Apr 3 2016, 8:21 PM
hvr added inline comments.Apr 4 2016, 2:01 AM
libraries/base/changelog.md
144

it's also not clear if this has any chance to make it into base-4.9 being so late...

duairc edited edge metadata.Apr 4 2016, 4:06 PM
duairc updated the Trac tickets for this revision.
duairc added inline comments.Apr 4 2016, 4:15 PM
libraries/base/Data/String.hs
83

It's in Data.Functor.Identity, in the deriving section. I would have done the same for Const, but I can't import Data.String from Data.Functor.Const, because Control.Applicative imports Const to re-export it, and Data.String transitively depends on Control.Applicative.

libraries/base/changelog.md
144

There's not really any real reason, I just it was better to group the Num hierarchy and the "Bits hierachy" classes in their own groupings. I can change this so that they're all on one line if you want.

144

That's okay. Should I start a new section 4.10 though? I wasn't sure what the best thing to do would be.

duairc updated this revision to Diff 7186.Apr 4 2016, 4:17 PM
duairc edited edge metadata.

Merged the all "Identity and Const" sections of the changelog.

bgamari edited edge metadata.Apr 7 2016, 5:07 AM
bgamari added subscribers: ekmett, nomeata.

@ekmett and @nomeata, any objection to this from the CLC?

No objections from my side, although Edward has a better grasp of possible consequences and the general design.

ekmett accepted this revision.Apr 10 2016, 7:45 PM
ekmett added a reviewer: ekmett.

LGTM

This revision was automatically updated to reflect the committed changes.