Skip to content

Implement cloneArray and cloneMutableArray for GHC >= 7.2.1 #7

Merged
merged 3 commits into from Mar 11, 2014

3 participants

@jberryman

Let me know if supporting < 7.2.1 is important or if anything looks amiss.

@tibbe
Haskell member
tibbe commented Mar 10, 2014

I think we should support the last 3 major GHC releases (not counting 7.2, which was a tech preview). Once 7.8 is out, I think this is OK.

@jberryman

Cool. This should compile on < 7.2.1, it just won't export the new functions. For cloneMutableArray I think we could fall back to something like this. Let me know if you want to do something like that or make any other changes.

@tibbe
Haskell member
tibbe commented Mar 10, 2014

@jberryman we used to have APIs that conditionally exported functions like that. It unfortunately doesn't end up well (users end up reproducing your ifdefs). I think we should either make it work on < 7.2 or just say that < 7.2 isn't supported. The simplest thing for you might just be to write the 5 line fallback for other systems. You can get mine from here: https://github.com/tibbe/unordered-containers/blob/master/Data/HashMap/Array.hs#L348

@gregorycollins
Haskell member

Yes please add the slow-path fallback for old GHC versions, it'll keep us out of trouble.

@jberryman jberryman closed this Mar 11, 2014
@jberryman jberryman reopened this Mar 11, 2014
@jberryman

Let me know how that looks.

@gregorycollins gregorycollins commented on an outdated diff Mar 11, 2014
Data/Primitive/Array.hs
@@ -156,6 +159,49 @@ copyMutableArray !dst !doff !src !soff !len = go 0
| otherwise = return ()
#endif
+-- | Return a newly allocated Array with the specified subrange of the
+-- provided Array. The provided Array should contain the full subrange
+-- specified by the two Ints, but this is not checked.
+cloneArray :: Array a -- ^ source array
+ -> Int -- ^ offset into destination array
+ -> Int -- ^ number of elements to copy
+ -> Array a
+{-# INLINE cloneArray #-}
+#if __GLASGOW_HASKELL__ >= 721
@gregorycollins
Haskell member

The constant should be "702" -- yes, this is very unintuitive, and yes, I mess it up all the time too :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jberryman

@gregorycollins Thanks, I meant to double-check I was doing that right. Fixed now.

@gregorycollins gregorycollins merged commit c123d42 into haskell:master Mar 11, 2014

1 check passed

Details default The Travis CI build passed
@jberryman

Not sure how releases usually work, but I would love to be able to depend on 0.5.3.0 with this functionality in my own library soon. Thanks again!

@jberryman

Can this be released?

@tibbe
Haskell member
tibbe commented May 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.