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

Simon Peyton Jones simon.peytonjones at gmail.com
Tue Oct 3 08:33:01 UTC 2023


Sure.

Just to be clear, I'm not trying to set a precedent here!

Simon

On Tue, 3 Oct 2023 at 09:26, Moritz Angermann <moritz.angermann at gmail.com>
wrote:

> I’m not going to stay in the way of this proposal. I’hope it’s
> understandable if I recuse myself.
>
> On Tue, 3 Oct 2023 at 4:18 PM, Simon Peyton Jones <
> simon.peytonjones at gmail.com> wrote:
>
>> We have to balance the cost of mitigation against the cost of breakage.
>>
>>    - Cost of breakage.  Adam has identified only two packages that woud,
>>    both of which can adapt promptly.  It's true that there is more code out
>>    there, but the absence on Hackage is strongly indicative that there is
>>    unlikely to be much.
>>    - Cost of mitigation.  Supporting both behaviours, having extra
>>    language flags, that must in turn themselves be deprecated away, are real
>>    costs.  The suck effort away from more productive activity.
>>    - I completely agree that if we had a classification,
>>    OverloadedRecordUpdate would have been classified as Experimental, and we
>>    would not be discussing deprecation cycles.  And signalling what is
>>    experimental and what is stable is a *primary *goal of the stability
>>    conversation we are having.  I agree strongly with Moritz on this point.
>>    - However the user manual (Section 6.5.11
>>    <https://ghc.gitlab.haskell.org/ghc/doc/users_guide/exts/overloaded_record_update.html?highlight=overloaded%20record#overloaded-record-update>)
>>    says "*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*".  Short of an official classification
>>    mechanism, which we don't have yet, it seems hard to imagine a clearer
>>    statement than that.
>>
>> My judgement: in this case we should forge ahead without introducing
>> OverloadedRecordNew and supporting both behaviours. (I have not even begun
>> to think about how hard that would be to implement.)  Let's spend our
>> precious cycles elsewhere.
>>
>> Simon
>>
>> On Tue, 3 Oct 2023 at 08:48, Moritz Angermann <moritz.angermann at gmail.com>
>> wrote:
>>
>>> Arnaud,
>>>
>>> That is a good argument. Without an opt-in to the new syntax, they can't
>>> adapt. I guess this brings us to the extension lifecycle. A solution could
>>> be
>>> 1. Deprecate OverloadedRecordUpdate in favour of
>>> OverloadedRecordUpdateNew. The user can adapt their code and use a
>>> different Extension.
>>> 2. Eventually Have OverloadedRecordUpdateNew be a synonym for changed
>>> OverloadedRecordUpdate. And Deprecate OverloadedRecordUpdateNew
>>>     over a longer period of time.
>>>
>>> I think this actually shows the value a `--std=experimental` could have.
>>> If `OverloadedRecordUpdate` was behind `--std=experimental` breaking it
>>> would
>>> be permissible by design. And we would not have this discussion, and
>>> Adam would not need to try to find all the potential breaking codes. This
>>> is part
>>> of the proposal to make `--std=experimental` a thing, allow fast(er)
>>> paced iteration on the experimental side of the compiler, while being
>>> extremely
>>> explicit about it being experimental features that can break at any time
>>> and without warning.
>>>
>>> The deprecation process outlined above would be fore non-experimental
>>> features. They can still be changed, but with a more rigorous change
>>> evolution.
>>>
>>> I hope this helps to clarify my position?
>>>
>>> Best,
>>>  Moritz
>>>
>>>
>>> On Tue, 3 Oct 2023 at 15:31, Arnaud Spiwack <arnaud.spiwack at tweag.io>
>>> wrote:
>>>
>>>> @Moritz: very clear thank you, though I'd like to understand how one
>>>> could adapt during the transition period to an argument swap, would you
>>>> say? Secondary question: if they can't adapt, isn't there a risk of making
>>>> the breakage more painful by delaying it, and letting more people use
>>>> -XOverloadedRecordUpdate with the argument order which isn't the final one?
>>>>
>>>> @Adam: we have to recognise that Hackage is not all the Haskell code. I
>>>> expect Hackage to be a little more conservative in extensions than end-user
>>>> applications. As such we're not guaranteed an absence of breakage (it's
>>>> still probably quite small, but it's hard to quantify).
>>>>
>>>> On Tue, 3 Oct 2023 at 09:21, Adam Gundry <adam at well-typed.com> wrote:
>>>>
>>>>> The backwards compatibility impact here is really extremely small,
>>>>> because OverloadedRecordUpdate is essentially unusable at present (as
>>>>> well as being explicitly documented as subject to change), so nobody
>>>>> uses it. While someone could implement a warning when the extension is
>>>>> turned on to say that it will change in the future, I'm not sure I see
>>>>> much point.
>>>>>
>>>>> I used https://hackage-search.serokell.io/?q=OverloadedRecordUpdate
>>>>> to
>>>>> look for occurrences of OverloadedRecordUpdate on Hackage, and I found
>>>>> precisely one pair of packages using it "for real":
>>>>>
>>>>> large-anon-0.3.0
>>>>> large-records-0.4
>>>>>
>>>>> These packages were created by my colleague Edsko in discussion with
>>>>> me,
>>>>> and will need to be amended when the proposal is implemented, because
>>>>> they want to take advantage of the new functionality.
>>>>>
>>>>> The other packages on Hackage containing the string
>>>>> OverloadedRecordUpdate are tools that reference all extensions:
>>>>>
>>>>> Cabal-syntax-3.10.1.0
>>>>> extensions-0.1.0.0
>>>>> fourmolu-0.14.0.0
>>>>> ghc-9.6.3
>>>>> ghc-boot-th-9.6.3
>>>>> ghc-exactprint-1.7.0.1
>>>>> ghc-hs-meta-0.1.2.0
>>>>> ghc-lib-9.6.2.20230523
>>>>> ghc-lib-parser-9.6.2.20230523
>>>>> hackport-0.8.4.0
>>>>> haskell-src-meta-0.8.12
>>>>> hindent-6.1.0
>>>>> hlint-3.6.1
>>>>> ormolu-0.7.2.0
>>>>>
>>>>> plus there are a few references in comments:
>>>>>
>>>>> lifx-lan-0.8.2
>>>>> optics-core-0.4.1.1
>>>>> pvector-0.1.1
>>>>> record-dot-preprocessor-0.2.16
>>>>> tztime-0.1.1.0
>>>>>
>>>>> While I generally agree with the "don't abruptly break existing code"
>>>>> position, in this case I don't think there is code out there to break.
>>>>>
>>>>> Adam
>>>>>
>>>>>
>>>>> On 03/10/2023 01:14, Moritz Angermann wrote:
>>>>> > 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
>>>>> > <mailto: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
>>>>> >     <mailto: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 <mailto: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
>>>>> >         <mailto: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 <mailto:arnaud.spiwack at tweag.io>>
>>>>> wrote:
>>>>> >          >>>> Dear all,
>>>>> >          >>>>
>>>>> >          >>>> [ Proposal #583
>>>>> >         https://github.com/ghc-proposals/ghc-proposals/pull/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
>>>>> <
>>>>> 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 <
>>>>> 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.
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Adam Gundry, Haskell Consultant
>>>>> Well-Typed LLP, https://www.well-typed.com/
>>>>>
>>>>> Registered in England & Wales, OC335890
>>>>> 27 Old Gloucester Street, London WC1N 3AX, England
>>>>> <https://www.google.com/maps/search/27+Old+Gloucester+Street,+London+WC1N+3AX,+England?entry=gmail&source=g>
>>>>>
>>>>> _______________________________________________
>>>>> 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
>>>>
>>> _______________________________________________
>>> 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/1ec2a9ed/attachment-0001.html>


More information about the ghc-steering-committee mailing list