haskell-suite / haskell-names

Haskell suite library for name resolution
52 stars 17 forks source link

Remember providence of Punned and Wildcarded named #86

Open nomeata opened 7 years ago

nomeata commented 7 years ago

I am using haskell-names to implement a simple renaming pass (including the module name in the symbol name), and I stumbled over a small lack of information on punned names.

Consider

{-# LANGUAGE RecordWildCards #-}
module RecordSelector where
data Record a = Record { field :: a }
unRecord r = field
  where Record { .. } = r

Here, the annotation I get for the use field in unRecord is

Var
  (Scoped None
     (SrcSpanInfo{srcInfoSpan = SrcSpan "RecordSelector.hs" 4 14 4 19,
                  srcInfoPoints = []}))
  (UnQual
     (Scoped (LocalValue (SrcLoc "RecordSelector.hs" 6 14))
        (SrcSpanInfo{srcInfoSpan = SrcSpan "RecordSelector.hs" 4 14 4 19,
                     srcInfoPoints = []}))
     (Ident
        (Scoped (LocalValue (SrcLoc "RecordSelector.hs" 6 14))
           (SrcSpanInfo{srcInfoSpan = SrcSpan "RecordSelector.hs" 4 14 4 19,
                        srcInfoPoints = []}))
        "field")))

so it says it is a locally defined value bound at position 6:14, but the requirement that the name is equal to the name of the record field is lost. So if I rename the record field, the program fails to compile.

Even if I use non-local information and look at what’s at 6:14, I find

PFieldWildcard
  (Scoped (RecPatWildcard [])
     (SrcSpanInfo{srcInfoSpan = SrcSpan "RecordSelector.hs" 6 14 6 16,
                  srcInfoPoints = []}))

which is not very helpful.

Relatedly, if I change .. to field = field, I find

PFieldPat
  (Scoped None
     (SrcSpanInfo{srcInfoSpan = SrcSpan "RecordSelector.hs" 6 14 6 25,
                  srcInfoPoints = [SrcSpan "RecordSelector.hs" 6 19 6 20]}))
  (UnQual
     (Scoped (LocalValue (SrcLoc "RecordSelector.hs" 6 20))
        (SrcSpanInfo{srcInfoSpan = SrcSpan "RecordSelector.hs" 6 14 6 19,
                     srcInfoPoints = []}))
     (Ident
        (Scoped (LocalValue (SrcLoc "RecordSelector.hs" 6 20))
           (SrcSpanInfo{srcInfoSpan = SrcSpan "RecordSelector.hs" 6 14 6 19,
                        srcInfoPoints = []}))
        "field"))
  (PVar
     (Scoped None
        (SrcSpanInfo{srcInfoSpan = SrcSpan "RecordSelector.hs" 6 20 6 25,
                     srcInfoPoints = []}))
     (Ident
        (Scoped ValueBinder
           (SrcSpanInfo{srcInfoSpan = SrcSpan "RecordSelector.hs" 6 20 6 25,
                        srcInfoPoints = []}))
        "field"))]

which says that the left field (the constructor field, at 6:14) is a local value bound at the position of the right name field (the variable, at 6:20), which is certainly wrong.

phischu commented 7 years ago

In the first case the information at 6:14 should be:

PFieldWildcard
  (Scoped (RecPatWildcard [
    Selector (ModuleName () "RecordSelector") (Ident () "field") (Ident () "Record") [Ident () "Record"]])
      (SrcSpanInfo{srcInfoSpan = SrcSpan "RecordSelector.hs" 6 14 6 16,
                  srcInfoPoints = []}))

This should allow you to figure out that the local value "field" came from a record pattern wildcard.

Alternatively we could add more information to the LocalValue constructor.

In the second case the left-hand-side of the record field pattern should be resolved to a global symbol that refers to the record selector.

nomeata commented 7 years ago

This should allow you to figure out that the local value "field" came from a record pattern wildcard.

Fair enough. I guess I was hoping to not have to keep track of any kind of environment for this renaming (which would allow me to do the renaming using a simple generic traversal instead of manually going through the AST), but maybe that is expecting too much.

In the second case the left-hand-side of the record field pattern should be resolved to a global symbol that refers to the record selector.

Right – so that is a bug, isn’t it?

phischu commented 7 years ago

Right – so that is a bug, isn’t it?

Yes, should be fixed by #89 which also fixes the other bug you encountered.

I am thinking about adding a field to LocalValue but it feels a little ad-hoc.

In the meantime I can offer you two workarounds:

  1. Rename local values too.
data RecordSelector_Record a = RecordSelector_Record { RecordSelector_field :: a }
RecordSelector_unRecord RecordSelector_r = RecordSelector_field
  where RecordSelector_Record { .. } = RecordSelector_r
  1. Desugar record wildcards and puns.
data RecordSelector_Record a = RecordSelector_Record { RecordSelector_field :: a }
RecordSelector_unRecord r = field
  where RecordSelector_Record { RecordSelector_field = field } = r

P.S.: Last time I tried to merge Haskell programs into one big module I got stuck because the TypeFamilies extension broke code from another module.

nomeata commented 7 years ago
  1. Rename local values too.

Well, to do so in my case I need to know what module the Record is defined in, so that the names of the renamed local values match that.

  1. Desugar record wildcards and puns.

That might be the way to go. I did that by hand for now.

P.S.: Last time I tried to merge Haskell programs into one big module I got stuck because the TypeFamilies extension broke code from another module.

Luckily the itch I was scratching does not use TypeFamilies.