Skip to content
  • Ryan Scott's avatar
    Use tcSplitForAllInvisTyVars (not tcSplitForAllTyVars) in more places · 645444af
    Ryan Scott authored and Marge Bot's avatar Marge Bot committed
    The use of `tcSplitForAllTyVars` in `tcDataFamInstHeader` was the immediate
    cause of #18939, and replacing it with a new `tcSplitForAllInvisTyVars`
    function (which behaves like `tcSplitForAllTyVars` but only splits invisible
    type variables) fixes the issue. However, this led me to realize that _most_
    uses of `tcSplitForAllTyVars` in GHC really ought to be
    `tcSplitForAllInvisTyVars` instead. While I was in town, I opted to replace
    most uses of `tcSplitForAllTys` with `tcSplitForAllTysInvis` to reduce the
    likelihood of such bugs in the future.
    
    I say "most uses" above since there is one notable place where we _do_ want
    to use `tcSplitForAllTyVars`: in `GHC.Tc.Validity.forAllTyErr`, which produces
    the "`Illegal polymorphic type`" error message if you try to use a higher-rank
    `forall` without having `RankNTypes` enabled. Here, we really do want to split
    all `forall`s, not just invisible ones, or we run the risk of giving an
    inaccurate error message in the newly added `T18939_Fail` test case.
    
    I debated at some length whether I wanted to name the new function
    `tcSplitForAllInvisTyVars` or `tcSplitForAllTyVarsInvisible`, but in the end,
    I decided that I liked the former better. For consistency's sake, I opted to
    rename the existing `splitPiTysInvisible` and `splitPiTysInvisibleN` functions
    to `splitInvisPiTys` and `splitPiTysInvisN`, respectively, so that they use the
    same naming convention. As a consequence, this ended up requiring a `haddock`
    submodule bump.
    
    Fixes #18939.
    645444af