Record full FieldLabel in ifConFields.
ClosedPublic
Actions

Authored by ezyang on Feb 22 2017, 10:49 PM.

Details

Reviewers

austin
adamgundry
bgamari

Commits

rGHC9d17028fbcec: Record full FieldLabel in ifConFields.
rGHCDIFF9d17028fbcec: Record full FieldLabel in ifConFields.

Summary

The previous implementation tried to be "efficient" by
storing field names once in IfaceConDecls, and only just
enough information for us to reconstruct the FieldLabel.
But this came at a bit of code complexity cost.

This patch undos the optimization, instead storing a full
FieldLabel at each data constructor. Consequently, this fixes
bugs Trac #12699 and Trac #13250.

Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>

Test Plan

validate

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.

ezyang created this revision.Feb 22 2017, 10:49 PM

Herald added a reviewer: austin. · View Herald TranscriptFeb 22 2017, 10:49 PM

Herald added a subscriber: thomie. · View Herald Transcript

warning squash

Thanks for this! I agree that it makes sense to do away with the premature "optimization" here.

compiler/iface/IfaceSyn.hs
212	Do we actually need the boolean any more? As far as I can see, it was needed only for `ifaceConDeclFields`, which has now gone away.

ezyang added inline comments.Feb 23 2017, 3:50 AM

compiler/iface/IfaceSyn.hs
212	You're right! Nice catch.

remove Bool

Harbormaster failed remote builds in B14078: Diff 11246!Feb 23 2017, 3:57 AM

Closed by commit rGHC9d17028fbcec: Record full FieldLabel in ifConFields. (authored by ezyang). · Explain WhyFeb 24 2017, 4:53 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

		Path
M		compiler/backpack/RnModIface.hs (13 lines)
M		compiler/iface/IfaceSyn.hs (75 lines)
M		compiler/iface/MkIface.hs (15 lines)
M		compiler/iface/TcIface.hs (24 lines)
A	M	testsuite/tests/backpack/should_compile/T13250.bkp (8 lines)
A	M	testsuite/tests/backpack/should_compile/T13250.stderr (10 lines)
M		testsuite/tests/backpack/should_compile/all.T (1 line)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	11240	5841574		Feb 22 2017, 10:49 PM	★	★
Diff 2	11241	5841574	warning squash	Feb 22 2017, 11:53 PM	★	★
Diff 3	11246	5841574	remove Bool	Feb 23 2017, 3:57 AM	★	★
Diff 4	11308	93ffcb0	rGHC9d17028fbcecb53480598c4fcc7bd9e71b2ac7cf	Feb 24 2017, 4:52 PM	★	★