[ghc-steering-committee] #583: HasField redesign, rec: accept

Tue Oct 3 00:14:18 UTC 2023

Arnaud,

thank you for your write up. And sorry that my view seems to be not clear.
Let me try to explain.
My position is: I'm against anything that _abruptly_ breaks existing code.
That's basically all there is to it.
Therefore I'm strongly against breaking changes without an appropriate
warning period (from the compiler).
I also strongly believe that most people do not read documentation, and the
authoritative messages are
the ones the compiler produces. As I've alluded to in a different email,
there are lots of people who work
on software as a 9-5 job, and don't spend their freetime tinkering and
playing with languages. They just
want to get the job done they are assigned. I have a lot of respect for
them. In my experience they don't
read compiler release announcements, or even go and read the compiler
documentation. They are given
the compiler version the company decided on for use in production. That's
the tool they use. And that tool
in my opinion needs to be rock solid, and not break easily across versions.
Thus yes, I would very much
like to see us not have breaking changes at all, but I can see that we may
need breaking changes
occasionally. In those cases I want it to be very visible to the consumers
that this breaking change is
coming towards them (compiler warnings). Giving them some time to adjust
(migration period), until the
breaking change happens.  Ultimately we should be able to compiler existing
code that compiles today
with at least the next compiler without it rejecting the code or needing
modifications to the code (-Werror
excluded).

Thus what I'm arguing for is:
- Let's implement this backwards compatibility.
- Add compiler warnings about the arguments being swapped in a future GHC
version. For _a least_ one major release.
- Make the breaking change in a subsequent release.

Alternatively I could also see:
- Adding compiler warnings now that the arguments will be swapped in a
future GHC version (for at least one major release).
- Implement the breaking change in a subsequent release.

Either of those would be ok with me. Implementing a breaking change from
one version to the next, without an
appropriate deprecation/migration period (that is, the compiler will warn
loudly that changes are coming) is something
I am _very_ vehemently against.

If the migration/deprecation warnings would provide a link to a GitHub
ticket or something where more information
can be found and maybe even a discussion could be had would probably also
be a good idea.

I hope this helps clarify my position? If not, feel free to ask more, I'm
also happy to jump onto a call to explain my
position if needed.

Best,
 Moritz

On Mon, 2 Oct 2023 at 23:31, Arnaud Spiwack <arnaud.spiwack at tweag.io> wrote:

