HsMatchContext GhcTc story

HsMatchContext has FunRhs constructor with mc_fun field which is only used for pretty printing (and as it turned out, in HIE). Since removal of NoGhcTc from that field, we now have to deal with having IdP there which introduces several complications.

Approach 0 (mpickering): Use IdP (NoGhcTc p) in HsMatchCtxt( !9624 (closed) )

If we accept the existing of NoGhcTc then the simplest implementation is to replace the mc_fun field in HsMatchContext with IdP (NoGhcTc p) so that the field contains a RdrName during parsing and a Name as context.

This is precisely the right approach to take because the "Context" is something which we define, and in the typechecker the context can't be an Id, precisely because we haven't constructed that Id yet when we need to provide the context for which we will construct the Id.

Approach 1: an new type family for mc_fun instead of IdP

Instead of having IdP in mc_fun we have CtxIdP type family which is defined like this:

type instance CtxIdP (GhcPass p) = CtxIdGhcP p

type family CtxIdGhcP p where
  CtxIdGhcP 'Parsed      = RdrName
  CtxIdGhcP 'Renamed     = Name
  CtxIdGhcP 'Typechecked = Name

This approach was deemed unsatisfactory because:

• This is basically a local reimplementation of NoGhcTc. We lose the consistency of "GhcPs things have RdrNames in them, GhcRn things have Names in them and GhcTc things have Ids in them".
• This results in a fairly invasive change because we change the inner structure of a data type here and also because of OutputableBndr. In order to pretty print CtxIdP we need to know that it's outputable. This results in a bunch of extra OutputableBndr constraints everywhere.

Approach 2: SDoc in mc_fun

Since mc_fun is only used for pretty printing, and having to add a lot of new Outputable constraints everywhere is annoying, why not just put SDoc there? This would let us to remove pass parametrization from HsMatchContext and HsStmtContext completely.

Well, turns out, there is another obstacle: HIE. We need Name for a ToHie (HsMatchContext p) instance. Me, @int-index and @simonpj talked about this and concluded that we don't know enough about HIE to determine whether it would break things to somehow avoid having that instance at all or deal with this some other way.

One thing that's known for sure is that it is used by Haddock one way or another, because using Id there resulted in some broken Haddock tests: https://gitlab.haskell.org/hithroc/ghc/-/jobs/764890#L4106

Approach 3 (current approach): Keep IdP in mc_fun and work around having Id there for GhcTc

This turned out to be the least invasive approach, yet not without its drawbacks.

The main problem of this approach lies within tcMonoBinds. tcMatchesFun needs to put an Id in mc_fun, but in tcMonoBinds we don't get the mono_id until after we have rhs_ty which we get in tcMatchesFun. This was solved by using fixM and making a thunk of rhs_ty to put into mono_id and then mono_id is passed to tcMatchesFun which then fills in that thunk.

The drawback is that fixM code is fragile, we rely on the fact that tcMatchesFun doesn't look at varType of mono_id. This might result in some fun debugging experience for someone who accidentally changes some code and inspects that value. Dancing around black holes as @rae calls it I believe (and I'm really fond of this metaphor)

Another thing of note that having fixM here should let us get rid of TcIdBndr_ExpType, however that turned out to be problematic, because TcIdBndr_ExpType is used in zonkTidyTcLclEnvs to determine whether to zonk or not and without that it always zonks, which results in inspection of the value and an infinite loop.

Relevant merge request: !5579 (closed)

What is the best way forward?

All 3 approaches seem unsatisfactory. Seems like Approach 2 is the closest to being ideal: we don't want to introduce something like CtxIdP and we don't want fixM. Approach 2 achieves that if someone figures out how to deal with HIE.