Skip to content

Simplify bindLHsTyVarBndrs and bindHsQTyVars

Ryan Scott requested to merge wip/simply-bind-tyvars into master

Both bindLHsTyVarBndrs and bindHsQTyVars take two separate Maybe arguments, which I find terribly confusing. Thankfully, it's possible to remove one Maybe argument from each of these functions, which this patch accomplishes:

  • bindHsQTyVars takes a Maybe SDoc argument, which is Just if GHC should warn about any of the quantified type variables going unused. However, every call site uses Nothing in practice. This makes sense, since it doesn't really make sense to warn about unused type variables bound by an LHsQTyVars. For instance, you wouldn't warn about the a in data Proxy a = Proxy going unused.

    As a result, I simply remove this Maybe SDoc argument altogether.

  • bindLHsTyVarBndrs also takes a Maybe SDoc argument for the same reasons that bindHsQTyVars took one. To make things more confusing, however, bindLHsTyVarBndrs also takes a separate HsDocContext argument, which is pretty-printed (to an SDoc) in warnings and error messages.

    In practice, the Maybe SDoc and the HsDocContext often contain the same text. See the call sites for bindLHsTyVarBndrs in rnFamInstEqn and rnConDecl, for instance. There are only a handful of call sites where the text differs between the Maybe SDoc and HsDocContext arguments:

    • In rnHsRuleDecl, where the Maybe SDoc says "In the rule" and the HsDocContext says "In the transformation rule".
    • In rnHsTyKi/rn_ty, where the Maybe SDoc says "In the type" but the HsDocContext is inhereted from the surrounding context (e.g., if rnHsTyKi were called on a top-level type signature, the HsDocContext would be "In the type signature" instead)

    In both cases, warnings/error messages arguably improve by unifying making the Maybe SDoc's text match that of the HsDocContext. As a result, I decided to remove the Maybe SDoc argument to bindLHsTyVarBndrs entirely and simply reuse the text from the HsDocContext.

    The Maybe SDoc argument has one other purpose: signaling when to emit "Unused quantified type variable" warnings. To recover this functionality, I replaced the Maybe SDoc argument with a boolean-like WarnUnusedForalls argument. The only bindLHsTyVarBndrs call site that chooses not to emit these warnings in bindHsQTyVars.

Edited by Ryan Scott

Merge request reports