Don't drop arguments in TH type arguments

Authored by harpocrates on Sep 30 2018, 9:25 AM.



When converting from TH AST back to HsType, we were occasionally
dropping type arguments. This resulted in incorrectly accepted programs
as well as incorrectly rejected programs.

Test Plan

make TEST=T15360a && make TEST=T15360b

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.
harpocrates created this revision.Sep 30 2018, 9:25 AM
tdammers accepted this revision.Oct 1 2018, 6:50 AM
This revision is now accepted and ready to land.Oct 1 2018, 6:50 AM

Hm... this does work, although it requires manually putting mk_apps at the end of every case (at least, almost every case), which is quite easy to forget. I wonder if we should instead factor out the mk_apps part so that those who add new functionality to cvtTypeKind are less likely to mess it up?

bgamari accepted this revision.Oct 4 2018, 12:47 PM

I'm fine with the approach for now. I agree that it may be a bit easy to mess up, but we can always refactor later.


There are a few cases like this that make @RyanGlScott's suggested refactoring tricky.


Here's another.


And another.

RyanGlScott added inline comments.Oct 4 2018, 5:14 PM

Indeed, I managed to overlook this. I suppose we'd need some CPS-style logic to implement this robustly, and I don't want to derail this Diff by demanding a complete rewrite of cvtTypeKind.

This revision was automatically updated to reflect the committed changes.