[Proposal] #175: Lift.liftTyped (recommendation: accept)

Ben Gamari

5 Nov 2018 5 Nov '18

1:37 p.m.

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

Attachments:

signature.asc (application/pgp-signature — 487 bytes)

Show replies by date

Richard Eisenberg

5 Nov 5 Nov

1:50 p.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

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 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@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

Eric Seidel

2:51 p.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

On Mon, Nov 5, 2018, at 13:50, Richard Eisenberg 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?

The current iteration of the proposal has a MINIMAL pragma on Lift, specifying that either lift or liftTyped must be explicitly implemented. https://github.com/harpocrates/ghc-proposals/blob/lift-typed/proposals/0000-... So code that currently relies on the default implementation of lift will indeed break, but not silently! The library author will get a warning that they need to provide lift or liftTyped (or, preferably, switch to DeriveLift).

Eric Seidel

3:26 p.m.

Though on second thought, this still doesn’t exactly provide a clean migration path. If I depend on version X of some library that uses the default implementation of lift, and I upgrade to a new GHC with the new Lift class, version X of my dependency will continue to compile, albeit with a warning. But I probably don’t pay much attention to warnings in my dependencies (and almost certainly don’t compile them with -Werror), so I may still be caught off-guard by the change. Sent from my iPhone

...

On Nov 5, 2018, at 12:51, Eric Seidel wrote:

...
On Mon, Nov 5, 2018, at 13:50, Richard Eisenberg 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?

The current iteration of the proposal has a MINIMAL pragma on Lift, specifying that either lift or liftTyped must be explicitly implemented.

https://github.com/harpocrates/ghc-proposals/blob/lift-typed/proposals/0000-...

So code that currently relies on the default implementation of lift will indeed break, but not silently! The library author will get a warning that they need to provide lift or liftTyped (or, preferably, switch to DeriveLift).

Iavor Diatchki

6:31 p.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

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 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 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@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

Richard Eisenberg

7 Nov 7 Nov

11:40 a.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

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 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 mailto:rae@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 mailto:ben@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@haskell.org mailto:ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org mailto:ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

Eric Seidel

12:45 p.m.

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 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 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 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 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@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

Eric Seidel

18 Dec 18 Dec

10:37 p.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

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 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 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 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 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@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

Joachim Breitner

19 Dec 19 Dec

4:17 a.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

Hi, thanks for the summary! Am Dienstag, den 18.12.2018, 22:37 -0500 schrieb Eric Seidel:

...

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.

I think 1. is at odds in GHC – we allow partiality in other places as well (partially initialized records, for example). Disallowing it feels like a too fundamental change to the language for the problem we need to solve here. 3. is a neat idea, but again a very big cannon for a niche problem. If we _had_ ghc-fix, then this might be a good approach, and we can keep this in mind as additional evidence that ghc-fix would be good (GSOC anyone), but let's not wait for that here. 2. is good, I think. We have had plenty of such kludges, e.g. around the Monad refactoring. Cheers, Joachim -- Joachim Breitner mail@joachim-breitner.de http://www.joachim-breitner.de/

Eric Seidel

5:49 a.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

Joachim wrote:

...

I think 1. is at odds in GHC – we allow partiality in other places as well (partially initialized records, for example). Disallowing it feels like a too fundamental change to the language for the problem we need to solve here.

Yes, I agree that changing the semantics of MINIMAL solely to fix this issue is not a great idea. But having a MINIMAL violation be a warning instead of an error generally feels shady to me. The only use of MINIMAL pragmas I've seen is to break mutually recursive definitions. That means there's an important difference between MINIMAL pragmas and other sources of partiality. In a partial record construction, your program will be fine so long as you don't access the uninitialized fields. This is dangerous, so we warn you about it, but it's still possible that your program will be correct. With a MINIMAL violation, it seems like the programmer is telling us that an instance *cannot* be correct unless it provides the minimal definitions. But I shouldn't derail the conversation.. Perhaps I'll write up a separate proposal about MINIMAL.

...

3. is a neat idea, but again a very big cannon for a niche problem. If we _had_ ghc-fix, then this might be a good approach, and we can keep this in mind as additional evidence that ghc-fix would be good (GSOC anyone), but let's not wait for that here.

Indeed, I wouldn't make this wait on ghc-fix either, but it would be a very nice thing to have right now!

...

2. is good, I think. We have had plenty of such kludges, e.g. around the Monad refactoring.

If we're ok with kludges like this to ease the migration, I think it's clearly the simplest solution. --- Simon wrote:

...

Is there any reason for not adopting the second alternative in section 5: make liftTyped the sole method of Lift?

