Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#9664 closed task (fixed)

Add identity functor to base

Reported by: hvr Owned by: ekmett
Priority: normal Milestone: 7.10.1
Component: Core Libraries Version:
Keywords: report-impact Cc: simonpj, ross, ekmett, core-libraries-committee@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D313
Wiki Page:

Description

base already provides a Constant functor, but the even more useful Identity (or Identity) functor does not exist in base yet.

This was already proposed to the CLC back in early 2013, and already missed inclusion in GHC 7.8

However, in order to implement this, coordination with transformers is desirable, as transformers currently provides a non-transforming Data.Functor.Identity, and we'd like to avoid name clashes.

Change History (14)

comment:1 Changed 5 years ago by ross

I suggest doing a major release of transformers at the same time as the next major GHC release, with Data.Functor.Identity removed and added to base. This version would depend on the new version of base to force an upgrade.

comment:2 in reply to:  1 Changed 5 years ago by hvr

Replying to ross:

I suggest doing a major release of transformers at the same time as the next major GHC release, with Data.Functor.Identity removed and added to base. This version would depend on the new version of base to force an upgrade.

Terrific!

I'll draft a transformers-patch in our locally forked repo over at http://git.haskell.org/packages/transformers.git and migrate Data.Functor.Identity into GHC HEAD's base so we can start using that in GHC HEAD.

comment:3 Changed 5 years ago by hvr

Differential Rev(s): Phab:D313
Status: newpatch

@ross: please take a look at http://git.haskell.org/packages/transformers.git/commitdiff/refs/heads/wip/T9664 ... does that look ok to you?

I had to conditionally expose the Data.Functor.Identity module, as otherwise it gets more complicated to tweak GHC's build-system to cope with a transformers that would only work with GHC>=7.9 & base>=4.8, as the in-tree transformers packages needs to be compiled multiple times, once with the bootstrapping compiler (which is either GHC 7.6 or GHC 7.8 for GHC HEAD), and later-on on with GHC HEAD.

comment:4 Changed 5 years ago by hvr

Cc: core-libraries-committee@… added

@ross, any comments?

comment:5 Changed 5 years ago by ross

Changing to have Data.Functor.Class import Data.Functor.Identity reduces the ifdefs. Ideally I'd like to avoid a release with conditional exports - is there any way to detect a GHC bootstrap build?

comment:6 in reply to:  5 Changed 5 years ago by hvr

Replying to ross:

Changing to have Data.Functor.Class import Data.Functor.Identity reduces the ifdefs.

Sure, I can easily change that (at the cost of some minor code duplication)

Ideally I'd like to avoid a release with conditional exports - is there any way to detect a GHC bootstrap build?

As for this I'm not sure how that helps here. Assuming we have a way of setting a Cabal flag when we're bootstrapping GHC (i.e. building stage 1), how would this improve the situation of having the Data.Functor.Identity module conditionally exposed (which now would be depending on that Cabal flag setting rather than predicating on impl(ghc>=7.9))?

comment:7 Changed 5 years ago by hvr

@ross, any comments to my last comment?

comment:8 Changed 5 years ago by hvr

@ross,

I noticed you already reversed the Identity<->Class import-dependency in recent transformers commmits

I've adapted

http://git.haskell.org/packages/transformers.git/commitdiff/refs/heads/wip/T9664

which is now a rather minimal change to transformers (i.e. moving the Identity.hs file into a different source folder, and adapting .cabal)

You write you "like to avoid a release with conditional exports". What problems do you anticipate that could occur by making use of such a conditional (non)export of Data.Functor.Identity?

Specifically, I used impl(ghc>=7.9) instead of Cabal flags in order to avoid any Cabal-related problems I could think of.

comment:9 Changed 5 years ago by ross

I've uploaded a new version of transformers incorporating this patch (and some others).

comment:10 Changed 5 years ago by Herbert Valerio Riedel <hvr@…>

In 87101364e0c2db5e472c6331ad35503028b2ec3c/ghc:

Move Data.Functor.Identity from transformers to base

This also updates the `transformers` submodule to the just
released `transformers-0.4.2.0` package version.

See #9664 for more details

Reviewed By: austin, ekmett

Differential Revision: https://phabricator.haskell.org/D313

comment:11 Changed 5 years ago by thomie

Resolution: fixed
Status: patchclosed

comment:12 Changed 5 years ago by Herbert Valerio Riedel <hvr@…>

In 8cbd25a49051171da7c73db57ebd87bb0296c2f7/ghc:

Make Data.Functor.Identity trustworthy again

Alas `{-# LANGUAGE Safe #-}` can't be used since `Data.Coerce` isn't "safe".
However, we use `coerce` just as an optimisation
(see also 4ba884bdd3a9521ea92fcda8f601a7d0f3537bc1 which broke the
safe-inferred status of `Data.Functor.Identity`), so this module at least
deserves `{-# LANGUAGE Trustworthy #-}`.

NOTE: `Data.Functor.Identity` was added to `base` in the context of #9664

Reviewed By: luite

Differential Revision: https://phabricator.haskell.org/D507

comment:13 Changed 5 years ago by hvr

Keywords: report-impact added

comment:14 Changed 5 years ago by Herbert Valerio Riedel <hvr@…>

In 1f60d635cee1ff3db72e0129f9035b147f52c9c4/ghc:

{Data,Generic(1),MonadZip} instances for Identity

These instances were missed when the identity functor was added to
the `base` package (re #9664).

Reviewed By: ekmett

Differential Revision: https://phabricator.haskell.org/D674
Note: See TracTickets for help on using tickets.