search

haskell / base16-bytestring

Fast base16 (hexadecimal) encoding and decoding for Haskell bytestrings.
Other
27 stars 10 forks source link

Build failures with older GHCs and upcoming bytestring-0.11 #13

Closed phadej closed 3 years ago

phadej commented 4 years ago 
                           8.10.2  8.8.4  8.6.5  8.4.4  8.2.2  8.0.2  7.10.3  7.8.4  7.6.3  7.4.2  7.2.2  7.0.4
base16-bytestring-1.0.0.0  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP
base16-bytestring-0.1.1.7  OK      OK     OK     OK     OK     OK     FAIL    FAIL   FAIL   FAIL   FAIL   FAIL 
base16-bytestring-0.1.1.6  OK      OK     OK     OK     OK     OK     FAIL    FAIL   FAIL   FAIL   FAIL   FAIL 
base16-bytestring-0.1.1.5  OK      OK     OK     OK     OK     OK     FAIL    FAIL   FAIL   FAIL   FAIL   FAIL 
base16-bytestring-0.1.1.4  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP
base16-bytestring-0.1.1.3  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP
base16-bytestring-0.1.1.2  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP
base16-bytestring-0.1.1.1  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP
base16-bytestring-0.1.1.0  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP
base16-bytestring-0.1.0.0  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP  NO-IP   NO-IP  NO-IP  NO-IP  NO-IP  NO-IP

Please make revisions to 0.1.1.7, 0.1.1.6 and 0.1.1.5. Thank you in advance.

phadej commented 4 years ago

ping @emilypi, you doesn't seem to watch this repository.

emilypi commented 4 years ago

@phadej Thanks for the update. I can bump the bounds on 1.0.0.0, and see if i can make it work.

emilypi commented 4 years ago

It looks like it'll be a simple revision. I've added 

packages: .

source-repository-package
  type: git
  location: https://github.com/haskell/bytestring
  tag: 0.11.0.0-rc1

allow-newer:
  *:bytestring

To my cabal.project and it seems to build with no problems.

phadej commented 4 years ago

Versions

base16-bytestring-0.1.1.7  OK      OK     OK     OK     OK     OK     FAIL    FAIL   FAIL   FAIL   FAIL   FAIL 
base16-bytestring-0.1.1.6  OK      OK     OK     OK     OK     OK     FAIL    FAIL   FAIL   FAIL   FAIL   FAIL 
base16-bytestring-0.1.1.5  OK      OK     OK     OK     OK     OK     FAIL    FAIL   FAIL   FAIL   FAIL   FAIL
emilypi commented 4 years ago

@phadej okay, amended those versions to cap at <0.11. It sounds like we either need to drop support for GHC <8.0, or be happy with that bound forever. @hvr @23Skidoo what do you guys want to do here? I'm in favor of people using 1.0.0.0 as the last version of this library and moving to base16

phadej commented 4 years ago

It sounds like we either need to drop support for GHC <8.0

How so?

diff --git a/Data/ByteString/Base16.hs b/Data/ByteString/Base16.hs
index 67c74fe..a730f2d 100644
--- a/Data/ByteString/Base16.hs
+++ b/Data/ByteString/Base16.hs
@@ -47,14 +47,16 @@ import GHC.IO (unsafeDupablePerformIO)
 -- > encode "foo"  == "666f6f"
 --
 encode :: ByteString -> ByteString
-encode (PS sfp soff slen)
-    | slen > maxBound `div` 2 =
-      error "Data.ByteString.Base16.encode: input too long"
-    | otherwise = unsafeCreate (slen * 2) $ \dptr ->
-        withForeignPtr sfp $ \sptr ->
-          encodeLoop dptr
-          (sptr `plusPtr` soff)
-          (sptr `plusPtr` (soff + slen))
+encode bs = withBS bs aux
+  where
+    aux sfp slen
+        | slen > maxBound `div` 2 =
+          error "Data.ByteString.Base16.encode: input too long"
+        | otherwise = unsafeCreate (slen * 2) $ \dptr ->
+            withForeignPtr sfp $ \sptr ->
+              encodeLoop dptr
+              sptr
+              (sptr `plusPtr` slen)

 -- | Decode a base16-encoded 'ByteString' value.
 -- If errors are encountered during the decoding process,
@@ -70,19 +72,21 @@ encode (PS sfp soff slen)
 -- @since 1.0.0.0
 --
 decode :: ByteString -> Either String ByteString
-decode (PS sfp soff slen)
-    | slen == 0 = Right empty
-    | r /= 0 = Left "invalid bytestring size"
-    | otherwise = unsafeDupablePerformIO $ do
-      dfp <- mallocPlainForeignPtrBytes q
-      withForeignPtr dfp $ \dptr ->
-        withForeignPtr sfp $ \sptr ->
-          decodeLoop dfp dptr
-          (plusPtr sptr soff)
-          (plusPtr sptr (soff + slen))
+decode bs = withBS bs aux
   where
-    !q = slen `quot` 2
-    !r = slen `rem` 2
+    aux sfp slen
+        | slen == 0 = Right empty
+        | r /= 0 = Left "invalid bytestring size"
+        | otherwise = unsafeDupablePerformIO $ do
+          dfp <- mallocPlainForeignPtrBytes q
+          withForeignPtr dfp $ \dptr ->
+            withForeignPtr sfp $ \sptr ->
+              decodeLoop dfp dptr
+              sptr
+              (plusPtr sptr slen)
+      where
+        !q = slen `quot` 2
+        !r = slen `rem` 2

 -- | Decode a Base16-encoded 'ByteString' value leniently, using a
 -- strategy that never fails.
@@ -99,14 +103,16 @@ decode (PS sfp soff slen)
 -- @since 1.0.0.0
 --
 decodeLenient :: ByteString -> ByteString
