-
Notifications
You must be signed in to change notification settings - Fork 58
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
Add Lift instances #332
Add Lift instances #332
Conversation
29d0ca7
to
4c850a6
Compare
Data/Primitive/Array.hs
Outdated
-- of the list does not match the given length, this makes demons fly | ||
-- out of your nose. We use it in the 'Lift' instance. If you edit the | ||
-- splice and break it, you get to keep both pieces. | ||
unsafeArrayFromListN' :: Int -> [a] -> Array a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would happen if we wrote this as a fold? Would GHC be able to make good decisions about when to build it as a list and convert it to an array vs. building it directly as an array (poke poke poke)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can check this tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I make the fold INLINABLE
, it doesn't actually seem to do anything. If I make it INLINE
, I get pokes up to around 32 elements, which suggests some judgement has been used. Unfortunately, going bigger seems to lead to some useless code duplication. So we might be better off just choosing our own cutoff, whatever that should be (currently set to 1, essentially). Another reasonable option would be to just leave it as I have it right now and push GHC for array literals or something.
Closes haskell#331
Pinning an array the user wants unpinned is much less likely to break code than failing to pin one the user wants pinned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!
Thanks, @phadej! I'll merge. |
Closes #331