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.
IdP (NoGhcTc p)
in HsMatchCtxt
( !9624 (closed) )
Approach 0 (mpickering): Use 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.
mc_fun
instead of IdP
Approach 1: an new type family for 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 haveRdrName
s in them,GhcRn
things haveName
s in them andGhcTc
things haveId
s 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 printCtxIdP
we need to know that it's outputable. This results in a bunch of extraOutputableBndr
constraints everywhere.
SDoc
in mc_fun
Approach 2: 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
IdP
in mc_fun
and work around having Id
there for GhcTc
Approach 3 (current approach): Keep 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.