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

Expose NFData instance for Down on earlier versions of base #28

Merged
merged 1 commit into from
Apr 16, 2017
Merged

Conversation

RyanGlScott
Copy link
Member

Before Down was migrated to Data.Ord in base-4.6.0, it lived in GHC.Exts. This means we can expose the NFData instance for Down unconditionally.

(Progress towards #27).

@RyanGlScott
Copy link
Member Author

Arg, I forgot about the fact that GHC.Exts... isn't Safe.

@hvr, what's the best way to handle this? Should we conditionally make Control.DeepSeq -XTrustworthy? Something else?

@hvr
Copy link
Member

hvr commented Apr 6, 2017

@RyanGlScott that's quite bad... I went great lengths to ensure we can be unconditionalle {-# LANGUAGE Safe #-} and deliberately avoid picking up instances that would force us to regress to Trustworthy... :-(

@RyanGlScott
Copy link
Member Author

Well, we have to pick something to support conditionally: either we only provide an NFData Down instance on base-4.6 or later only, or we only mark the module as Safe on base-4.6 or later.

I can live with the former option, though.

@hvr
Copy link
Member

hvr commented Apr 7, 2017

@RyanGlScott there's also the option to just drop support for base<4.6 which I've been considering for some time now. At some point we'll have to reduce the compat window anyway. And if it helps to avoid some CPP API conditionallity now might be the time. The additional amount of CPP (which I was planning to get rid of as soon as I had the chance) is the one thing that's been most annoying to me about NFData1/2 :-(

@hvr hvr merged commit 0792252 into master Apr 16, 2017
hvr added a commit that referenced this pull request Apr 16, 2017
hvr added a commit that referenced this pull request Apr 16, 2017
This was made necessary by #28

This hack reduces the surface-area requiring TRUSTWORTHY annotations
@hvr hvr deleted the down branch April 16, 2017 14:12
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.

None yet

2 participants