Re: TTG: Handling Source Locations

2019-02-13 Thread Shayan Najd
* "About the latter, until #15247 is fixed" ---> "About the latter, until #15884 is fixed" On Wed, 13 Feb 2019 at 14:00, Shayan Najd wrote: > > >is there any plan to get rid of all those panics? > > There are two sorts of panics related to TTG: the ones due to #15247 > (i.e. unused extension

Re: TTG: Handling Source Locations

2019-02-13 Thread Shayan Najd
>is there any plan to get rid of all those panics? There are two sorts of panics related to TTG: the ones due to #15247 (i.e. unused extension constructors), and the ones due to #15884 (i.e. issues with view patterns). About the former, I believe we all agree. Moreover, using Solution A

Re: TTG: Handling Source Locations

2019-02-13 Thread Ryan Scott
Yes, I agree. This will require sprinkling the codebase with EmptyCase due to [1], but that's still a sight better than calling `panic`. After GHC 8.10 is released (and the minimum version of GHC that HEAD supports is 8.8), we can even remove these empty cases by making the empty data type fields

Re: TTG: Handling Source Locations

2019-02-13 Thread Ryan Scott
> Yes, I have reported it while back. I don't know of the progress in fixing this. Reported what? #15884? [1] You do realize that there is a very simple workaround for that issue, right? Instead of writing this, which is subject to the pattern-guard completeness issues observed in #15753: f

Re: TTG: Handling Source Locations

2019-02-12 Thread Adam Gundry
While we're on the topic, is there any plan to get rid of all those panics? AFAICS they are entirely unnecessary: we should just use an empty datatype for unused constructor extension points, then we can eliminate it to get whatever we like. See #15247. Adam On 12/02/2019 15:40, Shayan Najd

Re: TTG: Handling Source Locations

2019-02-12 Thread Shayan Najd
> Someone could easily call rnPatAndThen when they should call rnLPatAndThen. > This would cause a panic. With Solution A, there shouldn't be two functions `rnLPatAndThen` and `rnPatAndThen` anyways. There should be only `rnPatAndThen` with an extra case for the wrapper node. > There's also the

Re: TTG: Handling Source Locations

2019-02-12 Thread Richard Eisenberg
That's true, but how would it play out in practice? For example, take a look at RnPat. There is a rnLPatAndThen which uses wrapSrcSpanCps to extract the location and then call rnPatAndThen. rnPatAndThen, in turn, just panics if it sees the extension point, because that's an unexpected

Re: TTG: Handling Source Locations

2019-02-12 Thread Shayan Najd
> [Richard:] I disagree here. If we add locations to a node twice, then we'll > have to use dL twice to find the underlying constructor. This is another case > there the type system offers the producer flexibility but hamstrings the > consumer. Depends on the semantics of `dL`: currently (for

Re: TTG: Handling Source Locations

2019-02-12 Thread Shayan Najd
> My problem, though, is that this is just a convention -- no one checks it. It > would be easy to forget. I am not sure if I understand: shouldn't the totality checker warn if there is no pattern for the wrapper constructor (hence enforce the convention)? On Tue, 12 Feb 2019 at 15:19, Richard

Re: TTG: Handling Source Locations

2019-02-12 Thread Richard Eisenberg
> On Feb 12, 2019, at 5:19 AM, Shayan Najd wrote: > > About the new code, the convention is straightforward: anytime you > destruct an AST node, assume a wrapper node inside (add an extra > case), or use the smart constructors/pattern synonyms. Aha! This, I did not know. So, you're saying

Re: TTG: Handling Source Locations

2019-02-12 Thread Vladislav Zavialov
> One way to think of it is this: we can now put SrcSpans where they make > sense, rather than everywhere. I claim an SrcSpan makes sense everywhere, so this is not a useful distinction. Think about it as code provenance, an AST node always comes from somewhere: a user-written .hs file, a GHCi

RE: TTG: Handling Source Locations

2019-02-12 Thread Simon Peyton Jones via ghc-devs
- | From: ghc-devs On Behalf Of Matthew | Pickering | Sent: 12 February 2019 09:08 | To: Vladislav Zavialov | Cc: GHC | Subject: Re: TTG: Handling Source Locations | | I just did this now, it was quite disconcerting that my code continued to | compile after applying `cL loc` to the return

Re: TTG: Handling Source Locations

2019-02-12 Thread Shayan Najd
.pdf On Sat, 9 Feb 2019 at 17:19, Richard Eisenberg wrote: > > Hi devs, > > I just came across [TTG: Handling Source Locations], as I was poking around > in RdrHsSyn and found wondrous things like (dL->L wiz waz) all over the place. > > General outline: > https://ghc.

Re: TTG: Handling Source Locations

2019-02-12 Thread Matthew Pickering
s p) = SrcSpan` for many a `Thing`, but I don't see it > as a downside at all. We should do so anyway, to get rid of parsing > API annotations and put them in the AST proper. > > All the best, > Vladislav > > On Sat, Feb 9, 2019 at 7:19 PM Richard Eisenberg wrote: > > > > H

Re: TTG: Handling Source Locations

2019-02-09 Thread Vladislav Zavialov
t came across [TTG: Handling Source Locations], as I was poking around > in RdrHsSyn and found wondrous things like (dL->L wiz waz) all over the place. > > General outline: > https://ghc.haskell.org/trac/ghc/wiki/ImplementingTreesThatGrow/HandlingSourceLocations > Phab diff: https://