#11135: Migrate more of Data.Functor.* from transformers to base. -------------------------------------+------------------------------------- Reporter: hvr | Owner: strake888 Type: task | Status: patch Priority: highest | Milestone: 8.0.1 Component: libraries/base | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1543 Wiki Page: | Phab:D1544 -------------------------------------+------------------------------------- Changes (by RyanGlScott): * status: new => patch * differential: => Phab:D1543 Phab:D1544 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11135#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11135: Migrate more of Data.Functor.* from transformers to base. -------------------------------------+------------------------------------- Reporter: hvr | Owner: Type: task | Status: upstream Priority: highest | Milestone: 8.0.1 Component: libraries/base | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1543 Wiki Page: | Phab:D1544 -------------------------------------+------------------------------------- Changes (by bgamari): * status: new => upstream -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11135#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11135: Migrate more of Data.Functor.* from transformers to base. -------------------------------------+------------------------------------- Reporter: hvr | Owner: Type: task | Status: upstream Priority: highest | Milestone: 8.0.1 Component: libraries/base | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1543 Wiki Page: | Phab:D1544 -------------------------------------+------------------------------------- Comment (by strake888): Sorry about this, I thought it would be a simple operation to patch transformers before the RC. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11135#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11135: Migrate more of Data.Functor.* from transformers to base. -------------------------------------+------------------------------------- Reporter: hvr | Owner: Type: task | Status: upstream Priority: highest | Milestone: 8.0.1 Component: libraries/base | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1543 Wiki Page: | Phab:D1544 -------------------------------------+------------------------------------- Comment (by RyanGlScott): Ross: I would like to see this change get into GHC 8.0, time permitting. There's one thing that bothers me about the most recent changes to `transformers`, however. After [http://hub.darcs.net/ross/transformers/patch/55d242cc15cb91c32a28433eff1e928... introducing] `Show1`/`Show2`/et al, you changed the `Show` instances to use them. For instance, the `Show` instance for `Backwards` [http://hackage.haskell.org/package/transformers-0.4.3.0/docs/src/Control- Applicative-Backwards.html#line-49 was]: {{{#!hs instance (Show1 f, Show a) => Show (Backwards f a) where showsPrec d (Backwards x) = showsUnary1 "Backwards" d x }}} But [http://hub.darcs.net/ross/transformers/browse/Control/Applicative/Backwards.... now] it's: {{{#!hs instance (Show1 f, Show a) => Show (Backwards f a) where showsPrec = showsPrec1 }}} The result is that the output of `show` changes. Before, the output of `show (Backwards "hello")` would be `"Backwards \"hello\""`, but with the latest changes, it's `"Backwards ['h','e','l','l','o']"`! This is because the `Show1 []` instance has to be parametric over its argument, so it can't tell that `Char` has a special `Show` instance that causes it to be shown differently. I feel like we should revert all the `Eq`/`Ord`/`Read`/`Show` instance in `transformers` back to the way they were (and keep the `Show1`/`Show2`/etc. instances separate) to prevent subtle changes like this. Do you agree? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11135#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11135: Migrate more of Data.Functor.* from transformers to base. -------------------------------------+------------------------------------- Reporter: hvr | Owner: Type: task | Status: upstream Priority: highest | Milestone: 8.0.1 Component: libraries/base | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1543 Wiki Page: | Phab:D1544 -------------------------------------+------------------------------------- Comment (by mfdyck.google): I have a patch, now enclosed, to move these to legacy in transformers, but some difficulty deriving Data and Typeable instances so far. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11135#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

Ross: I would like to see this change get into GHC 8.0, time permitting.

There's one thing that bothers me about the most recent changes to `transformers`, however. After [http://hub.darcs.net/ross/transformers/patch/55d242cc15cb91c32a28433eff1e928... introducing] `Show1`/`Show2`/et al, you changed the `Show` instances to use them. For instance, the `Show` instance for `Backwards` [http://hackage.haskell.org/package/transformers-0.4.3.0/docs/src/Control- Applicative-Backwards.html#line-49 was]:

