One way to think of it is this: we can now put SrcSpans where they make sense, rather than everywhere. We can still say (Located t) in places where we want to guarantee a SrcSpan.
Yes, this lets us add more than one; that's redundant but not harmful.
Simon
| -----Original Message-----
| 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 value of one of my
| functions.
|
| On Sat, Feb 9, 2019 at 5:40 PM Vladislav Zavialov
| wrote:
| >
| > I wholly share this concern, which is why I commented on the Phab diff:
| >
| > > Does this rely on the caller to call dL on the pattern? Very fragile,
| let's not do that.
| >
| > In addition, I'm worried about illegal states where we end up with
| > multiple nested levels of `NewPat`, and calling `dL` once is not
| > sufficient.
| >
| > As to the better solution, I think we should just go with Solution B
| > from the Wiki page. Yes, it's somewhat more boilerplate, but it
| > guarantees to have locations in the right places for all nodes. The
| > main argument against it was that we'd have to define `type instance
| > XThing (GhcPass 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:
| > >
| > > 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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgh
| > > c.haskell.org%2Ftrac%2Fghc%2Fwiki%2FImplementingTreesThatGrow%2FHand
| > > lingSourceLocations&data=02%7C01%7Csimonpj%40microsoft.com%7C915
| > > 2cd5c5b624a9fac5c08d690c9a908%7C72f988bf86f141af91ab2d7cd011db47%7C1
| > > %7C0%7C636855593134767677&sdata=I6kltUVNtcMItCao1dPvnM86%2FlE8ky
| > > CwshV81dD6mbY%3D&reserved=0 Phab diff:
| > > https://phabricator.haskell.org/D5036
| > > Trac ticket:
| > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgh
| > > c.haskell.org%2Ftrac%2Fghc%2Fticket%2F15495&data=02%7C01%7Csimon
| > > pj%40microsoft.com%7C9152cd5c5b624a9fac5c08d690c9a908%7C72f988bf86f1
| > > 41af91ab2d7cd011db47%7C1%7C0%7C636855593134767677&sdata=VeRbLhJD
| > > ZQv%2FCZ39lMpwo2SRhmcyIsHRgwXNYDN28cA%3D&reserved=0
| > > Commit:
| > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
| > > tlab.haskell.org%2Fghc%2Fghc%2Fcommit%2F509d5be69c7507ba5d0a5f39ffd1
| > > 613a59e73eea&data=02%7C01%7Csimonpj%40microsoft.com%7C9152cd5c5b
| > > 624a9fac5c08d690c9a908%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C
| > > 636855593134767677&sdata=nv9GjvSvGweBPmsHEVD1jBB7yz0Br0hDHtZ5Exv
| > > uDqU%3D&reserved=0
| > >
| > > I see why this change is wanted and how the new version works.
| > >
| > > It seems to me, though, that this move makes us *less typed*. That
| is, it would be very easy (and disastrous) to forget to match on a
| location node. For example, I can now do this:
| > >
| > > > foo :: LPat p -> ...
| > > > foo (VarPat ...) = ...
| > >
| > > Note that I have declared that foo takes a located pat, but then I
| forgot to extract the location with dL. This would type-check, but it
| would fail. Previously, the type checker would ensure that I didn't
| forget to match on the L constructor. This error would get caught after
| some poking about, because foo just wouldn't work.
| > >
| > > However, worse, we might forget to *add* a location when downstream
| functions expect one. This would be harder to detect, for two reasons:
| > > 1. The problem is caught at deconstruction, and figuring out where an
| object was constructed can be quite hard.
| > > 2. The problem might silently cause trouble, because dL won't
| actually fail on a node missing a location -- it just gives noSrcSpan. So
| the problem would manifest as a subtle degradation in the quality of an
| error message, perhaps not caught until several patches (or years!)
| later.
| > >
| > > So I'm uncomfortable with this direction of travel.
| > >
| > > Has this aspect of this design been brought up before? I have to say
| I don't have a great solution to suggest. Perhaps the best I can think of
| is to make Located a type family. It would branch on the type index to
| HsSyn types, introducing a Located node for GhcPass but not for other
| types. This Isn't really all that extensible (I think) and it gives
| special status to GHC's usage of the AST. But it seems to solve the
| immediate problems without the downside above.
| > >
| > > Sorry for reopening something that has already been debated, but
| (unless I'm missing something) the current state of affairs seems like a
| potential wellspring of subtle bugs.
| > >
| > > Thanks,
| > > Richard
| > > _______________________________________________
| > > ghc-devs mailing list
| > > ghc-devs@haskell.org
| > > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmai
| > > l.haskell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-devs&data=02%
| > > 7C01%7Csimonpj%40microsoft.com%7C9152cd5c5b624a9fac5c08d690c9a908%7C
| > > 72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636855593134767677&sd
| > > ata=EAHftJhgmjTvm8f3de99dOFSoddfwu1KPoVHMwP1KtA%3D&reserved=0
| > _______________________________________________
| > ghc-devs mailing list
| > ghc-devs@haskell.org
| > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.
| > haskell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-devs&data=02%7C01
| > %7Csimonpj%40microsoft.com%7C9152cd5c5b624a9fac5c08d690c9a908%7C72f988
| > bf86f141af91ab2d7cd011db47%7C1%7C0%7C636855593134767677&sdata=EAHf
| > tJhgmjTvm8f3de99dOFSoddfwu1KPoVHMwP1KtA%3D&reserved=0
| _______________________________________________
| ghc-devs mailing list
| ghc-devs@haskell.org
| https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.has
| kell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-
| devs&data=02%7C01%7Csimonpj%40microsoft.com%7C9152cd5c5b624a9fac5c08d
| 690c9a908%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636855593134767677
| &sdata=EAHftJhgmjTvm8f3de99dOFSoddfwu1KPoVHMwP1KtA%3D&reserved=0
