TTG: Handling Source Locations

Tue Feb 12 11:00:27 UTC 2019

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 <ghc-devs-bounces at haskell.org> On Behalf Of Matthew
|  Pickering
|  Sent: 12 February 2019 09:08
|  To: Vladislav Zavialov <vladislav at serokell.io>
|  Cc: GHC <ghc-devs at haskell.org>
|  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 <vladislav at serokell.io>
|  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 <rae at cs.brynmawr.edu>
|  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 at 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 at 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 at 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