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

Consider using Data.Functor.Identity #70

Closed
treeowl opened this issue Nov 18, 2014 · 4 comments
Closed

Consider using Data.Functor.Identity #70

treeowl opened this issue Nov 18, 2014 · 4 comments

Comments

@treeowl
Copy link
Contributor

treeowl commented Nov 18, 2014

Data.Sequence, at least, uses its own Id type. This looks like it's about the same as Identity from Data.Functor.Identity in base. Is there a reason not to use that? If not, I can give it a whirl.

@foxik
Copy link
Contributor

foxik commented Nov 18, 2014

Containers should work with older GHCs (Data.Functor.Identity will be added only to base-4.8, if I look correctly), and also with other Haskell compilers (it used to be Haskell-98 with CPP and compiler-specific stuff, today probably Haskell-2010 with CPP is the right target). Therefore, the Sequence-specific Id has to stay for older GHC and non-GHC compilers.

I am not sure whether it would be worth it to use type Id = Data.Functor.Identity for GHC base-4.8 -- is there any advantage in using Data.Functor.Identity then the Functor/Monad/Applicative instances?

@treeowl
Copy link
Contributor Author

treeowl commented Nov 18, 2014

It looks like base-4.8 is actually stealing Data.Functor.Identity away from transformers. If containers is allowed to depend on transformers for older versions, then we can use Data.Functor.Identity regardless, and get all that code out of Data.Sequence. The version in base-4.8 will have some coercion-based optimizations (e.g., fmap = coerce, (<*>) = coerce, foldMap = coerce, foldl = coerce), but I don't know if any of those will help anything in Data.Sequence—they only matter when certain sorts of polymorphism are in play.

@foxik
Copy link
Contributor

foxik commented Nov 18, 2014

As containers are a boot package (https://ghc.haskell.org/trac/ghc/wiki/Commentary/Libraries), any dependency is a big burden, so adding transformers for saving 10 lines of code is definitely not going to happen.

Nevertheless, if using Data.Functor.Identity on new GHCs should bring some advantage (some speedup?), we could use it, while providing Data.Sequence.Id for other compilers.

@treeowl
Copy link
Contributor Author

treeowl commented Nov 21, 2014

Fixed by #76

@treeowl treeowl closed this as completed Nov 21, 2014
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

No branches or pull requests

2 participants