Agreed: it doesn't affect the heap layout, just the types. My reading of the wiki page was not that the heap indirection was the main motivation for this little enquiry, though. Rather some type annoyance things. I don't know that E is the best solution, or even a good solution: I just think it deserves to be part of this conversation.

On Wed, Oct 30, 2019 at 2:12 PM Sebastian Graf <sgraf1337@gmail.com> wrote:

> I would like to submit a solution E, which is just a variant of D (or a meshing together of D and B), but may have different pros and cons.

I like this conceptually: No `WrapL`/`WrapX`/`XWrap`/`XRec` (the trouble to find a fitting name already suggests that it's maybe a little too general a concept), but rather a nicely targeted `XLoc`.

But I'm afraid that the unconditional ping-pong incurs an indirection even if you instantiate `XLoc` to `()` all the time. I think that's a no-go, according to the wiki page: Think of TH, which would pay needlessly.
Also it's not much better than just the old ping-pong style, where we could always pass `noSrcLoc` instead of `()` for basically the same heap layout.
Am Mi., 30. Okt. 2019 um 14:06 Uhr schrieb Spiwack, Arnaud <arnaud.spiwack@tweag.io>:
I would like to submit a solution E, which is just a variant of D (or a meshing together of D and B), but may have different pros and cons.

Keep the ping-pong style. But! Let Located take the pass as an argument. Now, Located would be
data Located p a = L (XLoc p) a
We could define XLoc p to be a source span in GHC passes, and () in Template Haskell.

I’m not arguing in favour of E, at this point: just submitting an alternative. I don’t like A though: I’m assuming that if we are free to add the L constructor or not, it will be forgotten often. This is the sort of mistake which will be hard to catch before releases. It sounds like unnecessary risk.

PS: Maybe a friendlier version of Located, in this solution E, could be
data Located a p = L (XLoc p) (a p)
On Mon, Oct 28, 2019 at 11:46 AM Richard Eisenberg <rae@richarde.dev> wrote:
A nice property of Solution D (and I think this is a new observation) is that we could accommodate both located source and unlocated. For example:

> data GhcPass (c :: Pass) -- as today
> data Pass = MkPass Phase HasLoc
> data Phase = Parsed | Renamed | Typechecked
> data HasLoc = YesLoc | NoLoc
>
> type instance WrapL (GhcPass (MkPass p YesLoc)) f = Located (f (GhcPass (MkPass p YesLoc)))
> type instance WrapL (GhcPass (MkPass p NoLoc)) f = f (GhcPass (MkPass p NoLoc))

I don't actually think this is a good idea, as it would mean making a bunch of functions polymorphic in the HasLoc parameter, which is syntactically annoying. But the existence of this approach suggests that the design scales.

Regardless of what you think of this new idea, I'm in favor of Solution D. I like my types to stop me from making errors, and I'm willing to put up with the odd type error asking me to write `unLoc` as I work in order to avoid errors.

Richard

> On Oct 28, 2019, at 10:30 AM, Vladislav Zavialov <vladislav@serokell.io> wrote:
>
>> Are you arguing for Solution D? Or are you proposing some new solution E? I can't tell.
>
> I suspect that I’m arguing for Solution B, but it’s hard to tell because it’s not described in enough detail in the Wiki.
>
>> Easy
>>
>> type instance WrapL ToolPass t = ...
>>
>> What am I missing?
>
>
> This assumes that `WrapL` is an open type family. In this case, there’s no problem. The merge request description has the following definition of WrapL:
>
> type instance WrapL p (f :: * -> *) where
> WrapL (GhcPass p) f = Located (f (GhcPass p))
> WrapL p f = f p
> type LPat p = WrapL p Pat
>
> That wouldn’t be extensible. However, if WrapL is open, then Solution D sounds good to me.
>
> - Vlad
>
>> On 28 Oct 2019, at 13:20, Simon Peyton Jones <simonpj@microsoft.com> wrote:
>>
>> Vlad
>>
>> Are you arguing for Solution D? Or are you proposing some new solution E? I can't tell.
>>
>>
>> | As to merge request !1970, it isn’t good to special-case GhcPass in a
>> | closed type family, making other tools second-class citizens. Let’s say I
>> | have `MyToolPass`, how would I write an instance of `WrapL` for it?
>>
>> Easy
>>
>> type instance WrapL ToolPass t = ...
>>
>> What am I missing?
>>
>> Simon
>>
>> | -----Original Message-----
>> | From: Vladislav Zavialov <vladislav@serokell.io>
>> | Sent: 28 October 2019 10:07
>> | To: Simon Peyton Jones <simonpj@microsoft.com>
>> | Cc: ghc-devs@haskell.org
>> | Subject: Re: Handling source locations in HsSyn via TTG
>> |
>> | I care about this, and I maintain my viewpoint described in
>> | https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.has
>> | kell.org%2Fpipermail%2Fghc-devs%2F2019-
>> | February%2F017080.html&data=02%7C01%7Csimonpj%40microsoft.com%7C06c859
>> | d8bc0f48c73cb208d75b8e895c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63
>> | 7078540055975603&sdata=FhkqfWGXaNX%2Fz4IiCcvoVCyzVsSAlyz6Y1dxEGUjX9I%3
>> | D&reserved=0
>> |
>> | I’m willing to implement this.
>> |
>> | As to merge request !1970, it isn’t good to special-case GhcPass in a
>> | closed type family, making other tools second-class citizens. Let’s say I
>> | have `MyToolPass`, how would I write an instance of `WrapL` for it?
>> |
>> | - Vlad
>> |
>> | > On 28 Oct 2019, at 12:31, Simon Peyton Jones via ghc-devs <ghc-
>> | devs@haskell.org> wrote:
>> | >
>> | > Friends
>> | >
>> | > As you know
>> | >
>> | > • We are trying to use “Trees That Grow” (TTG) to move HsSyn towards
>> | a situation in which GHC is merely a client of a generic HsSyn data type
>> | that can be used by clients other than GHC.
>> | > • One sticking point has been the question of attaching source
>> | locations. We used to have a “ping-pong” style, in which very node is
>> | decorated with a source location, but that’s a bit more awkward in TTG.
>> | > • This wiki page outlines some choices, while ticket #15495 has a
>> | lot of discussion.
>> | > • HEAD embodies Solution A. But it has the disadvantage that the
>> | type system doesn’t enforce locations to be present at all. That has
>> | undesirable consequences (eg ticket #17330)
>> | > • The proposal is to move to Solution D on that page; you can see
>> | how it plays out in MR !1970.
>> | > • (I think Solutions B and C are non-starters by comparison.)
>> | > If you care, please check out the design and the MR. We can change
>> | later, of course, but doing so changes a lot of code, including client
>> | code, so we’d prefer not to.
>> | >
>> | > Let’s try to converge by the end of the week.
>> | >
>> | > Thanks
>> | >
>> | > Simon
>> | >
>> | >
>> | >
>> | >
>> | >
>> | > _______________________________________________
>> | > ghc-devs mailing list
>> | > ghc-devs@haskell.org
>> | >
>> | https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.hask
>> | ell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-
>> | devs&data=02%7C01%7Csimonpj%40microsoft.com%7C06c859d8bc0f48c73cb208d7
>> | 5b8e895c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637078540055975603&a
>> | mp;sdata=CRYEYmjuAYYIhoAeTPbi%2FctCiWmkIjL6pKUTBgVBwo8%3D&reserved=0
>>
>
> _______________________________________________
> ghc-devs mailing list
> ghc-devs@haskell.org
> http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

_______________________________________________
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

_______________________________________________
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs