[TTG: Handling Source Locations] Foundation and Pat
Needs ReviewPublic

Authored by shayan-najd on Aug 2 2018, 4:28 AM.

Details

Reviewers
bgamari
alanz
simonpj
Trac Issues
#15495
Summary
  • the class HasSrcSpan, and its functions (e.g., cL and dL), are introduced
  • some instances of HasSrcSpan are introduced
  • some constructors L are replaced with cL
  • some patterns L are replaced with dL view pattern
  • XPat is renamed to NewPat
  • some type annotation are necessarily updated updated (e.g., Pat p --> Pat (GhcPass p))
  • (there was a bug in an earlier version of this patch related to using functor on Located things that is fixed)
Test Plan
  • GHC and the related code (e.g., Haddock) fully compile on my Linux system
  • the patch passes the tests and ./Validate
shayan-najd created this revision.Aug 2 2018, 4:28 AM
shayan-najd retitled this revision from [TTG: Handling Source Locations] - introduced class `HasSrcSpan`, and its functions (e.g., `cL` and `dL`) - instances of `HasSrcSpan` is introduced - some constructors `L` replaced with `cL` - some patterns `L` replaced with `dL` - `XPat` renamed... to [TTG: Handling Source Locations] Foundation and Pat.Aug 2 2018, 4:34 AM
shayan-najd edited the summary of this revision. (Show Details)
shayan-najd edited reviewers, added: simonpj; removed: goldfire.
shayan-najd edited the summary of this revision. (Show Details)Aug 2 2018, 4:59 AM
shayan-najd removed subscribers: goldfire, rwbarton, thomie and 2 others.
shayan-najd updated the Trac tickets for this revision.Aug 9 2018, 7:01 AM
shayan-najd edited the summary of this revision. (Show Details)Aug 14 2018, 4:47 PM
shayan-najd edited the test plan for this revision. (Show Details)

Seems like this patch would be a lot cleaner if a pattern synonym was defined for L. Having to use ViewPatterns everywhere is not very ergonomic.

Seems like this patch would be a lot cleaner if a pattern synonym was defined for L. Having to use ViewPatterns everywhere is not very ergonomic.

I would like to use a pattern synonym (as we did in the wiki), but then we will have issues with the incomplete pattern warnings, wouldn't we?
Can we use the COMPLETE pragma for such a polymorphic pattern instantiated at different types?

It's quite hard to review this patch because it's dominated by routine changes, so it's hard to find the actual payload.

Can you just identify the payload? You partly do that in the Description, but it'd be helpful to say which module these various things are defined in.

Why does HsPat change but not HsExpr say?

An unrelated thing: we should rename Pat to HsPat for consistency.

Other remaks by email.

compiler/deSugar/DsMeta.hs
1696

This is very strange. Why not just add a case to repP? We know this is GhcRn! Much more direct and (probably) more efficient.