I suspect this would break more code than the current proposal, since you do sometimes have to write a manual `lift` instance (eg if you have a field like Text that doesn't have an instance). Fixing such code would also require CPP, which we usually try to avoid.

Richard Eisenberg

21 Dec 21 Dec

10:04 p.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

I favor Simon's suggestion. Just rip out `lift` from the class and define it as a top-level function. This breaks backward-compat, but in an inescapable way. And TH users are accustomed to breakage. My problem earlier wasn't just that it wasn't backward-compatible, but that it was incompatible in such a terrible way. Simon's suggestion leads to a better incompatibility. Richard

...

On Dec 19, 2018, at 5:49 AM, Eric Seidel wrote:

Joachim wrote:

...
I think 1. is at odds in GHC – we allow partiality in other places as well (partially initialized records, for example). Disallowing it feels like a too fundamental change to the language for the problem we need to solve here.

Yes, I agree that changing the semantics of MINIMAL solely to fix this issue is not a great idea. But having a MINIMAL violation be a warning instead of an error generally feels shady to me.

The only use of MINIMAL pragmas I've seen is to break mutually recursive definitions. That means there's an important difference between MINIMAL pragmas and other sources of partiality. In a partial record construction, your program will be fine so long as you don't access the uninitialized fields. This is dangerous, so we warn you about it, but it's still possible that your program will be correct. With a MINIMAL violation, it seems like the programmer is telling us that an instance *cannot* be correct unless it provides the minimal definitions.

But I shouldn't derail the conversation.. Perhaps I'll write up a separate proposal about MINIMAL.

...
3. is a neat idea, but again a very big cannon for a niche problem. If we _had_ ghc-fix, then this might be a good approach, and we can keep this in mind as additional evidence that ghc-fix would be good (GSOC anyone), but let's not wait for that here.

Indeed, I wouldn't make this wait on ghc-fix either, but it would be a very nice thing to have right now!

...
2. is good, I think. We have had plenty of such kludges, e.g. around the Monad refactoring.

If we're ok with kludges like this to ease the migration, I think it's clearly the simplest solution.

---

Simon wrote:

...
Is there any reason for not adopting the second alternative in section 5: make liftTyped the sole method of Lift?

I suspect this would break more code than the current proposal, since you do sometimes have to write a manual `lift` instance (eg if you have a field like Text that doesn't have an instance). Fixing such code would also require CPP, which we usually try to avoid. _______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

Eric Seidel

19 Jan 19 Jan

11:59 a.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

Let's try to pick this discussion back up. The proposal has been lingering on our end for a couple months now! We all seem to be in agreement that the basic proposal of adding `liftTyped` should be accepted. The discussion has mostly focused on backwards-compatibility concerns around the default implementation of `Lift`, specifically that the new mutually-recursive definitions of `lift` and `liftTyped` could break **clients** of libraries that use TH without even issuing a warning in the libraries themselves. There are two proposed solutions that seem to have some support. 1. I suggested a targeted warning/error about empty `Lift` instances that could be bundled with GHC. TH is bundled with GHC, so this would guarantee that we notify (or break) libraries before client code is affected. It would also let library authors avoid CPP while supporting multiple versions of TH. Joachim expressed his support for this solution. 2. The proposal suggests an alternative that replaces `lift` with `liftTyped` as the sole method of `Lift`. This is probably how we would design the `Lift` class today if we were starting from scratch. On the other hand, it forces library authors to use CPP to support multiple versions of TH, which people often try to avoid (e.g. due to tooling issues). Simon and Richard have expressed support for this solution. I would actually be happy with either solution. Joachim, are you happy with option 2? If so, perhaps we should just agree on it and move forward with the proposal. On Fri, Dec 21, 2018, at 22:04, Richard Eisenberg wrote:

...

I favor Simon's suggestion. Just rip out `lift` from the class and define it as a top-level function. This breaks backward-compat, but in an inescapable way. And TH users are accustomed to breakage. My problem earlier wasn't just that it wasn't backward-compatible, but that it was incompatible in such a terrible way. Simon's suggestion leads to a better incompatibility.

Richard

...
On Dec 19, 2018, at 5:49 AM, Eric Seidel wrote:

Joachim wrote:

...
I think 1. is at odds in GHC – we allow partiality in other places as well (partially initialized records, for example). Disallowing it feels like a too fundamental change to the language for the problem we need to solve here.

Yes, I agree that changing the semantics of MINIMAL solely to fix this issue is not a great idea. But having a MINIMAL violation be a warning instead of an error generally feels shady to me.

The only use of MINIMAL pragmas I've seen is to break mutually recursive definitions. That means there's an important difference between MINIMAL pragmas and other sources of partiality. In a partial record construction, your program will be fine so long as you don't access the uninitialized fields. This is dangerous, so we warn you about it, but it's still possible that your program will be correct. With a MINIMAL violation, it seems like the programmer is telling us that an instance *cannot* be correct unless it provides the minimal definitions.

But I shouldn't derail the conversation.. Perhaps I'll write up a separate proposal about MINIMAL.

...
3. is a neat idea, but again a very big cannon for a niche problem. If we _had_ ghc-fix, then this might be a good approach, and we can keep this in mind as additional evidence that ghc-fix would be good (GSOC anyone), but let's not wait for that here.

Indeed, I wouldn't make this wait on ghc-fix either, but it would be a very nice thing to have right now!

...
2. is good, I think. We have had plenty of such kludges, e.g. around the Monad refactoring.

If we're ok with kludges like this to ease the migration, I think it's clearly the simplest solution.

---

Simon wrote:

...
Is there any reason for not adopting the second alternative in section 5: make liftTyped the sole method of Lift?

I suspect this would break more code than the current proposal, since you do sometimes have to write a manual `lift` instance (eg if you have a field like Text that doesn't have an instance). Fixing such code would also require CPP, which we usually try to avoid. _______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

Joachim Breitner

20 Jan 20 Jan

4:30 a.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

Hi, given that TH has never been the best when it comes to backwards compatibility, and if it is indeed how it should have been in the first place, then yup, fine with me. Cheers, Joachim Am Samstag, den 19.01.2019, 11:59 -0500 schrieb Eric Seidel:

...

Let's try to pick this discussion back up. The proposal has been lingering on our end for a couple months now!

We all seem to be in agreement that the basic proposal of adding `liftTyped` should be accepted. The discussion has mostly focused on backwards-compatibility concerns around the default implementation of `Lift`, specifically that the new mutually-recursive definitions of `lift` and `liftTyped` could break **clients** of libraries that use TH without even issuing a warning in the libraries themselves.

There are two proposed solutions that seem to have some support.

1. I suggested a targeted warning/error about empty `Lift` instances that could be bundled with GHC. TH is bundled with GHC, so this would guarantee that we notify (or break) libraries before client code is affected. It would also let library authors avoid CPP while supporting multiple versions of TH. Joachim expressed his support for this solution.

2. The proposal suggests an alternative that replaces `lift` with `liftTyped` as the sole method of `Lift`. This is probably how we would design the `Lift` class today if we were starting from scratch. On the other hand, it forces library authors to use CPP to support multiple versions of TH, which people often try to avoid (e.g. due to tooling issues). Simon and Richard have expressed support for this solution.

I would actually be happy with either solution.

Joachim, are you happy with option 2? If so, perhaps we should just agree on it and move forward with the proposal.

On Fri, Dec 21, 2018, at 22:04, Richard Eisenberg wrote:

...
I favor Simon's suggestion. Just rip out `lift` from the class and define it as a top-level function. This breaks backward-compat, but in an inescapable way. And TH users are accustomed to breakage. My problem earlier wasn't just that it wasn't backward-compatible, but that it was incompatible in such a terrible way. Simon's suggestion leads to a better incompatibility.

Richard

...
On Dec 19, 2018, at 5:49 AM, Eric Seidel wrote:

Joachim wrote:

...
I think 1. is at odds in GHC – we allow partiality in other places as well (partially initialized records, for example). Disallowing it feels like a too fundamental change to the language for the problem we need to solve here.

Yes, I agree that changing the semantics of MINIMAL solely to fix this issue is not a great idea. But having a MINIMAL violation be a warning instead of an error generally feels shady to me.

The only use of MINIMAL pragmas I've seen is to break mutually recursive definitions. That means there's an important difference between MINIMAL pragmas and other sources of partiality. In a partial record construction, your program will be fine so long as you don't access the uninitialized fields. This is dangerous, so we warn you about it, but it's still possible that your program will be correct. With a MINIMAL violation, it seems like the programmer is telling us that an instance *cannot* be correct unless it provides the minimal definitions.

But I shouldn't derail the conversation.. Perhaps I'll write up a separate proposal about MINIMAL.

...
3. is a neat idea, but again a very big cannon for a niche problem. If we _had_ ghc-fix, then this might be a good approach, and we can keep this in mind as additional evidence that ghc-fix would be good (GSOC anyone), but let's not wait for that here.

Indeed, I wouldn't make this wait on ghc-fix either, but it would be a very nice thing to have right now!

...
2. is good, I think. We have had plenty of such kludges, e.g. around the Monad refactoring.

If we're ok with kludges like this to ease the migration, I think it's clearly the simplest solution.

---

Simon wrote:

...
Is there any reason for not adopting the second alternative in section 5: make liftTyped the sole method of Lift?

I suspect this would break more code than the current proposal, since you do sometimes have to write a manual `lift` instance (eg if you have a field like Text that doesn't have an instance). Fixing such code would also require CPP, which we usually try to avoid. _______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee -- Joachim Breitner mail@joachim-breitner.de http://www.joachim-breitner.de/

Eric Seidel

21 Jan 21 Jan

9:26 a.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

Sounds like we have a consensus then. Ben, do you want to declare this proposal accepted (using the alternative of `liftTyped` as the sole method of `Lift`)? On Sun, Jan 20, 2019, at 04:30, Joachim Breitner wrote:

...

Hi,

given that TH has never been the best when it comes to backwards compatibility, and if it is indeed how it should have been in the first place, then yup, fine with me.

Cheers, Joachim

Am Samstag, den 19.01.2019, 11:59 -0500 schrieb Eric Seidel:

...
Let's try to pick this discussion back up. The proposal has been lingering on our end for a couple months now!

We all seem to be in agreement that the basic proposal of adding `liftTyped` should be accepted. The discussion has mostly focused on backwards-compatibility concerns around the default implementation of `Lift`, specifically that the new mutually-recursive definitions of `lift` and `liftTyped` could break **clients** of libraries that use TH without even issuing a warning in the libraries themselves.

There are two proposed solutions that seem to have some support.

1. I suggested a targeted warning/error about empty `Lift` instances that could be bundled with GHC. TH is bundled with GHC, so this would guarantee that we notify (or break) libraries before client code is affected. It would also let library authors avoid CPP while supporting multiple versions of TH. Joachim expressed his support for this solution.

2. The proposal suggests an alternative that replaces `lift` with `liftTyped` as the sole method of `Lift`. This is probably how we would design the `Lift` class today if we were starting from scratch. On the other hand, it forces library authors to use CPP to support multiple versions of TH, which people often try to avoid (e.g. due to tooling issues). Simon and Richard have expressed support for this solution.

I would actually be happy with either solution.

Joachim, are you happy with option 2? If so, perhaps we should just agree on it and move forward with the proposal.

On Fri, Dec 21, 2018, at 22:04, Richard Eisenberg wrote:

...
I favor Simon's suggestion. Just rip out `lift` from the class and define it as a top-level function. This breaks backward-compat, but in an inescapable way. And TH users are accustomed to breakage. My problem earlier wasn't just that it wasn't backward-compatible, but that it was incompatible in such a terrible way. Simon's suggestion leads to a better incompatibility.

Richard

...
On Dec 19, 2018, at 5:49 AM, Eric Seidel wrote:

Joachim wrote:

...
I think 1. is at odds in GHC – we allow partiality in other places as well (partially initialized records, for example). Disallowing it feels like a too fundamental change to the language for the problem we need to solve here.

Yes, I agree that changing the semantics of MINIMAL solely to fix this issue is not a great idea. But having a MINIMAL violation be a warning instead of an error generally feels shady to me.

The only use of MINIMAL pragmas I've seen is to break mutually recursive definitions. That means there's an important difference between MINIMAL pragmas and other sources of partiality. In a partial record construction, your program will be fine so long as you don't access the uninitialized fields. This is dangerous, so we warn you about it, but it's still possible that your program will be correct. With a MINIMAL violation, it seems like the programmer is telling us that an instance *cannot* be correct unless it provides the minimal definitions.

But I shouldn't derail the conversation.. Perhaps I'll write up a separate proposal about MINIMAL.

...
3. is a neat idea, but again a very big cannon for a niche problem. If we _had_ ghc-fix, then this might be a good approach, and we can keep this in mind as additional evidence that ghc-fix would be good (GSOC anyone), but let's not wait for that here.

Indeed, I wouldn't make this wait on ghc-fix either, but it would be a very nice thing to have right now!

...
2. is good, I think. We have had plenty of such kludges, e.g. around the Monad refactoring.

If we're ok with kludges like this to ease the migration, I think it's clearly the simplest solution.

---

Simon wrote:

...
Is there any reason for not adopting the second alternative in section 5: make liftTyped the sole method of Lift?

I suspect this would break more code than the current proposal, since you do sometimes have to write a manual `lift` instance (eg if you have a field like Text that doesn't have an instance). Fixing such code would also require CPP, which we usually try to avoid. _______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee -- Joachim Breitner mail@joachim-breitner.de http://www.joachim-breitner.de/

_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee Email had 1 attachment: + signature.asc 1k (application/pgp-signature)

Simon Peyton Jones

30 Jan 30 Jan

8:09 a.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

I'm content with liftTyped. And while (2) is nicer ab-initio, it's not a big deal having two members of the class, so I'm fine with (1). Simon | -----Original Message----- | From: ghc-steering-committee | On Behalf Of Eric Seidel | Sent: 19 January 2019 17:00 | To: ghc-steering-committee@haskell.org | Subject: Re: [ghc-steering-committee] [Proposal] #175: Lift.liftTyped | (recommendation: accept) | | Let's try to pick this discussion back up. The proposal has been | lingering on our end for a couple months now! | | We all seem to be in agreement that the basic proposal of adding | `liftTyped` should be accepted. The discussion has mostly focused on | backwards-compatibility concerns around the default implementation of | `Lift`, specifically that the new mutually-recursive definitions of | `lift` and `liftTyped` could break **clients** of libraries that use TH | without even issuing a warning in the libraries themselves. | | There are two proposed solutions that seem to have some support. | | 1. I suggested a targeted warning/error about empty `Lift` instances that | could be bundled with GHC. TH is bundled with GHC, so this would | guarantee that we notify (or break) libraries before client code is | affected. It would also let library authors avoid CPP while supporting | multiple versions of TH. Joachim expressed his support for this solution. | | 2. The proposal suggests an alternative that replaces `lift` with | `liftTyped` as the sole method of `Lift`. This is probably how we would | design the `Lift` class today if we were starting from scratch. On the | other hand, it forces library authors to use CPP to support multiple | versions of TH, which people often try to avoid (e.g. due to tooling | issues). Simon and Richard have expressed support for this solution. | | I would actually be happy with either solution. | | Joachim, are you happy with option 2? If so, perhaps we should just agree | on it and move forward with the proposal. | | On Fri, Dec 21, 2018, at 22:04, Richard Eisenberg wrote: | > I favor Simon's suggestion. Just rip out `lift` from the class and | > define it as a top-level function. This breaks backward-compat, but in | > an inescapable way. And TH users are accustomed to breakage. My | > problem earlier wasn't just that it wasn't backward-compatible, but | > that it was incompatible in such a terrible way. Simon's suggestion | > leads to a better incompatibility. | > | > Richard | > | > > On Dec 19, 2018, at 5:49 AM, Eric Seidel wrote: | > > | > > Joachim wrote: | > >> I think 1. is at odds in GHC – we allow partiality in other places | > >> as well (partially initialized records, for example). Disallowing | > >> it feels like a too fundamental change to the language for the | > >> problem we need to solve here. | > > | > > Yes, I agree that changing the semantics of MINIMAL solely to fix | this issue is not a great idea. But having a MINIMAL violation be a | warning instead of an error generally feels shady to me. | > > | > > The only use of MINIMAL pragmas I've seen is to break mutually | recursive definitions. That means there's an important difference between | MINIMAL pragmas and other sources of partiality. In a partial record | construction, your program will be fine so long as you don't access the | uninitialized fields. This is dangerous, so we warn you about it, but | it's still possible that your program will be correct. With a MINIMAL | violation, it seems like the programmer is telling us that an instance | *cannot* be correct unless it provides the minimal definitions. | > > | > > But I shouldn't derail the conversation.. Perhaps I'll write up a | separate proposal about MINIMAL. | > > | > >> 3. is a neat idea, but again a very big cannon for a niche problem. | > >> If we _had_ ghc-fix, then this might be a good approach, and we can | > >> keep this in mind as additional evidence that ghc-fix would be good | > >> (GSOC anyone), but let's not wait for that here. | > > | > > Indeed, I wouldn't make this wait on ghc-fix either, but it would be | a very nice thing to have right now! | > > | > >> 2. is good, I think. We have had plenty of such kludges, e.g. | > >> around the Monad refactoring. | > > | > > If we're ok with kludges like this to ease the migration, I think | it's clearly the simplest solution. | > > | > > --- | > > | > > Simon wrote: | > > | > >> Is there any reason for not adopting the second alternative in | section 5: make liftTyped the sole method of Lift? | > > | > > I suspect this would break more code than the current proposal, since | you do sometimes have to write a manual `lift` instance (eg if you have a | field like Text that doesn't have an instance). Fixing such code would | also require CPP, which we usually try to avoid. | > > _______________________________________________ | > > ghc-steering-committee mailing list | > > ghc-steering-committee@haskell.org | > > https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-commi | > > ttee | > | _______________________________________________ | ghc-steering-committee mailing list | ghc-steering-committee@haskell.org | https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

Richard Eisenberg

11 Feb 11 Feb

4:20 p.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

I see that you've posted acceptance, Ben. Thanks. But my understanding is that the accepted version was the one without `lift` in the class. Is that your understanding, too? If so, would you mind posting to the proposal author? (I think it's cleaner to have communication on this come from the shepherd.) Thanks! Richard

...

On Jan 30, 2019, at 8:09 AM, Simon Peyton Jones via ghc-steering-committee wrote:

I'm content with liftTyped. And while (2) is nicer ab-initio, it's not a big deal having two members of the class, so I'm fine with (1).

Simon

| -----Original Message----- | From: ghc-steering-committee | On Behalf Of Eric Seidel | Sent: 19 January 2019 17:00 | To: ghc-steering-committee@haskell.org | Subject: Re: [ghc-steering-committee] [Proposal] #175: Lift.liftTyped | (recommendation: accept) | | Let's try to pick this discussion back up. The proposal has been | lingering on our end for a couple months now! | | We all seem to be in agreement that the basic proposal of adding | `liftTyped` should be accepted. The discussion has mostly focused on | backwards-compatibility concerns around the default implementation of | `Lift`, specifically that the new mutually-recursive definitions of | `lift` and `liftTyped` could break **clients** of libraries that use TH | without even issuing a warning in the libraries themselves. | | There are two proposed solutions that seem to have some support. | | 1. I suggested a targeted warning/error about empty `Lift` instances that | could be bundled with GHC. TH is bundled with GHC, so this would | guarantee that we notify (or break) libraries before client code is | affected. It would also let library authors avoid CPP while supporting | multiple versions of TH. Joachim expressed his support for this solution. | | 2. The proposal suggests an alternative that replaces `lift` with | `liftTyped` as the sole method of `Lift`. This is probably how we would | design the `Lift` class today if we were starting from scratch. On the | other hand, it forces library authors to use CPP to support multiple | versions of TH, which people often try to avoid (e.g. due to tooling | issues). Simon and Richard have expressed support for this solution. | | I would actually be happy with either solution. | | Joachim, are you happy with option 2? If so, perhaps we should just agree | on it and move forward with the proposal. | | On Fri, Dec 21, 2018, at 22:04, Richard Eisenberg wrote: | > I favor Simon's suggestion. Just rip out `lift` from the class and | > define it as a top-level function. This breaks backward-compat, but in | > an inescapable way. And TH users are accustomed to breakage. My | > problem earlier wasn't just that it wasn't backward-compatible, but | > that it was incompatible in such a terrible way. Simon's suggestion | > leads to a better incompatibility. | > | > Richard | > | > > On Dec 19, 2018, at 5:49 AM, Eric Seidel wrote: | > > | > > Joachim wrote: | > >> I think 1. is at odds in GHC – we allow partiality in other places | > >> as well (partially initialized records, for example). Disallowing | > >> it feels like a too fundamental change to the language for the | > >> problem we need to solve here. | > > | > > Yes, I agree that changing the semantics of MINIMAL solely to fix | this issue is not a great idea. But having a MINIMAL violation be a | warning instead of an error generally feels shady to me. | > > | > > The only use of MINIMAL pragmas I've seen is to break mutually | recursive definitions. That means there's an important difference between | MINIMAL pragmas and other sources of partiality. In a partial record | construction, your program will be fine so long as you don't access the | uninitialized fields. This is dangerous, so we warn you about it, but | it's still possible that your program will be correct. With a MINIMAL | violation, it seems like the programmer is telling us that an instance | *cannot* be correct unless it provides the minimal definitions. | > > | > > But I shouldn't derail the conversation.. Perhaps I'll write up a | separate proposal about MINIMAL. | > > | > >> 3. is a neat idea, but again a very big cannon for a niche problem. | > >> If we _had_ ghc-fix, then this might be a good approach, and we can | > >> keep this in mind as additional evidence that ghc-fix would be good | > >> (GSOC anyone), but let's not wait for that here. | > > | > > Indeed, I wouldn't make this wait on ghc-fix either, but it would be | a very nice thing to have right now! | > > | > >> 2. is good, I think. We have had plenty of such kludges, e.g. | > >> around the Monad refactoring. | > > | > > If we're ok with kludges like this to ease the migration, I think | it's clearly the simplest solution. | > > | > > --- | > > | > > Simon wrote: | > > | > >> Is there any reason for not adopting the second alternative in | section 5: make liftTyped the sole method of Lift? | > > | > > I suspect this would break more code than the current proposal, since | you do sometimes have to write a manual `lift` instance (eg if you have a | field like Text that doesn't have an instance). Fixing such code would | also require CPP, which we usually try to avoid. | > > _______________________________________________ | > > ghc-steering-committee mailing list | > > ghc-steering-committee@haskell.org | > > https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-commi | > > ttee | > | _______________________________________________ | ghc-steering-committee mailing list | ghc-steering-committee@haskell.org | https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee _______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

Joachim Breitner

1 May 1 May

2:56 p.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

Hi, I am bit lost here. Richard, do you feel confident to talk to the authors about the changes we are still waiting for (or just do them on the PR branch yourself), so that we can merge this accepted proposal? Cheers, Joachim Am Montag, den 11.02.2019, 16:20 -0500 schrieb Richard Eisenberg:

...

I see that you've posted acceptance, Ben. Thanks. But my understanding is that the accepted version was the one without `lift` in the class. Is that your understanding, too? If so, would you mind posting to the proposal author? (I think it's cleaner to have communication on this come from the shepherd.)

Thanks! Richard

...
On Jan 30, 2019, at 8:09 AM, Simon Peyton Jones via ghc-steering-committee wrote:

I'm content with liftTyped. And while (2) is nicer ab-initio, it's not a big deal having two members of the class, so I'm fine with (1).

Simon

...
-----Original Message----- From: ghc-steering-committee On Behalf Of Eric Seidel Sent: 19 January 2019 17:00 To: ghc-steering-committee@haskell.org Subject: Re: [ghc-steering-committee] [Proposal] #175: Lift.liftTyped (recommendation: accept)

Let's try to pick this discussion back up. The proposal has been lingering on our end for a couple months now!

We all seem to be in agreement that the basic proposal of adding `liftTyped` should be accepted. The discussion has mostly focused on backwards-compatibility concerns around the default implementation of `Lift`, specifically that the new mutually-recursive definitions of `lift` and `liftTyped` could break **clients** of libraries that use TH without even issuing a warning in the libraries themselves.

There are two proposed solutions that seem to have some support.

1. I suggested a targeted warning/error about empty `Lift` instances that could be bundled with GHC. TH is bundled with GHC, so this would guarantee that we notify (or break) libraries before client code is affected. It would also let library authors avoid CPP while supporting multiple versions of TH. Joachim expressed his support for this solution.

2. The proposal suggests an alternative that replaces `lift` with `liftTyped` as the sole method of `Lift`. This is probably how we would design the `Lift` class today if we were starting from scratch. On the other hand, it forces library authors to use CPP to support multiple versions of TH, which people often try to avoid (e.g. due to tooling issues). Simon and Richard have expressed support for this solution.

I would actually be happy with either solution.

Joachim, are you happy with option 2? If so, perhaps we should just agree on it and move forward with the proposal.

...
I favor Simon's suggestion. Just rip out `lift` from the class and define it as a top-level function. This breaks backward-compat, but in an inescapable way. And TH users are accustomed to breakage. My problem earlier wasn't just that it wasn't backward-compatible, but that it was incompatible in such a terrible way. Simon's suggestion leads to a better incompatibility.

Richard

...
On Dec 19, 2018, at 5:49 AM, Eric Seidel wrote:

Joachim wrote:

...
I think 1. is at odds in GHC – we allow partiality in other places as well (partially initialized records, for example). Disallowing it feels like a too fundamental change to the language for the problem we need to solve here.

Yes, I agree that changing the semantics of MINIMAL solely to fix

...
...
The only use of MINIMAL pragmas I've seen is to break mutually

recursive definitions. That means there's an important difference between MINIMAL pragmas and other sources of partiality. In a partial record construction, your program will be fine so long as you don't access the uninitialized fields. This is dangerous, so we warn you about it, but it's still possible that your program will be correct. With a MINIMAL violation, it seems like the programmer is telling us that an instance *cannot* be correct unless it provides the minimal definitions.

...
But I shouldn't derail the conversation.. Perhaps I'll write up a

separate proposal about MINIMAL.

...
...
3. is a neat idea, but again a very big cannon for a niche problem. If we _had_ ghc-fix, then this might be a good approach, and we can keep this in mind as additional evidence that ghc-fix would be good (GSOC anyone), but let's not wait for that here.

Indeed, I wouldn't make this wait on ghc-fix either, but it would be

a very nice thing to have right now!

...
...
2. is good, I think. We have had plenty of such kludges, e.g. around the Monad refactoring.

If we're ok with kludges like this to ease the migration, I think

it's clearly the simplest solution.

...
---

Simon wrote:

...
Is there any reason for not adopting the second alternative in

On Fri, Dec 21, 2018, at 22:04, Richard Eisenberg wrote: this issue is not a great idea. But having a MINIMAL violation be a warning instead of an error generally feels shady to me. section 5: make liftTyped the sole method of Lift?

...
...
I suspect this would break more code than the current proposal, since

you do sometimes have to write a manual `lift` instance (eg if you have a field like Text that doesn't have an instance). Fixing such code would also require CPP, which we usually try to avoid.

...
_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-commi ttee

_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee -- Joachim Breitner mail@joachim-breitner.de http://www.joachim-breitner.de/

Eric Seidel

5:30 p.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

I think this proposal has been subsumed by https://github.com/ghc-proposals/ghc-proposals/pull/209. On Wed, May 1, 2019, at 14:56, Joachim Breitner wrote:

...

Hi,

I am bit lost here. Richard, do you feel confident to talk to the authors about the changes we are still waiting for (or just do them on the PR branch yourself), so that we can merge this accepted proposal?

Cheers, Joachim

Am Montag, den 11.02.2019, 16:20 -0500 schrieb Richard Eisenberg:

...
I see that you've posted acceptance, Ben. Thanks. But my understanding is that the accepted version was the one without `lift` in the class. Is that your understanding, too? If so, would you mind posting to the proposal author? (I think it's cleaner to have communication on this come from the shepherd.)

Thanks! Richard

...
On Jan 30, 2019, at 8:09 AM, Simon Peyton Jones via ghc-steering-committee wrote:

I'm content with liftTyped. And while (2) is nicer ab-initio, it's not a big deal having two members of the class, so I'm fine with (1).

Simon

...
-----Original Message----- From: ghc-steering-committee On Behalf Of Eric Seidel Sent: 19 January 2019 17:00 To: ghc-steering-committee@haskell.org Subject: Re: [ghc-steering-committee] [Proposal] #175: Lift.liftTyped (recommendation: accept)

Let's try to pick this discussion back up. The proposal has been lingering on our end for a couple months now!

We all seem to be in agreement that the basic proposal of adding `liftTyped` should be accepted. The discussion has mostly focused on backwards-compatibility concerns around the default implementation of `Lift`, specifically that the new mutually-recursive definitions of `lift` and `liftTyped` could break **clients** of libraries that use TH without even issuing a warning in the libraries themselves.

There are two proposed solutions that seem to have some support.

1. I suggested a targeted warning/error about empty `Lift` instances that could be bundled with GHC. TH is bundled with GHC, so this would guarantee that we notify (or break) libraries before client code is affected. It would also let library authors avoid CPP while supporting multiple versions of TH. Joachim expressed his support for this solution.

2. The proposal suggests an alternative that replaces `lift` with `liftTyped` as the sole method of `Lift`. This is probably how we would design the `Lift` class today if we were starting from scratch. On the other hand, it forces library authors to use CPP to support multiple versions of TH, which people often try to avoid (e.g. due to tooling issues). Simon and Richard have expressed support for this solution.

I would actually be happy with either solution.

Joachim, are you happy with option 2? If so, perhaps we should just agree on it and move forward with the proposal.

...
I favor Simon's suggestion. Just rip out `lift` from the class and define it as a top-level function. This breaks backward-compat, but in an inescapable way. And TH users are accustomed to breakage. My problem earlier wasn't just that it wasn't backward-compatible, but that it was incompatible in such a terrible way. Simon's suggestion leads to a better incompatibility.

Richard

...
On Dec 19, 2018, at 5:49 AM, Eric Seidel wrote:

Joachim wrote: > I think 1. is at odds in GHC – we allow partiality in other places > as well (partially initialized records, for example). Disallowing > it feels like a too fundamental change to the language for the > problem we need to solve here.

Yes, I agree that changing the semantics of MINIMAL solely to fix

...
...
The only use of MINIMAL pragmas I've seen is to break mutually

recursive definitions. That means there's an important difference between MINIMAL pragmas and other sources of partiality. In a partial record construction, your program will be fine so long as you don't access the uninitialized fields. This is dangerous, so we warn you about it, but it's still possible that your program will be correct. With a MINIMAL violation, it seems like the programmer is telling us that an instance *cannot* be correct unless it provides the minimal definitions.

...
But I shouldn't derail the conversation.. Perhaps I'll write up a

separate proposal about MINIMAL.

...
> 3. is a neat idea, but again a very big cannon for a niche problem. > If we _had_ ghc-fix, then this might be a good approach, and we can > keep this in mind as additional evidence that ghc-fix would be good > (GSOC anyone), but let's not wait for that here.

Indeed, I wouldn't make this wait on ghc-fix either, but it would be

a very nice thing to have right now!

...
> 2. is good, I think. We have had plenty of such kludges, e.g. > around the Monad refactoring.

If we're ok with kludges like this to ease the migration, I think

it's clearly the simplest solution.

...
---

Simon wrote:

> Is there any reason for not adopting the second alternative in

On Fri, Dec 21, 2018, at 22:04, Richard Eisenberg wrote: this issue is not a great idea. But having a MINIMAL violation be a warning instead of an error generally feels shady to me. section 5: make liftTyped the sole method of Lift?

...
...
I suspect this would break more code than the current proposal, since

you do sometimes have to write a manual `lift` instance (eg if you have a field like Text that doesn't have an instance). Fixing such code would also require CPP, which we usually try to avoid.

...
_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-commi ttee

_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee -- Joachim Breitner mail@joachim-breitner.de http://www.joachim-breitner.de/

_______________________________________________ ghc-steering-committee mailing list ghc-steering-committee@haskell.org https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

Attachments: * signature.asc

Joachim Breitner

2 May 2 May

3:43 a.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

Hi, Am Mittwoch, den 01.05.2019, 17:30 -0400 schrieb Eric Seidel:

...

I think this proposal has been subsumed by https://github.com/ghc-proposals/ghc-proposals/pull/209.

thanks. In this case I’ll close #175. Cheers, Joachim -- Joachim Breitner mail@joachim-breitner.de http://www.joachim-breitner.de/

Simon Peyton Jones

19 Dec 19 Dec

4:28 a.m.

New subject: [Proposal] #175: Lift.liftTyped (recommendation: accept)

Is there any reason for not adopting the second alternative in section 5: make liftTyped the sole method of Lift? I have elaborated on the tracker https://github.com/ghc-proposals/ghc-proposals/pull/175#issuecomment-4485275... Simon | -----Original Message----- | From: ghc-steering-committee | On Behalf Of Eric Seidel | Sent: 19 December 2018 03:37 | To: ghc-steering-committee | Subject: Re: [ghc-steering-committee] [Proposal] #175: Lift.liftTyped | (recommendation: accept) | | 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 | 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 | 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 | 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 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@haskell.org | > >>> > https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-c | > >>> > ommittee | > >>> | > >>> _______________________________________________ | > >>> ghc-steering-committee mailing list | > >>> ghc-steering-committee@haskell.org | > >>> https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-com | > >>> mittee | > > | > > _______________________________________________ | > > ghc-steering-committee mailing list | > > ghc-steering-committee@haskell.org | > > https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-commi | > > ttee | _______________________________________________ | ghc-steering-committee mailing list | ghc-steering-committee@haskell.org | https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

2210

Age (days ago)

2388

Last active (days ago)

List overview

Download

19 comments

6 participants

participants (6)

Ben Gamari
Eric Seidel
Iavor Diatchki
Joachim Breitner
Richard Eisenberg
Simon Peyton Jones