Fix egregious duplication of vars in RnTypes

Authored by RyanGlScott on Aug 30 2017, 2:38 PM.



RnTypes contains a fairly intricate algorithm to extract
the kind and type variables of an HsType. This algorithm carefully
maintains the separation between type variables and kind variables
so that the difference between -XPolyKinds and -XTypeInType can
be respected.

But after doing all this, rmDupsInRdrTyVars stupidly just
concatenated the lists of type and kind variables at the end. If a
variable were used as both a type and a kind, the algorithm would
produce *both*! This led to all kinds of problems, including Trac #13988.

This is mostly Richard Eisenberg's patch. The only original
contribution I made was adapting call sites of rnImplicitBndrs to
work with the new definition of rmDupsInRdrTyVars. That is,
rnImplicitBndrs checks for variables that are illegally used in
both type and kind positions without using -XTypeInType, but in
order to check this, one cannot have filtered duplicate variables out
before passing them to rnImplicitBndrs. To accommodate for this, I
needed to concoct variations on the existing extract- functions in
RnTypes which do not remove duplicates, and use those near
rnImplicitBndrs call sites.

test case: ghci/scripts/T13988

Test Plan

make test TEST=T13988

Diff Detail

rGHC Glasgow Haskell Compiler
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
RyanGlScott created this revision.Aug 30 2017, 2:38 PM
goldfire accepted this revision.Aug 30 2017, 4:19 PM

LGTM. Thanks!

This revision is now accepted and ready to land.Aug 30 2017, 4:19 PM
simonpj accepted this revision.Aug 31 2017, 4:25 AM

Some comment-y suggesitnos.


I think it'd help to have type synonyms FreeKiTyVarsWithDups and FreeKiTyVarsNoDups, and use them.


Where is the "preferring kind vars" implemented. Say so here. Perhaps in rmDupsInRdrTyVars?


What are the different consequences of considering it to be "kind-variable-like" or "type-variable-like"? Where in the code are those consequences implemented?

RyanGlScott marked 3 inline comments as done.
  • Polish some traceRns
  • Address @simonpj's comments

Done. I didn't replace every usage of FreeKiTyVars with one of these two synonyms, but I did put them where it mattered (e.g., rnImplicitBndrs, rmDupsInRdrTyVars, extract*Dups, etc.).


I added this to the Haddocks for rmDupsInRdrTyVars.

This revision was automatically updated to reflect the committed changes.