-decodeLenient (PS !sfp !soff !slen)
-    | slen == 0 = empty
-    | otherwise = unsafeDupablePerformIO $ do
-      dfp <- mallocPlainForeignPtrBytes (q * 2)
-      withForeignPtr dfp $ \dptr ->
-        withForeignPtr sfp $ \sptr ->
-          lenientLoop dfp dptr
-          (plusPtr sptr soff)
-          (plusPtr sptr (soff + slen))
+decodeLenient bs = withBS bs aux
   where
-    !q = slen `quot` 2
+    aux !sfp !slen
+        | slen == 0 = empty
+        | otherwise = unsafeDupablePerformIO $ do
+          dfp <- mallocPlainForeignPtrBytes (q * 2)
+          withForeignPtr dfp $ \dptr ->
+            withForeignPtr sfp $ \sptr ->
+              lenientLoop dfp dptr
+              sptr
+              (plusPtr sptr slen)
+      where
+        !q = slen `quot` 2
diff --git a/Data/ByteString/Base16/Internal.hs b/Data/ByteString/Base16/Internal.hs
index 2907f8a..59a7fb9 100644
--- a/Data/ByteString/Base16/Internal.hs
+++ b/Data/ByteString/Base16/Internal.hs
@@ -1,3 +1,4 @@
+{-# LANGUAGE CPP #-}
 {-# LANGUAGE BangPatterns #-}
 {-# LANGUAGE MagicHash #-}
 module Data.ByteString.Base16.Internal
@@ -6,6 +7,8 @@ module Data.ByteString.Base16.Internal
 , decodeLoop
 , lenientLoop
   -- * utils
+, mkBS
+, withBS
 , c2w
 , aix
 , reChunk
@@ -67,7 +70,7 @@ decodeLoop !dfp !dptr !sptr !end = go dptr sptr
     !hi = "\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00\x10\x20\x30\x40\x50\x60\x70\x80\x90\xff\xff\xff\xff\xff\xff\xff\xa0\xb0\x

     go !dst !src
-      | src == end = return (Right (PS dfp 0 (dst `minusPtr` dptr)))
+      | src == end = return (Right (mkBS dfp (dst `minusPtr` dptr)))
       | otherwise = do
         !x <- peek src
         !y <- peek (plusPtr src 1)
@@ -98,7 +101,7 @@ lenientLoop !dfp !dptr !sptr !end = goHi dptr sptr 0
     !hi = "\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\x00\x10\x20\x30\x40\x50\x60\x70\x80\x90\xff\xff\xff\xff\xff\xff\xff\xa0\xb0\x

     goHi !dst !src !n
-      | src == end = return (PS dfp 0 n)
+      | src == end = return (mkBS dfp n)
       | otherwise = do
         !x <- peek src

@@ -109,7 +112,7 @@ lenientLoop !dfp !dptr !sptr !end = goHi dptr sptr 0
         else goLo dst (plusPtr src 1) a n

     goLo !dst !src !a !n
-      | src == end = return (PS dfp 0 n)
+      | src == end = return (mkBS dfp n)
       | otherwise = do
         !y <- peek src

@@ -126,6 +129,23 @@ lenientLoop !dfp !dptr !sptr !end = goHi dptr sptr 0
 -- ------------------------------------------------------------------ --
 -- Utils

+mkBS :: ForeignPtr Word8 -> Int -> ByteString
+#if MIN_VERSION_bytestring(0,11,0)
+mkBS dfp n = BS dfp n
+#else
+mkBS dfp n = PS dfp 0 n
+#endif
+{-# INLINE mkBS #-}
+
+withBS :: ByteString -> (ForeignPtr Word8 -> Int -> r) -> r
+#if MIN_VERSION_bytestring(0,11,0)
+withBS (BS !sfp !slen)       kont = kont sfp slen
+#else
+withBS (PS !sfp !soff !slen) kont = kont (plusPtr pstr soff) slen
+#endif
+{-# INLINE withBS #-}
+
+
 aix :: Word8 -> Addr# -> Word8
 aix (W8# w) table = W8# (indexWord8OffAddr# table (word2Int# w))
 {-# INLINE aix #-}
diff --git a/base16-bytestring.cabal b/base16-bytestring.cabal
index 13f933b..eed9668 100644
--- a/base16-bytestring.cabal
+++ b/base16-bytestring.cabal
@@ -58,7 +58,7 @@ library

   build-depends:
       base        >=4   && <5
-    , bytestring  >=0.9 && <0.11
+    , bytestring  >=0.9 && <0.12

   ghc-options:      -Wall -funbox-strict-fields
   default-language: Haskell2010

Seems to the trick, except I cannot test it on GHC-7.10.3 and older as at least text needs some (hopefully similar) changes too.

This bytestring-0.11 release for (already late) GHC-9.0 is very bad timing.

emilypi commented 4 years ago

Ah! I didn't look at the changeset too deeply. If that's all we need, then that's great

phadej commented 4 years ago

The relaxing revision on base16-bytestring is incorrect. https://hackage.haskell.org/package/base16-bytestring-1.0.0.0/revisions/

Data/ByteString/Base16/Internal.hs:70:37:
    Not in scope: data constructor ‘PS’
    Perhaps you meant ‘BS’ (imported from Data.ByteString.Internal)

Please undo the revision (to bytestring <0.11), or require GHC-8.0 (i.e. base >=4.9).

I'm sorry if I was inmprecise. base16-bytestring-1.0.0.0 was fine with its original bounds. I didn't mean it was safe to relax.

emilypi commented 4 years ago

Yeah, sorry that was my bad - i forgot to revert that change when I fully understood what you were saying! It's revised now.

emilypi commented 3 years ago

Fixed as of #15