Skip to content

Refactor tcDeriving to generate tyfam insts before any bindings

Ryan Scott requested to merge wip/deriving-refactor into master

Previously, there was an awful hack in genInst (now called genInstBinds after this patch) where we had to return a continutation rather than directly returning the bindings for a derived instance. This was done for staging purposes, as we had to first infer the instance contexts for derived instances and then feed these contexts into the continuations to ensure the generated instance bindings had accurate instance contexts. Note [Staging of tcDeriving] in GHC.Tc.Deriving described this confusing state of affairs.

The root cause of this confusing design was the fact that genInst was trying to generate instance bindings and associated type family instances for derived instances simultaneously. This really isn't possible, however: as Note [Staging of tcDeriving] explains, one needs to have access to the associated type family instances before one can properly infer the instance contexts for derived instances. The use of continuation-returning style was an attempt to circumvent this dependency, but it did so in an awkward way.

This patch detangles this awkwardness by splitting up genInst into two functions: genFamInsts (for associated type family instances) and genInstBinds (for instance bindings). Now, the tcDeriving function calls genFamInsts and brings all the family instances into scope before calling genInstBinds. This removes the need for the awkward continuation-returning style seen in the previous version of genInst, making the code easier to understand.

There are some knock-on changes as well:

  1. hasStockDeriving now needs to return two separate functions: one that describes how to generate family instances for a stock-derived instance, and another that describes how to generate the instance bindings. I factored out this pattern into a new StockGenFns data type.
  2. While documenting StockGenFns, I realized that there was some inconsistency regarding which StockGenFns functions needed which arguments. In particular, the function in GHC.Tc.Deriv.Generics which generates Rep(1) instances did not take a SrcSpan like other gen_* functions did, and it included an extra [Type] argument that was entirely redundant. As a consequence, I refactored the code in GHC.Tc.Deriv.Generics to more closely resemble other gen_* functions. A happy result of all this is that all StockGenFns functions now take exactly the same arguments, which makes everything more uniform.

This is purely a refactoring that should not have any effect on user-observable behavior. The new design paves the way for an eventual fix for #20719 (closed).

Merge request reports