Handle DuplicateRecordFields correctly in filterImports (fixes #14487)
ClosedPublic

Authored by adamgundry on Jun 7 2018, 6:05 AM.

Details

Summary

filterImports needed a small adjustment to correctly handle record field definitions arising from modules with DuplicateRecordFields enabled.

Previously hiding fields was not possible with DuplicateRecordFields enabled.

Test Plan

new test rename/should_compile/T14487

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 created this revision.Jun 7 2018, 6:05 AM
bgamari requested changes to this revision.Jun 7 2018, 9:43 AM

Looks reasonable to me, although a small trail of breadcrumbs back to the ticket would be nice.

compiler/rename/RnNames.hs
886

Can we have a comment including a reference to the ticket here?

This revision now requires changes to proceed.Jun 7 2018, 9:43 AM
bgamari edited the summary of this revision. (Show Details)Jun 7 2018, 9:44 AM
adamgundry updated this revision to Diff 16789.Jun 8 2018, 4:30 AM
  • Add a comment [skip ci]

Suggested refactor

compiler/rename/RnNames.hs
893

This same pattern is used in TcRnExports.check_occs.

Let's define a function in Avail.hs

availsNamesWithOccs :: [AvailInfo] -> [(Name, OccName)]
availsNamesWithOccs avails = concatMap AvailNamesWithOccs

availNamesWithOccs :: AvailInfo -> [(Name, OccName)]
availNamesWithOccs (Avail n) = [(n, nameOccName n)]
availNamesWithOccs (AvailTc n ns fs)
  = [ (n, nameOccName n) | n <- ns ] ++
     [ (flSelector fl, mkVarOccFS (flLabel fl)) | fl <- fs ]

Now we can use that function from both places.

Needs a good comment with an example!

bgamari requested changes to this revision.Jun 15 2018, 1:12 PM

Requesting changes in light of the suggested refactor.

This revision now requires changes to proceed.Jun 15 2018, 1:12 PM
adamgundry updated this revision to Diff 16945.Jun 15 2018, 3:59 PM
  • Refactor by adding avail[s]NamesWithOccs
  • Don't show mangled selector names in -Wduplicate-exports

Thanks for the feedback. I've made the suggested refactor and fixed a tiny bug I spotted along the way.

bgamari accepted this revision.Jun 17 2018, 10:17 AM

Looks good, thanks!

This revision is now accepted and ready to land.Jun 17 2018, 10:17 AM
This revision was automatically updated to reflect the committed changes.