Read COMPLETE sets from external packages
ClosedPublic

Authored by RyanGlScott on Mar 1 2017, 9:07 AM.

Details

Summary

Currently, COMPLETE pragmas are not read from external packages at
all, which quite limits their usefulness. This extends ExternalPackageState
to include COMPLETE sets from other packages, and plumbs around the
appropriate values to make it work the way you'd expect it to.

Requires a binary submodule update.

Fixes Trac #13350.

Test Plan

make test TEST=T13350

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.
RyanGlScott created this revision.Mar 1 2017, 9:07 AM
simonpj accepted this revision.Mar 1 2017, 5:28 PM
simonpj added a subscriber: simonpj.

More comments; but otherwise looks good to me.

compiler/main/HscTypes.hs
3025

This would be a great place for a Note, outlining the way COMPLETE works, with some examples, and poitners to the moving parts.

3025–3030

competeMatchTyCon would be better

3037

Keyed by the completeMatchType field., i.e. the TyCon.

This revision is now accepted and ready to land.Mar 1 2017, 5:28 PM
RyanGlScott planned changes to this revision.Mar 1 2017, 5:32 PM

Thanks @simonpj, I'll update the comments once D3245 (on which this depends, and which also modifies the functionality of COMPLETE sets) is merged.

compiler/main/HscTypes.hs
3025

@rwbarton, do you want to document this as a part of D3245, or should I do it here instead?

3025–3030

Same question.

RyanGlScott marked 5 inline comments as done.
  • Fix the build, add a Note per simonpj's request
This revision is now accepted and ready to land.Mar 2 2017, 11:10 AM
bgamari requested changes to this revision.Mar 2 2017, 3:35 PM

Sadly it seems this already needs to be rebased.

This revision now requires changes to proceed.Mar 2 2017, 3:35 PM
RyanGlScott updated this revision to Diff 11505.Mar 2 2017, 5:28 PM
RyanGlScott edited edge metadata.

Rebase on top of master

RyanGlScott updated this revision to Diff 11521.Mar 2 2017, 7:13 PM

Rebase on top of master (again?!?)

  • Bump binary submodule
RyanGlScott edited the summary of this revision. (Show Details)Mar 2 2017, 10:36 PM

As observed here, this requires a binary submodule update. I've opened a PR against binary here: https://github.com/kolmodin/binary/pull/132

This revision was automatically updated to reflect the committed changes.

@bgamari: The required changes were merged into binary in https://github.com/kolmodin/binary/commit/0147456b11c38d1121fd84a2b53effefde111240, so reapplying this patch with binary bumped to that commit should work. Should I open another Diff, or should we just commit it directly?

@bgamari: The required changes were merged into binary in https://github.com/kolmodin/binary/commit/0147456b11c38d1121fd84a2b53effefde111240, so reapplying this patch with binary bumped to that commit should work. Should I open another Diff, or should we just commit it directly?

I'm merging this again currently.

RyanGlScott, I'm very sorry; it looks like arc decided that I deserve authorship of your patch. ☹

No worries! I'm just glad we finally landed the darn thing. :)