[ghc-steering-committee] [Proposal] #175: Lift.liftTyped (recommendation: accept)

[Eric Seidel]

To recap the discussion so far (thanks Joachim!):

- Ben recommended accepance.
- Richard pointed out that the proposed change will break instances that rely on the default method in a very bad way: they will compile but loop at runtime due to the mutually recursive defaults.
- Iavor replied that it seems unlikely that much code would be affected, as DeriveLift has been the recommended strategy for a long time.

I'm inclined to agree with Iavor that the actual impact is likely to be minimal, but this is something we could investigate with a scan of Hackage. (I believe people have done this in the past, so there may even be some scripts we can repurpose?) 

We could also mitigate the issue in a couple ways:

1. We could make violations of MINIMAL pragmas errors instead of warnings. This would need its own proposal, but it seems like a good idea to me regardless.
2. We could add a special warning/error for empty Lift instances. This is ad-hoc and kludgy, but easily doable since template-haskell is bundled with ghc.
3. We could provide a ghc-fix tool that automatically fixes this issue (and maybe others!). A number of other languages do this and it seems to work well for them. It also sounds like a very nice application of the ghc-exactprint work that Alan has done.

On Wed, Nov 7, 2018, at 12:45, Eric Seidel wrote:
> This is a wonderfully clever idea, but it has the unfortunate 
> consequence of making the new default lift (written in terms of 
> liftTyped) unusable even if you provide a custom liftTyped. I’ve had to 
> write my own lift on occasion (eg for types that contain Text fields), 
> so this is something we should consider. That being said, the 
> consequence of the TypeError fix is that custom instances will need an 
> extra one-line definition of lift, which is not that severe. 
> 
> This whole issue does make me wonder though, why does a violation of the 
> MINIMAL pragma produce a warning rather than an error? It seems like the 
> standard use case is to describe how one can break a chain of mutually-
> recursive default definitions. Failure to do so will result in a non-
> terminating program, which really ought to be an error. 
> 
> Sent from my iPhone
> 
> > On Nov 7, 2018, at 10:40, Richard Eisenberg <rae at cs.brynmawr.edu> wrote:
> > 
> > If we really want to encourage users to use `DeriveLift`, we could do something like
> > 
> > class Lift t where
> >   ...
> >   default lift :: TypeError "Use DeriveLift" => ...
> > 
> > Then, anyone relying on the old behavior would get a beautiful error message telling them exactly how to migrate.
> > 
> > Richard
> > 
> >> On Nov 5, 2018, at 6:31 PM, Iavor Diatchki <iavor.diatchki at gmail.com> wrote:
> >> 
> >> Hello,
> >> 
> >> I'd be surprised if there is a lot of code affected by that.   Either way,  I guess this wouldn't be an issue, if we just changed the proposal to make the default instance for `typedLift` looks like the old default for `lift` (i.e., use the `Data` constraint).
> >> 
> >> The proposed design does have two benefits though:
> >>    * The new design requires fewer GHC extensions (no need for `DefaultSignatures`)
> >>    * It encourages programmers to use `DeriveLift`, which is shorter, safer, and more direct.
> >> 
> >> -Iavor
> >> 
> >>  
> >> 
> >>> On Mon, Nov 5, 2018 at 10:50 AM Richard Eisenberg <rae at cs.brynmawr.edu> wrote:
> >>> Will this break existing code? The proposal suggests that liftTyped will have a suitable default implementation.
> >>> 
> >>> Actually, this will break code, but not in the way one might think (with an undefined liftTyped). Instead, the proposal removes the default implementation of lift to use liftTyped. This means that any code relying on the default implementation of lift will now have an instance with mutually recursive, non-terminating methods. And this will all be silent. And, it's something that might be discovered only in clients of a library, rather than in the library itself. So I think that's pretty problematic. Even with our recommendation to derive Lift instances, this change in behavior is so terrible that it makes me lean against the proposal. Is there a design / migration path that avoids this problem?
> >>> 
> >>> (Yes, yes, I know that I could voiced this opinion earlier, but it didn't occur to me until just now.)
> >>> 
> >>> Richard
> >>> 
> >>> > On Nov 5, 2018, at 1:37 PM, Ben Gamari <ben at well-typed.com> wrote:
> >>> > 
> >>> > Hi everyone,
> >>> > 
> >>> > I have been asked to Shepard #175, which proposes to add a `liftTyped`
> >>> > method to the `Lift` typeclass used to lift Haskell values into Template
> >>> > Haskell splices. Currently the `Lift` typeclass provides no guarantee of
> >>> > type-safety, even when used in a Typed Template Haskell context.
> >>> > 
> >>> > The proposal resolves this limitation by adding a typed lifting
> >>> > operation to the `Lift` class:
> >>> > 
> >>> >    class Lift t where
> >>> >      lift :: t -> Q Exp
> >>> >      liftTyped :: t -> Q (TExp t)
> >>> > 
> >>> > While this addition will break manually-written `Lift` instances, we
> >>> > recommended that users derive such instances for quite a while now, so
> >>> > it is not expected that the breakage will be wide-spread. In light of
> >>> > this, I recommend that we accept this proposal.
> >>> > 
> >>> > Given that we will likely want to get this in to 8.8, I suggest that we
> >>> > limit the discussion to a week unless there is dissent.
> >>> > 
> >>> > Cheers,
> >>> > 
> >>> > - Ben
> >>> > _______________________________________________
> >>> > ghc-steering-committee mailing list
> >>> > ghc-steering-committee at haskell.org
> >>> > https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee
> >>> 
> >>> _______________________________________________
> >>> ghc-steering-committee mailing list
> >>> ghc-steering-committee at haskell.org
> >>> https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee
> > 
> > _______________________________________________
> > ghc-steering-committee mailing list
> > ghc-steering-committee at haskell.org
> > https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee