Skip to content

Draft: Reject puns in T2T (#24153)

Vladislav Zavialov requested to merge wip/int-index/t2t-expr-no-puns into master

This patch contains pun-related changes extracted from !11166 (closed).

I'm opening this MR (marked as Draft) so that the changes don't get lost, but I haven't properly addressed Simon's comments, cited below

Simon: !11166 (comment 533054)

I don't think we need this complexity. Not only is there a new data type, a new field in every HsVar, but even a new type class -- sigh!!

Fortunately I don't think need all this.

During typechecking we have tcl_rdr :: LocalRdrEnv, which gives the renamer env. We can easily check for punning here, rather than doing so in the renamer.

Not only is that simpler, but it's also more efficient: only happens in T2T rather than all the time.

Vlad: !11166 (comment 533098)

Right, this is how I attempted to implement it initially. Unfortunately, I no longer think it's possible. We need an RdrName, not a Name, because we want to distinguish qualified and unqualified occurrences. Compare

  1. f (Just @a a) = vfun a -- pun in T2T
  2. f (Just @a a) = vfun OtherMod.a -- no pun in T2T

In the first case, we want to report a pun because it's unclear which a is referred to by vfun a. In the second case, there's no pun, as OtherMod.a refers to something else entirely (some a coming from another module)

We can easily check this in the renamer, where we have an RdrName, by using promoteRdrName and looking up the promoted name in the environment

promoteRdrName :: RdrName -> Maybe RdrName
promoteRdrName (Unqual occ) = fmap Unqual (promoteOccName occ)
promoteRdrName (Qual m occ) = fmap (Qual m) (promoteOccName occ)
promoteRdrName (Orig _ _) = Nothing
promoteRdrName (Exact _)  = Nothing

As you can see, Unqual is promoted to Unqual and Qual is promoted to Qual, so we can't mix those up. Let's consider our cases (1) and (2) again

  1. f (Just @a a) = vfun a Here we have Unqual (OccName VarName "a"), which is promoted to Unqual (OccName TvName "a"). The promoted name can also be found in the environment, so we report a pun
  2. f (Just @a a) = vfun OtherMod.a Here we have Qual "OtherMod" (OccName VarName "a"), which is promoted to Qual "OtherMod" (OccName TvName "a"). The promoted name is looked up in OtherMod, so there is no pun.

Now imagine we try to do the same thing in the type checker, but we only have the resolved Name. How can we promote its namespace? I tried the nameRdrName helper, only to discover that it simply wraps the name as Exact.

Simon: !11166 (comment 533617)

I thought about it a bit in the interim. (You made some good points in your replies which are now invisible, sadly. I hope not lost.)

My main suggestion. Why not do this:

type instance XVar (GhcPass GhcRn) = RdrName

That is, with each occurrence, record the original RdrName, alongside the Name that the renamer has resolved it to.

  • The RdrName is easily to hand (obviously)
  • It would support more accurate pretty-printing of the output of the renamer (or typechecker actually); e.g. whether it is qualified.
  • In T2T you would have the info you needed
  • You could even have type instance XVar (GhcPass _) = RdrName: same regardless of which pass.

That seems less ad-hoc than your earlier version. No type class I hope!

Merge request reports