Skip to content

Refactor some cruft in TcDerivInfer.inferConstraints

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

The latest installment in my quest to clean up the code in TcDeriv*. This time, my sights are set on TcDerivInfer.inferConstraints, which infers the context for derived instances. This function is a wee bit awkward at the moment:

  • It's not terribly obvious from a quick glance, but inferConstraints is only ever invoked when using the stock or anyclass deriving strategies, as the code for inferring the context for newtype- or via-derived instances is located separately in mk_coerce_based_eqn. But there's no good reason for things to be this way, so I moved this code from mk_coerce_based_eqn to inferConstraints so that everything related to inferring instance contexts is located in one place.
  • In this process, I discovered that the Haddocks for the auxiliary function inferConstraintsDataConArgs are completely wrong. It claims that it handles both stock and newtype deriving, but this is completely wrong, as discussed above—it only handles stock. To rectify this, I renamed this function to inferConstraintsStock to reflect its actual purpose and created a new inferConstraintsCoerceBased function to specifically handle newtype (and via) deriving.

Doing this revealed some opportunities for further simplification:

  • Removing the context-inference–related code from mk_coerce_based_eqn made me realize that the overall structure of the function is basically identical to mk_originative_eqn. In fact, I was easily able to combine the two functions into a single mk_eqn_from_mechanism function.

    As part of this merger, I now invoke atf_coerce_based_error_checks from doDerivInstErrorChecks1.

  • I discovered that GHC defined this function:

    typeToTypeKind = liftedTypeKind `mkVisFunTy` liftedTypeKind

    No fewer than four times in different modules. I consolidated all of these definitions in a single location in TysWiredIn.

Merge request reports