Skip to content

Add a bunch of NFData instances#2817

Merged
hdgarrood merged 1 commit into
masterfrom
add-generic-nfdata-instances
Apr 8, 2017
Merged

Add a bunch of NFData instances#2817
hdgarrood merged 1 commit into
masterfrom
add-generic-nfdata-instances

Conversation

@hdgarrood

Copy link
Copy Markdown
Contributor

I also added derived Generic instances in order to be able to define
the NFData instances without having to write the necessary code by hand;
I expect I'll do it incorrectly if I try to do it by hand.

I am mainly doing this because I want to use it to help diagnose bugs
like #2772 but I also think it might come in handy in real code at some
point too; e.g. if we ever want to store these types in Pursuit's
database.

I also added derived Generic instances in order to be able to define
the NFData instances without having to write the necessary code by hand;
I expect I'll do it incorrectly if I try to do it by hand.

I am mainly doing this because I want to use it to help diagnose bugs
like #2772 but I also think it might come in handy in real code at some
point too; e.g. if we ever want to store these types in Pursuit's
database.

@paf31 paf31 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. FYI, you can add NFData to the deriving clause if you use -XDeriveAnyClass.

@hdgarrood

Copy link
Copy Markdown
Contributor Author

Thanks! I opted to do it this way because apparently DeriveAnyClass doesn't work great together with GeneralizedNewtypeDeriving (which we use in one or two places IIRC).

@paf31

paf31 commented Apr 8, 2017

Copy link
Copy Markdown
Contributor

Oh good point.

@hdgarrood hdgarrood merged commit 6dc0e2e into master Apr 8, 2017
@hdgarrood hdgarrood deleted the add-generic-nfdata-instances branch April 8, 2017 22:51
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.

2 participants