[ghc-steering-committee] #583: HasField redesign, rec: accept
Moritz Angermann
moritz.angermann at gmail.com
Tue Oct 3 08:26:08 UTC 2023
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/4e347a75/attachment-0001.html>
More information about the ghc-steering-committee
mailing list