> Sorry I took a little bit of time to react to this, it was a lot to take
> in and I didn't have the mental space last week.
>
> The only person that may have spoken against the current state of the
> proposal is Moritz. Yet I realise that I don't actually know, Moritz, what
> your position is.
>
> To recap: to use -XOverloadedRecordUpdate in current GHC, you need to use
> -XRebindableSyntax and provide a setField function. In the new proposal you
> can use -XOverloadedRecordUpdate without -XRebindableSyntax, but when
> -XRebindableSyntax is on, the setField function that you have to provide
> has its argument swapped. The current documentation of
> OverloadedRecordUpdate has the following text at the top “*EXPERIMENTAL* *This
> design of this extension may well change in the future. It would be
> inadvisable to start using this extension for long-lived libraries just
> yet.*”.
>
> Now, I don't quite see how we could have a transition period that would
> allow a smooth transition there. There is no piece of code, with
> RebindableSyntax, that would compile before and after the change. So unless
> I'm missing something the position we can take as a committee can be either
> - Let's have the breakage without a transition period
> - Let's not make the breaking change ever and use the earlier argument
> order for set
>
> Which one do you argue for, or am I missing another option?
>
> On Sun, 24 Sept 2023 at 15:36, Eric Seidel <eric at seidel.io> wrote:
>
>> I am in favor of this proposal.
>>
>> On Thu, Sep 21, 2023, at 03:37, Arnaud Spiwack wrote:
>> > Dear all.
>> >
>> > I submitted my recommendation 3 weeks ago, and only Simon has commented
>> > yet. Please let me know your thoughts.
>> >
>> > On Wed, 6 Sept 2023 at 16:27, Arnaud Spiwack <arnaud.spiwack at tweag.io>
>> wrote:
>> >> Dear all,
>> >>
>> >> Don't forget to opine here. To reiterate, I really don't expect the
>> proposal to be controversial. The text of the proposal is rather long, but
>> is made easy to read. So it shouldn't take too much of your time.
>> >>
>> >> /Arnaud
>> >>
>> >> On Thu, 31 Aug 2023 at 01:03, Simon Peyton Jones <
>> simon.peytonjones at gmail.com> wrote:
>> >>> I support acceptance.
>> >>>
>> >>> Simon
>> >>>
>> >>> On Wed, 30 Aug 2023 at 16:09, Arnaud Spiwack <arnaud.spiwack at tweag.io>
>> wrote:
>> >>>> Dear all,
>> >>>>
>> >>>> [ Proposal #583
>> https://github.com/ghc-proposals/ghc-proposals/pull/583 ]
>> >>>>
>> >>>> Our own Adam proposes to amend the design of the highly experimental
>> OverloadedRecordUpdate extension as had been designed in proposal #158 [
>> https://github.com/ghc-proposals/ghc-proposals/blob/master/proposals/0158-record-set-field.rst
>> ] and #405 [ https://github.com/ghc-proposals/ghc-proposals/pull/405 ].
>> >>>>
>> >>>> Specifically, Adam proposes a modification of the type classes that
>> would back the extension.
>> >>>>
>> >>>> In the previous design the HasField class is defined as a lens:
>> >>>>
>> >>>> class HasField (n :: k) r a | r n -> a
>> >>>>   hasField :: r -> (a -> r, a)
>> >>>>
>> >>>> The proposal is to replace it by two classes (slightly simplified)
>> >>>>
>> >>>> class HasField (n :: k) r a | r n -> a
>> >>>>   hasField :: r -> a
>> >>>>
>> >>>> class SetField (n::k) r a | r n -> a
>> >>>>   modifyField :: (a -> a) -> r -> a
>> >>>>   setField :: a -> r -> a
>> >>>>
>> >>>> This is originally motivated by some performance consideration: the
>> prototype implementation of HasField as a lens can be very time consuming
>> because instances of HasFields are generated eagerly at record definition
>> sites, whereas the simple HasField instances can simply reuse the selectors
>> already generated by GHC. But a lot of thoughts have been put into the new
>> design, and my summary can certainly not do it justice: the proposal is
>> very well argumented.
>> >>>>
>> >>>> A point I'll make here is that the new design is actually parametric
>> in the data representation of the field type. Something that wasn't
>> possible in the original design.
>> >>>>
>> >>>> This proposal is not technically backward compatible, because the
>> order of argument in which OverloadedRecordUpdate expects the argument of
>> setField is changed. This is not essential to the proposal, but this is a
>> more consistent order argument with the rest of Haskell. And considering
>> that OverloadedRecordUpdate is very loudly advertised as experimental, I
>> recommend accepting this breakage.
>> >>>>
>> >>>> Overall the proposal is actually more backward compatible with GHC
>> 9.8 than the original design, as the HasField class is left unchanged.
>> >>>>
>> >>>> Overall, the proposal looks quite reasonable to me, and well-argued.
>> I recommend acceptance.
>> >>>>
>> >>>> --
>> >>>> Arnaud Spiwack
>> >>>> Director, Research at https://moduscreate.com and https://tweag.io.
>> >>>> _______________________________________________
>> >>>> ghc-steering-committee mailing list
>> >>>> ghc-steering-committee at haskell.org
>> >>>>
>> https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee
>> >>
>> >>
>> >> --
>> >> Arnaud Spiwack
>> >> Director, Research at https://moduscreate.com and https://tweag.io.
>> >
>> >
>> > --
>> > Arnaud Spiwack
>> > Director, Research at https://moduscreate.com and https://tweag.io.
>> > _______________________________________________
>> > 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
>>
>
>
> --
> Arnaud Spiwack
> Director, Research at https://moduscreate.com and https://tweag.io.
> _______________________________________________
> ghc-steering-committee mailing list
> ghc-steering-committee at haskell.org
> https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-steering-committee/attachments/20231003/fb7677d3/attachment.html>