Backporting srcLoc to the GHC 7.10 branch
Simon Peyton Jones
simonpj at microsoft.com
Mon Jul 13 22:17:07 UTC 2015
We (Ben/Herbert/myself/Simon) had a pow-wow too.
· We have a strong bias against making changes late in the release cycle
· And especially ones that change the API at all
But on the other hand
· Mateusz did first float this back in mid-April and we didn’t pay enough attention at the time which is making us feel guilty.
· Although nothing is risk free, it seems unlikely to break anything because the changes only have an effect if you import the new module.
· Stackage gives us a pretty good smoke-test for if we’ve broken anything.
· Zalora want this strongly; and FP Complete do mildly.
As to the stability of the feature itself, I’ll feel no compunction about changing it a bit in 7.12, say. We often change new features in the light of feedback, and putting it in a release is a way to get people to actually use it and thereby get that feedback.
So we decided, strictly without setting a precedent, to try applying the patch to 7.10.2, validating with the usual test suite and Stackage.
I hope that’s ok with everyone.
Simon
From: Michael Snoyman [mailto:michael at snoyman.com]
Sent: 13 July 2015 18:33
To: Simon Peyton Jones; Mateusz Kowalczyk; ghc-devs at haskell.org
Cc: Mark Lentczner
Subject: Re: Backporting srcLoc to the GHC 7.10 branch
Hi Simon,
We had a small pow-wow over here. We're already providing relevant customers with a custom-built GHC, and only using this feature internally to their codebases, so it's not a necessity to get this into upstream GHC 7.10. It would be nice if the library ecosystem could start to add in support for this feature, but on the other hand having it out in the wild ties everyone's hands with improving the feature before the 7.12 release.
So put FP Complete down as somewhat ambivalent on whether it should go out.
On Mon, Jul 13, 2015 at 3:16 AM Simon Peyton Jones <simonpj at microsoft.com<mailto:simonpj at microsoft.com>> wrote:
> * There is some change you want to make to 7.10.2.
> I'm not sure precisely what it is.
| The change would be to put the latest two commits from
|
| https://github.com/nh2/ghc/commits/ghc-7.10.1-release-srcloc-ip
|
| on top of 7.10 branch.
Aha. Now you say "the top two commits" I can see what you are talking about, namely the entire CallStack feature. Specifically
https://github.com/nh2/ghc/commit/5cd10eb4b6eef06f53bbdcfae4b9f8f505ea522c
| > * The change is an API change, which we do not normally make.
| > But you argue that it will harm no one (why are you so sure?),
| > and you are confident that it will not break anything (why?)
|
| The API change is precisely at this commit.
|
| https://github.com/nh2/ghc/commit/d4e476817a7df449b71a2acc515b4d0fa6f8
| 9b6b
It's much more than that! The main commit (the one you want) adds `CallStack`, `getCallStack` etc, as well as special treatment for implicit parameters of that type.
That widens the API, which is certainly less disruptive than changing it.
Let's see what Ben and Herbert have to say.
Do any other ghc-devs, or Mark or Michael, have an opinion?
Simon
|
| I never said that it will not harm anyone or that it will not break
| anything. I only said that it is a small price to pay.
|
| The change is your own in the typechecker, a two SrcSpans turning into
| RealSrcSpans. I think we can agree at least that it is not a large
| change. I understand if it gets rejected on this basis regardless, I
| am merely trying my chances.
|
| > * FP Complete and Zalora specifically want this change.
| > Zalora = you? Who at FP Complete wants the change?
|
| Some of us at Zalora, yes. This thread of messages was created my FP
| Complete and they did the backport. They even say that they'll include
| the changes into their custom build of GHC and ship that to their
| customers. So it seems like they'd want the change. The very first
| message in this thread is Michael Snoyman asking if such backport
| could make it into 7.10.
|
| > Ben and Herbert are the guys you have to persuade if you really want
| this.
| > I suspect it'll be more effective to open a ticket, milestoned for
| > 7.10.2, with specifics on it. Email gets lost; tickets don't.
|
| Noted, thanks.
|
| > Thanks
| >
| > Simon
| >
| >
| > | -----Original Message-----
| > | From: ghc-devs [mailto:ghc-devs-bounces at haskell.org<mailto:ghc-devs-bounces at haskell.org>] On Behalf Of
| > | Mateusz Kowalczyk
| > | Sent: 06 July 2015 17:40
| > | To: ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
| > | Subject: Re: Backporting srcLoc to the GHC 7.10 branch
| > |
| > | On 04/20/2015 05:29 PM, Niklas Hambüchen wrote:
| > | > Hello everybody,
| > | >
| > | > I'm glad to announce that I performed the suggested backporting
| as
| > | > part
| > | of
| > | > my work for FP Complete!
| > | >
| > | > With the help of Eric (thanks for your support in #ghc!) we now
| > | > have
| > | this
| > | > patch for 7.10 and 7.8.
| > | >
| > | > As promised, here are the commits:
| > | >
| > | > * https://github.com/nh2/ghc/commits/ghc-7.10.1-release-srcloc-
| ip
| > | > * https://github.com/nh2/ghc/commits/ghc-7.8.4-release-srcloc-ip
| > | >
| > | > You can see in the history that I only needed to cherry-pick
| "Make
| > | > the location in TcLclEnv and CtLoc into a RealSrcSpan" as a
| > | > dependent
| > | commit,
| > | > so the changes beyond the actual feature are fairly minimal.
| > | >
| > | > For 7.8, I had to do a little more cleanup, see the "Use
| wrapIPTc
| > | instead
| > | > of coercionToTcCoercion on wrapIP" commit.
| > | >
| > | > Regarding the commit of the feature itself, I had to do quite
| some
| > | merge
| > | > resolution, especially due to the (lack of) the -fwarn-
| redundant-
| > | > constraints patch https://github.com/nh2/ghc/commit/32973bf3
| > | > (alias
| > | "but it
| > | > was Christmas, so I did some recreational programming that went
| > | > much further").
| > | > However, the type checker had a strong opinion of what the right
| > | > merge decision was, at least for the 7.10 backport; for 7.8
| there
| > | > was an ambiguity (whether to return `Nothing` or `Just` in
| > | > `interactDict`),
| > | which
| > | > was resolved with Eric's help.
| > | >
| > | > Please be invited to review the two commits.
| > | >
| > | > We would like to make 7.8 and 7.10 binaries with this feature
| > | > available
| > | as
| > | > well, and will do so after getting a review!
| > | >
| > | > Niklas
| > | >
| > | > _______________________________________________
| > | > ghc-devs mailing list
| > | > ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
| > | > http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
| > | >
| > |
| > | Hi all,
| > |
| > | In light of how small the actual backported changes are, I wonder
| if
| > | it would be possible to include it in 7.10.2 (or .3 if we're doing
| > | that). I know that at first it was rejected but it was rejected
| > | before the diff existed. In practice, the only API change was two
| > | SrcSpans changing into RealSrcSpans inside the typechecker. Seems
| > | like a small price, considering two commercial companies that use
| > | Haskell (FP Complete and I'm here on behalf of Zalora's internal
| team) would love to see it.
| > |
| > | I know SPJ suggested perhaps lobbying for early 7.12 instead but
| how
| > | early could we possibly lobby for it considering 7.10.2 is not
| even
| > | out of the door yet?
| > |
| > | Niklas, do you also have a diff on top of 7.10.2 (rc-1/2)? I'd be
| > | interested in seeing that too.
| > |
| > | --
| > | Mateusz K.
| > | _______________________________________________
| > | ghc-devs mailing list
| > | ghc-devs at haskell.org<mailto:ghc-devs at haskell.org>
| > | http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
| >
|
|
| --
| Mateusz K.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-devs/attachments/20150713/326170fc/attachment-0001.html>
More information about the ghc-devs
mailing list