Don't drop arguments in TH type arguments
ClosedPublic

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

Details

Summary

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

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
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.

compiler/hsSyn/Convert.hs
1391

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

1422

Here's another.

1429

And another.

RyanGlScott added inline comments.Oct 4 2018, 5:14 PM
compiler/hsSyn/Convert.hs
1391

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.