Skip to content

Instantiate field types properly in stock-derived instances

Ryan Scott requested to merge wip/T20375-T20387 into master

This MR consists of three commits. The first two commits are preparatory refactorings that pave the way for the third commit (Instantiate field types properly in stock-derived instances), which is the main thrust of the MR. I've reproduced the commit message for this commit below:

Previously, the deriving machinery was very loosey-goosey about how it used the types of data constructor fields when generating code. It would usually just consult dataConOrigArgTys, which returns the uninstantiated field types of each data constructor. Usually, you can get away with this, but issues #20375 (closed) and #20387 (closed) revealed circumstances where this approach fails.

Instead, when generated code for a stock-derived instance C (T arg_1 ... arg_n), one must take care to instantiate the field types of each data constructor with arg_1 ... arg_n. The particulars of how this is accomplished is described in the new Note [Instantiating field types in stock deriving] in GHC.Tc.Deriv.Generate. Some highlights:

  • DerivInstTys now has a new dit_dc_inst_arg_env :: DataConEnv [Type] field that caches the instantiated field types of each data constructor. Whenever we need to consult the field types somewhere in GHC.Tc.Deriv.* we avoid using dataConOrigArgTys and instead look it up in dit_dc_inst_arg_env.
  • Because DerivInstTys now stores the instantiated field types of each constructor, some of the details of the GHC.Tc.Deriv.Generics.mkBindsRep function were able to be simplified. In particular, we no longer need to apply a substitution to instantiate the field types in a Rep(1) instance, as that is already done for us by DerivInstTys. We still need a substitution to implement the "wrinkle" section of Note [Generating a correctly typed Rep instance], but the code is nevertheless much simpler than before.
  • The tyConInstArgTys function has been removed in favor of the new GHC.Core.DataCon.dataConInstUnivs function, which is really the proper tool for the job. dataConInstUnivs is much like tyConInstArgTys except that it takes a data constructor, not a type constructor, as an argument, and it adds extra universal type variables from that data constructor at the end of the returned list if need be. dataConInstUnivs takes care to instantiate the kinds of the universal type variables at the end, thereby avoiding a bug in tyConInstArgTys discovered in #20387 (closed) (comment 377037).

Fixes #20375 (closed). Fixes #20387 (closed).

Merge request reports