Reify DuplicateRecordFields by label, rather than by selector
ClosedPublic

Authored by adamgundry on Dec 7 2015, 11:36 AM.

Details

Summary

See Note [Reifying field labels] in TcSplice. This makes
typical uses of TH work better with DuplicateRecordFields.
If reify is called on the Name of a field label produced by
the output of a previous reify, and there are multiple fields
with that label defined in the same module, it may fail with
an ambiguity error.

Test Plan

Added tests, and manually tested that this makes
Aeson's deriveJSON avoid the $sel: prefixes.

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.
adamgundry retitled this revision from to Reify DuplicateRecordFields by label, rather than by selector.Dec 7 2015, 11:36 AM
adamgundry updated this object.
adamgundry edited the test plan for this revision. (Show Details)
adamgundry added reviewers: austin, bgamari, simonpj.
adamgundry updated the Trac tickets for this revision.
bgamari requested changes to this revision.Dec 12 2015, 10:11 AM

We should probably add a note to reify's Haddocks noting that field names cannot be reified when DuplicateRecordFields is enabled.

This revision now requires changes to proceed.Dec 12 2015, 10:11 AM
adamgundry updated this revision to Diff 5716.Dec 15 2015, 7:04 AM
  • Merge remote-tracking branch 'origin/master' into wip/T11103
  • Implement alternative version of reifyFieldLabel
adamgundry updated this object.Dec 15 2015, 7:07 AM

I've revised the implementation because I realised we can do slightly better. Now the potential error arises only in a very obscure set of circumstances (see T11103.hs for an example).

We should probably add a note to reify's Haddocks noting that field names cannot be reified when DuplicateRecordFields is enabled.

Field names usually can be reified when the extension is enabled, it's just that field names output by reify might be ambiguous if they are reified again (and DuplicateRecordFields was used on the defining module). My feeling is that this is a sufficiently obscure corner that it may not be worth documenting in reify's Haddocks. After all, the name really is ambiguous, so arguably the error message is (now) correct.

bgamari accepted this revision.Dec 15 2015, 8:59 AM

Alright, that sounds reasonable enough.

This revision is now accepted and ready to land.Dec 15 2015, 8:59 AM
This revision was automatically updated to reflect the committed changes.