[ghc-steering-committee] #583: HasField redesign, rec: accept
Arnaud Spiwack
arnaud.spiwack at tweag.io
Mon Oct 9 08:06:32 UTC 2023
Dear all,
Absent votes, based on my current understanding of everybody's position,
I'll mark the proposal as accepted, without modification, on Wednesday
(11th October).
On Tue, 3 Oct 2023 at 11:52, Arnaud Spiwack <arnaud.spiwack at tweag.io> wrote:
> Simon votes A above. I vote A too (I think I actually feel pretty strongly
> about this one).
>
> On Tue, 3 Oct 2023 at 11:52, Arnaud Spiwack <arnaud.spiwack at tweag.io>
> wrote:
>
>> Let's do a quick vote. All, which do you prefer?
>>
>> A. Introduce the breakage immediately and change the argument order of
>> setField (current state of the proposal)
>> B. Introduce a new extension -XOverloadedRecordUpdateNew which would use
>> a setField with a different argument order, so that it can be used during
>> the transition period. Eventually OverloadedRecordUpdateNew becomes
>> deprecated and OverloadedRecordUpdate swaps the argument order. (Moritz's
>> proposal)
>> C. Keep the originally proposed setField argument order (nobody have
>> suggested this yet, but I'm including it for completeness)
>>
>> On Tue, 3 Oct 2023 at 10:33, Simon Peyton Jones <
>> simon.peytonjones at gmail.com> wrote:
>>
>>> 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
>>>>>>
>>>>>
>>
>> --
>> Arnaud Spiwack
>> Director, Research at https://moduscreate.com and https://tweag.io.
>>
>
>
> --
> Arnaud Spiwack
> Director, Research at https://moduscreate.com and https://tweag.io.
>
--
Arnaud Spiwack
Director, Research at https://moduscreate.com and https://tweag.io.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-steering-committee/attachments/20231009/06278b2d/attachment-0001.html>
More information about the ghc-steering-committee
mailing list