{{{#!hs instance (Show1 f, Show a) => Show (Backwards f a) where showsPrec d (Backwards x) = showsUnary1 "Backwards" d x }}}

But [http://hub.darcs.net/ross/transformers/browse/Control/Applicative/Backwards.... now] it's:

{{{#!hs instance (Show1 f, Show a) => Show (Backwards f a) where showsPrec = showsPrec1 }}}

The result is that the output of `show` changes. Before, the output of `show (Backwards "hello")` would be `"Backwards \"hello\""`, but with the latest changes, it's `"Backwards ['h','e','l','l','o']"`! This is because

#11135: Migrate more of Data.Functor.* from transformers to base. -------------------------------------+------------------------------------- Reporter: hvr | Owner: Type: task | Status: upstream Priority: highest | Milestone: 8.0.1 Component: libraries/base | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1543 Wiki Page: | Phab:D1544 -------------------------------------+------------------------------------- Comment (by ross): Replying to [comment:8 RyanGlScott]: the `Show1 []` instance has to be parametric over its argument, so it can't tell that `Char` has a special `Show` instance that causes it to be shown differently.

I feel like we should revert all the `Eq`/`Ord`/`Read`/`Show` instance

in `transformers` back to the way they were (and keep the `Show1`/`Show2`/etc. instances separate) to prevent subtle changes like this. Do you agree? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11135#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

Ryan: I'm really very negative on the idea of reverting to the old approach.

We rather deliberately changed them in HEAD months ago to make it so

#11135: Migrate more of Data.Functor.* from transformers to base. -------------------------------------+------------------------------------- Reporter: hvr | Owner: Type: task | Status: upstream Priority: highest | Milestone: 8.0.1 Component: libraries/base | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1543 Wiki Page: | Phab:D1544 -------------------------------------+------------------------------------- Comment (by ross): Replying to [comment:11 ekmett]: that they can be used in more situations without requiring the `Functor` constraint a few months back, the old approach had rendered these classes useless for a lot of real world situations around GADTs and the like, in a way that simultaneously made implementations inefficient and less often applicable.

The old transformers approach winds up requiring

{{{#!hs instance (Functor f, Eq1 f, Eq1 g) => Eq1 (Compose f g) }}}

while the current approach lets you use

{{{#!hs instance (Eq1 f, Eq1 g) => Eq1 (Compose f g) }}}

and the code works in one pass regardless of whether or not f and g are

known.

The price of admission is a manual implementation. The current

compromise Ross has implemented gets a design that is readily implemented and implementable for a wide arrange of things.

Now, we could modify the way we pass in the "manual dictionary" version

of 'Show/Read' to allow the `showList`/`readList` trick to work. It'd admittedly make the classes harder to implement and use. I could get behind an alternative API that passed in more stuff to `readsPrecWith` and `showsPrecWith`, or one that added a second combinator so that most people didn't have to deal with the noise.

I'd be okay with either proceeding with the status quo, or even more

comfortable proceeding with a fix that modifies the methods in the `Show1` and `Read1` classes to pass more information, but it'd take a lot of convincing to bring me around to the old version in transformers, especially when we very consciously changed to get away from it! Yes, I'd come to the same conclusion, and have already made the necessary changes for `Show1`, and am now working through `Read1`. Note that the version before the changes also gave the wrong output on `show (Compose (Just "abc"))`. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11135#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11135: Migrate more of Data.Functor.* from transformers to base. -------------------------------------+------------------------------------- Reporter: hvr | Owner: Type: task | Status: upstream Priority: highest | Milestone: 8.0.1 Component: libraries/base | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1543 Wiki Page: | Phab:D1544 Phab:D1630 -------------------------------------+------------------------------------- Comment (by ross): I believe I've fixed the Show1/Read1 issue. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11135#comment:14 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11135: Migrate more of Data.Functor.* from transformers to base. -------------------------------------+------------------------------------- Reporter: hvr | Owner: Type: task | Status: upstream Priority: highest | Milestone: 8.0.1 Component: libraries/base | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1543 Wiki Page: | Phab:D1544 Phab:D1630 -------------------------------------+------------------------------------- Comment (by Herbert Valerio Riedel ): In [changeset:"e0e03d5b9d5cd678f6402534451964d491f16540/ghc" e0e03d5b/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="e0e03d5b9d5cd678f6402534451964d491f16540" Move Data.Functor.(Classes,Compose,Product,Sum) into base These modules were previously provided by the `transformers` package. Hence the submodule update. This patch was originally contributed by M Farkas-Dyck and subsequently taken over and completed by Ryan. The original proposal discussion can be found at https://mail.haskell.org/pipermail/libraries/2015-July/026014.html This addresses #11135 Differential Revision: https://phabricator.haskell.org/D1543 }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11135#comment:16 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11135: Migrate more of Data.Functor.* from transformers to base. -------------------------------------+------------------------------------- Reporter: hvr | Owner: Type: task | Status: upstream Priority: highest | Milestone: 8.0.1 Component: libraries/base | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1543 Wiki Page: | Phab:D1544 Phab:D1630 -------------------------------------+------------------------------------- Comment (by ekmett): @duairc: The benefit of having `Eq1`, `Ord1`, etc. in the first place is that the resulting instances are fully Haskell 98 and do not require language extensions at all. {{{#!hs instance Eq (f (g a)) => Eq (Compose f g a) }}} requires `FlexibleContexts`. This means that `transformers` which works on *every* compiler since Haskell 98 _without any CPP_ simply could not accept instances of this form. Even allowing for that, it is beneficial to allow things like {{{#!hs instance (Eq1 f, Eq1 g) => Eq1 (Compose f g) }}} because instances like that are more useful in the presence of polymorphic recursion than the at-first-glance simpler pointwise `Eq` instance, because it says something fundamental about how the instances are defined: that it isn't able to do something hinky with the argument type, and do something fundamentally different for `Compose Foo [] Int` vs. `Compose Foo [] Double`, that both have to use the `Eq` or `Ord` or `Read` or `Show` instance for 'Int' or `Double` in a homogeneous way, not separate `FlexibleInstances`. This is confidence you do not get from the pointwise definitions. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11135#comment:18 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11135: Migrate more of Data.Functor.* from transformers to base. -------------------------------------+------------------------------------- Reporter: hvr | Owner: Type: task | Status: upstream Priority: highest | Milestone: 8.0.1 Component: libraries/base | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1543 Wiki Page: | Phab:D1544 Phab:D1630 -------------------------------------+------------------------------------- Comment (by ekmett): Up until now we've avoided adding instances that would require extensions of that sort to base as they aren't suitable for describing in a language report without extending the language, and we lack a suitable precedent. Ultimately, we _had_ to move the classes from `Data.Functor.Classes` when we moved the other `Data.Functor.*` data types or we'd have wound up with orphan instances things that were previously perfectly sound and that are already seeing use. In the original Data.Functor migration proposal the inclusion of Data.Functor.Classes was offered up as a reluctant way to avoid random bikeshedding and loss of functionality during the migration of data types that truly belong further up the hierarchy, where we can avoid duplication of them and permit nicer extensions. On the other hand, we don't exactly have to go fishing for 3 dozen new classes to describe every other class 'one argument up'. There are viable solutions for folks who don't care about Haskell 98 (so long as they also don't care about the `Functor` concerns brought up above.) E.g. in my `constraints` package, using `ConstraintKinds` and `PolyKinds`, I supply {{{#!hs data Dict p where Dict :: p => Dict p newtype p :- q = Sub (p => Dict q) class Lifting p t where lifting :: p a :- p (t a) }}} Then everything from `Lifting Eq`, `Lifting Monad`, to `Lifting Monoid` all "just work". However, while more universal they aren't as effective as the manual dictionary passing done here. Such a pattern rules out some nice usecases, while simultaneously requiring a ridiculous number of instances to be defined by all users and a fair bit of sophistication to use. Consequently, I'm very much not advocating for something like that more radical approach. As for `Semigroup (Compose f g a)` -- we have an `Applicative (Compose f g)` because there is a canonical construction for nesting them. However, the choice of `Semigroup` here is quite open. You can't reason usefully about the instance you propose except instance by instance and there are multiple viable candidates. Let's consider `Monoid`: Given the existence of both: {{{#!hs instance (Applicative f, Applicative g, Monoid a) => Monoid (Compose f g a) instance Applicative (f (g a)) => Monoid (Compose f g a) }}} and the lack of a compelling motivation to pick one over the other, while one forces us to incur a flexible context, and the other requires a needless unit in the applicative structure when weakened to give the `Semigroup` constraint, I'd say it is better to supply neither instance. We've been meaning towards supplying instances when they are unambiguous and non-controversial. I'd say that such an instance for `Semigroup (Compose f g a)` would pass neither test. For right now I see the migration of the `Data.Functor.Classes` from `transformers` to `base` as more of a necessary evil than as a pattern to emulate, and trying to hit a decent compromise point, and I'd rather see if they see any sort of meaningful adoption outside of the `transformers` class hierarchy before we go and double/triple down on this design immediately. That said, as `Monoid` (now being in `Prelude`) and `Semigroup` in a few releases become more ingrained in our culture the idea of a `Monoid1` and `Semigroup1` to extend the consistency of this module might take better hold, but deciding that we want to do so probably belongs in the context of a much broader discussion in the libraries@ mailing list, rather than a rapidfire amendment in the last days before a release. E.g. is `Monoid1` sufficiently distinguished from `Applicative` to be worth defining? After all, you can likely prove that you need full parametricity in the argument, which would enable you to define an `Applicative` structure instead, meaning it likely doesn't add any expressive power, which would morally place `Semigroup1` at the same level as semi-applicative / `Apply` in the class hierarchy -- with nearly equivalent parametricity requirements. I personally find such structures useful, but I don't think there is community will to support a finer- grained AMP at this time. '''tl;dr''' My answer to your two questions could best be summaried as 1.) Insofar as it prevents people from writing portable code if they want to and would break such code that already exists, then I think the answer is yes. 2.) Even if you don't buy my reasoning in (1), unilaterally adding such an instance would rule out a potentially much more interesting discussion that should be had on libraries@ that I at least personally think would be more valuable. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11135#comment:20 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11135: Migrate more of Data.Functor.* from transformers to base. -------------------------------------+------------------------------------- Reporter: hvr | Owner: Type: task | Status: upstream Priority: highest | Milestone: 8.0.1 Component: libraries/base | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1543 Wiki Page: | Phab:D1544 Phab:D1630 -------------------------------------+------------------------------------- Comment (by Ben Gamari ): In [changeset:"edcf17bd2ae503c2dda43ded40dca0950edfd018/ghc" edcf17b/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="edcf17bd2ae503c2dda43ded40dca0950edfd018" Move Const to own module in Data.Functor.Const and enable PolyKinds `Const` from `Control.Applicative` can trivially be made kind-polymorphic in its second argument. There has been a Trac issue about this for nearly a year now. It doesn't look like anybody objects to it, so I figured I might as well make a patch. Trac Issues: #10039, #10865, #11135 Differential Revision: https://phabricator.haskell.org/D1630 Reviewers: ekmett, hvr, bgamari Subscribers: RyanGlScott, thomie }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11135#comment:22 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11135: Migrate more of Data.Functor.* from transformers to base. -------------------------------------+------------------------------------- Reporter: hvr | Owner: Type: task | Status: closed Priority: highest | Milestone: 8.0.1 Component: libraries/base | Version: 7.10.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1543 Wiki Page: | Phab:D1544 Phab:D1630 -------------------------------------+------------------------------------- Comment (by sboo): Could we add this? {{{#!hs import System.Mem.StableName instance Eq1 StableName where liftEq _ = eqStableName }}} I didn't see it in https://phabricator.haskell.org/D1543 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11135#comment:24 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler