Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix declareFieldLabels or declareLenses with DuplicateRecordFields #328

Merged
merged 1 commit into from
Jul 27, 2020

Conversation

adamgundry
Copy link
Member

Fixes #323.

@adamgundry adamgundry requested a review from arybczak July 20, 2020 15:39
where
-- When DuplicateRecordFields is enabled, reified datatypes contain
-- "mangled" field names that look like $sel:foo:MkT where foo is the field
-- name and MkT is the first data constructor. We strip off the prefix and
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is MkT always the first constructor, even in cases like data T = MkT1 { name1 :: String } | MkT2 { name2 :: String } name2 will be mangled as $sel:name2:mkT1?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's just the first constructor in the type, regardless of whether the field occurs or not. I've tweaked the test to check this, and cleaned up the code a bit. In particular it now checks that the suffix is the expected one, which should lead to easier to understand errors if GHC's behaviour should ever change.

@adamgundry adamgundry force-pushed the 323-drf-bug branch 2 times, most recently from d73a1c0 to 4e0339e Compare July 21, 2020 21:46
{-# LANGUAGE TypeFamilies #-}
{-# LANGUAGE TypeOperators #-}
{-# LANGUAGE UndecidableInstances #-}
{-# OPTIONS_GHC -fforce-recomp #-}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I left that in by mistake. For some reason changing the TH code wasn't leading to recompilation when I was testing. Fixed.

Copy link
Collaborator

@arybczak arybczak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks :)

@arybczak arybczak merged commit 6ae192d into master Jul 27, 2020
@arybczak arybczak deleted the 323-drf-bug branch July 27, 2020 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

declareFieldLabels generates bogus Labels in presence of DuplicateRecordFields
2 participants