diff --git a/Changelog.md b/Changelog.md --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,18 @@ +0.10.10.1 – June 2020 + + * Fix off-by-one infinite loop in primMapByteStringBounded ([#203]) + * Don't perform unaligned writes when it isn't known to be safe ([#133]) + * Improve the performance of sconcat for lazy and strict bytestrings ([#142]) + * Document inadvertent 0.10.6.0 behaviour change in findSubstrings + * Fix benchmark builds ([#52]) + * Documentation fixes + * Test fixes + +[#52]: https://github.com/haskell/bytestring/issues/52 +[#133]: https://github.com/haskell/bytestring/pull/133 +[#142]: https://github.com/haskell/bytestring/pull/142 +[#203]: https://github.com/haskell/bytestring/issues/203 + 0.10.10.0 July 2019 July 2019 * Build with GHC 8.8, and tests with QC 2.10+ @@ -41,4 +56,3 @@ * Fix strictness of lazy bytestring foldl' * Numerous minor documentation fixes * Various testsuite improvements - diff --git a/Data/ByteString.hs b/Data/ByteString.hs --- a/Data/ByteString.hs +++ b/Data/ByteString.hs @@ -1413,9 +1413,11 @@ {-# DEPRECATED findSubstring "findSubstring is deprecated in favour of breakSubstring." #-} --- | Find the indexes of all (possibly overlapping) occurences of a --- substring in a string. +-- | Find the indices of all non-overlapping occurences of a substring in a +-- string. -- +-- Note, prior to @0.10.6.0@ this function returned the indices of all +-- possibly-overlapping matches. findSubstrings :: ByteString -- ^ String to search for. -> ByteString -- ^ String to seach in. -> [Int] @@ -1432,6 +1434,14 @@ where (a, b) = breakSubstring pat (unsafeDrop n src) +-- In +-- [0.10.6.0]() +-- 'findSubstrings' was refactored to call an improved 'breakString' +-- implementation, but the refactored code no longer matches overlapping +-- strings. The behaviour change appears to be inadvertent, but the function +-- had already been deprecated for more than seven years. At this time +-- (@0.10.10.1@), the deprecation was twelve years in the past. +-- {-# DEPRECATED findSubstrings "findSubstrings is deprecated in favour of breakSubstring." #-} -- --------------------------------------------------------------------- diff --git a/Data/ByteString/Builder/ASCII.hs b/Data/ByteString/Builder/ASCII.hs --- a/Data/ByteString/Builder/ASCII.hs +++ b/Data/ByteString/Builder/ASCII.hs @@ -85,23 +85,25 @@ import Foreign -#if defined(INTEGER_GMP) +#if __GLASGOW_HASKELL__ >= 811 -#if !(MIN_VERSION_base(4,8,0)) +import GHC.Num.Integer +#define HAS_INTEGER_CONSTR 1 +#define quotRemInteger integerQuotRem# + +#elif defined(INTEGER_GMP) + +#define HAS_INTEGER_CONSTR 1 +#define IS S# + +# if !(MIN_VERSION_base(4,8,0)) import Data.Monoid (mappend) # endif -import Foreign.C.Types -import qualified Data.ByteString.Builder.Prim.Internal as P -import Data.ByteString.Builder.Prim.Internal.UncheckedShifts - ( caseWordSize_32_64 ) - # if __GLASGOW_HASKELL__ < 710 import GHC.Num (quotRemInteger) # endif -import GHC.Types (Int(..)) - # if __GLASGOW_HASKELL__ < 611 import GHC.Integer.Internals # else @@ -109,6 +111,14 @@ # endif #endif +#if HAS_INTEGER_CONSTR +import qualified Data.ByteString.Builder.Prim.Internal as P +import Data.ByteString.Builder.Prim.Internal.UncheckedShifts + ( caseWordSize_32_64 ) +import Foreign.C.Types +import GHC.Types (Int(..)) +#endif + ------------------------------------------------------------------------------ -- Decimal Encoding ------------------------------------------------------------------------------ @@ -304,7 +314,7 @@ -- Fast decimal 'Integer' encoding. ------------------------------------------------------------------------------ -#if defined(INTEGER_GMP) +#if HAS_INTEGER_CONSTR -- An optimized version of the integer serialization code -- in blaze-textual (c) 2011 MailRank, Inc. Bryan O'Sullivan -- . It is 2.5x faster on Int-sized integers and 4.5x faster @@ -323,7 +333,7 @@ -- | Decimal encoding of an 'Integer' using the ASCII digits. integerDec :: Integer -> Builder -integerDec (S# i#) = intDec (I# i#) +integerDec (IS i#) = intDec (I# i#) integerDec i | i < 0 = P.primFixed P.char8 '-' `mappend` go (-i) | otherwise = go ( i) diff --git a/Data/ByteString/Builder/Prim.hs b/Data/ByteString/Builder/Prim.hs --- a/Data/ByteString/Builder/Prim.hs +++ b/Data/ByteString/Builder/Prim.hs @@ -633,9 +633,9 @@ -- using a 'BoundedPrim'. For example, we can write a 'Builder' that filters -- a strict 'S.ByteString' as follows. -- --- > import Data.ByteString.Builder.Primas P (word8, condB, emptyB) +-- > import qualified Data.ByteString.Builder.Prim as P -- --- > filterBS p = P.condB p P.word8 P.emptyB +-- > filterBS p = P.condB p (P.liftFixedToBounded P.word8) P.emptyB -- {-# INLINE primMapByteStringBounded #-} primMapByteStringBounded :: BoundedPrim Word8 -> S.ByteString -> Builder @@ -652,7 +652,7 @@ touchForeignPtr ifp -- input buffer consumed k br - | op0 `plusPtr` bound < ope = + | op0 `plusPtr` bound <= ope = goPartial (ip0 `plusPtr` min outRemaining inpRemaining) | otherwise = return $ bufferFull bound op0 (goBS ip0) diff --git a/Data/ByteString/Builder/Prim/Binary.hs b/Data/ByteString/Builder/Prim/Binary.hs --- a/Data/ByteString/Builder/Prim/Binary.hs +++ b/Data/ByteString/Builder/Prim/Binary.hs @@ -272,9 +272,6 @@ int64LE = fromIntegral >$< word64LE --- TODO: Ensure that they are safe on architectures where an unaligned write is --- an error. - -- | Encode a single native machine 'Int'. The 'Int's is encoded in host order, -- host endian form, for the machine you are on. On a 64 bit machine the 'Int' -- is an 8 byte value, on a 32 bit machine, 4 bytes. Values encoded this way diff --git a/Data/ByteString/Builder/Prim/Internal.hs b/Data/ByteString/Builder/Prim/Internal.hs --- a/Data/ByteString/Builder/Prim/Internal.hs +++ b/Data/ByteString/Builder/Prim/Internal.hs @@ -198,7 +198,19 @@ {-# INLINE CONLIKE storableToF #-} storableToF :: forall a. Storable a => FixedPrim a +-- Not all architectures are forgiving of unaligned accesses; whitelist ones +-- which are known not to trap (either to the kernel for emulation, or crash). +#if defined(i386_HOST_ARCH) || defined(x86_64_HOST_ARCH) \ + || ((defined(arm_HOST_ARCH) || defined(aarch64_HOST_ARCH)) \ + && defined(__ARM_FEATURE_UNALIGNED)) \ + || defined(powerpc_HOST_ARCH) || defined(powerpc64_HOST_ARCH) \ + || defined(powerpc64le_HOST_ARCH) storableToF = FP (sizeOf (undefined :: a)) (\x op -> poke (castPtr op) x) +#else +storableToF = FP (sizeOf (undefined :: a)) $ \x op -> + if (ptrToWordPtr op) `mod` (fromIntegral (alignment (undefined :: a))) == 0 then poke (castPtr op) x + else with x $ \tp -> copyBytes op (castPtr tp) (sizeOf (undefined :: a)) +#endif {- {-# INLINE CONLIKE liftIOF #-} diff --git a/Data/ByteString/Internal.hs b/Data/ByteString/Internal.hs --- a/Data/ByteString/Internal.hs +++ b/Data/ByteString/Internal.hs @@ -91,9 +91,14 @@ import Foreign.C.String (CString) -#if !(MIN_VERSION_base(4,11,0)) && MIN_VERSION_base(4,9,0) -import Data.Semigroup (Semigroup((<>))) +#if MIN_VERSION_base(4,13,0) +import Data.Semigroup (Semigroup (sconcat)) +import Data.List.NonEmpty (NonEmpty ((:|))) +#elif MIN_VERSION_base(4,9,0) +import Data.Semigroup (Semigroup ((<>), sconcat)) +import Data.List.NonEmpty (NonEmpty ((:|))) #endif + #if !(MIN_VERSION_base(4,8,0)) import Data.Monoid (Monoid(..)) #endif @@ -156,6 +161,7 @@ #if MIN_VERSION_base(4,9,0) instance Semigroup ByteString where (<>) = append + sconcat (b:|bs) = concat (b:bs) #endif instance Monoid ByteString where @@ -264,8 +270,8 @@ go !_ !0 cs = return (len, cs) go !p !n (c:cs) = poke p (c2w c) >> go (p `plusPtr` 1) (n-1) cs --- Unpacking bytestrings into lists effeciently is a tradeoff: on the one hand --- we would like to write a tight loop that just blats the list into memory, on +-- Unpacking bytestrings into lists efficiently is a tradeoff: on the one hand +-- we would like to write a tight loop that just blasts the list into memory, on -- the other hand we want it to be unpacked lazily so we don't end up with a -- massive list data structure in memory. -- diff --git a/Data/ByteString/Lazy/Internal.hs b/Data/ByteString/Lazy/Internal.hs --- a/Data/ByteString/Lazy/Internal.hs +++ b/Data/ByteString/Lazy/Internal.hs @@ -51,8 +51,12 @@ import Data.Word (Word8) import Foreign.Storable (Storable(sizeOf)) -#if !(MIN_VERSION_base(4,11,0)) && MIN_VERSION_base(4,9,0) -import Data.Semigroup (Semigroup((<>))) +#if MIN_VERSION_base(4,13,0) +import Data.Semigroup (Semigroup (sconcat)) +import Data.List.NonEmpty (NonEmpty ((:|))) +#elif MIN_VERSION_base(4,9,0) +import Data.Semigroup (Semigroup ((<>), sconcat)) +import Data.List.NonEmpty (NonEmpty ((:|))) #endif #if !(MIN_VERSION_base(4,8,0)) import Data.Monoid (Monoid(..)) @@ -84,6 +88,7 @@ #if MIN_VERSION_base(4,9,0) instance Semigroup ByteString where (<>) = append + sconcat (b:|bs) = concat (b:bs) #endif instance Monoid ByteString where diff --git a/Data/ByteString/Short/Internal.hs b/Data/ByteString/Short/Internal.hs --- a/Data/ByteString/Short/Internal.hs +++ b/Data/ByteString/Short/Internal.hs @@ -111,7 +111,7 @@ -- | A compact representation of a 'Word8' vector. -- --- It has a lower memory overhead than a 'ByteString' and and does not +-- It has a lower memory overhead than a 'ByteString' and does not -- contribute to heap fragmentation. It can be converted to or from a -- 'ByteString' (at the cost of copying the string data). It supports very few -- other operations. diff --git a/bytestring.cabal b/bytestring.cabal --- a/bytestring.cabal +++ b/bytestring.cabal @@ -1,5 +1,5 @@ Name: bytestring -Version: 0.10.10.0 +Version: 0.10.10.1 Synopsis: Fast, compact, strict and lazy byte strings with a list interface Description: An efficient compact, immutable byte string type (both strict and lazy) @@ -54,7 +54,9 @@ Maintainer: Duncan Coutts Homepage: https://github.com/haskell/bytestring Bug-reports: https://github.com/haskell/bytestring/issues -Tested-With: GHC==7.10.1, GHC==7.8.4, GHC==7.6.3, GHC==7.4.2, GHC==6.12.3 +Tested-With: GHC==8.10.1, GHC==8.8.3, GHC==8.6.5, GHC==8.4.4, GHC==8.2.2, + GHC==8.0.2, GHC==7.10.3, GHC==7.8.4, GHC==7.6.3, GHC==7.4.2, + GHC==7.2.2, GHC==7.0.4 Build-Type: Simple Cabal-Version: >= 1.10 extra-source-files: README.md Changelog.md @@ -133,7 +135,10 @@ install-includes: fpstring.h -- flags for the decimal integer serialization code - if impl(ghc >= 6.11) + if impl(ghc >= 8.11) + build-depends: ghc-bignum >= 1.0 + + if impl(ghc >= 6.11) && impl(ghc < 8.11) if !flag(integer-simple) cpp-options: -DINTEGER_GMP build-depends: integer-gmp >= 0.2 @@ -150,10 +155,19 @@ other-modules: Rules QuickCheckUtils TestFramework + Data.ByteString + Data.ByteString.Char8 + Data.ByteString.Internal + Data.ByteString.Lazy + Data.ByteString.Lazy.Char8 + Data.ByteString.Lazy.Internal + Data.ByteString.Short + Data.ByteString.Short.Internal + Data.ByteString.Unsafe hs-source-dirs: . tests build-depends: base, ghc-prim, deepseq, random, directory, test-framework, test-framework-quickcheck2, - QuickCheck >= 2.10 && < 2.14 + QuickCheck >= 2.10 && < 2.15 c-sources: cbits/fpstring.c include-dirs: include ghc-options: -fwarn-unused-binds @@ -191,14 +205,31 @@ type: exitcode-stdio-1.0 hs-source-dirs: . tests tests/builder main-is: TestSuite.hs - other-modules: Data.ByteString.Builder.Tests - Data.ByteString.Builder.Prim.Tests + other-modules: Data.ByteString + Data.ByteString.Internal + Data.ByteString.Lazy + Data.ByteString.Lazy.Internal + Data.ByteString.Short + Data.ByteString.Short.Internal + Data.ByteString.Unsafe + Data.ByteString.Builder + Data.ByteString.Builder.ASCII + Data.ByteString.Builder.Extra + Data.ByteString.Builder.Internal + Data.ByteString.Builder.Prim + Data.ByteString.Builder.Prim.ASCII + Data.ByteString.Builder.Prim.Binary + Data.ByteString.Builder.Prim.Internal + Data.ByteString.Builder.Prim.Internal.Base16 + Data.ByteString.Builder.Prim.Internal.Floating + Data.ByteString.Builder.Prim.Internal.UncheckedShifts Data.ByteString.Builder.Prim.TestUtils + Data.ByteString.Builder.Prim.Tests + Data.ByteString.Builder.Tests TestFramework - build-depends: base, ghc-prim, deepseq, - QuickCheck >= 2.10 && < 2.14, + QuickCheck >= 2.10 && < 2.15, byteorder == 1.0.*, dlist >= 0.5 && < 0.9, directory, diff --git a/tests/Properties.hs b/tests/Properties.hs --- a/tests/Properties.hs +++ b/tests/Properties.hs @@ -1240,31 +1240,47 @@ prop_tailsBB xs = tails xs == map P.unpack (P.tails (P.pack xs)) -prop_findSubstringsBB s x l - = C.findSubstrings (C.pack p) (C.pack s) == naive_findSubstrings p s +-- The correspondence between the test 'ByteString' and naive test 'String' +-- must be injective, otherwise the ByteString may find matches at positions +-- that don't match in the "corresponding" string. To that end, we start +-- with and pack a Word8 array, rather than a unicode String. +-- +prop_findSubstringsBB :: [Word8] -> Int -> Int -> Bool +prop_findSubstringsBB ws x l + = let bstr = P.pack ws + -- we look for some random substring of the test string + slice = C.take l $ C.drop x bstr + str = C.unpack bstr + substr = C.unpack slice + in C.findSubstrings slice bstr == naive_findSubstrings substr str where - _ = l :: Int - _ = x :: Int - - -- we look for some random substring of the test string - p = take (model l) $ drop (model x) s - -- naive reference implementation + -- Note, overlapping matches have been broken since 2015, so at this + -- point just test for the current behaviour. naive_findSubstrings :: String -> String -> [Int] - naive_findSubstrings p s = [x | x <- [0..length s], p `isPrefixOf` drop x s] + naive_findSubstrings p q + | null p = [0..length q] + | otherwise = go 0 (length p) p (length q) q + go n !lp p !lq q = + if (lp > lq) + then [] + else if p `isPrefixOf` q + then n : go (n + lp) lp p (lq - lp) (drop lp q) + else go (n + 1) lp p (lq - 1) (tail q) -prop_findSubstringBB s x l - = C.findSubstring (C.pack p) (C.pack s) == naive_findSubstring p s +-- See above re injective string -> bytestring correspondence. +prop_findSubstringBB :: [Word8] -> Int -> Int -> Bool +prop_findSubstringBB ws x l + = let bstr = P.pack ws + -- we look for some random substring of the test string + slice = C.take l $ C.drop x bstr + str = C.unpack bstr + substr = C.unpack slice + in C.findSubstring slice bstr == naive_findSubstring substr str where - _ = l :: Int - _ = x :: Int - - -- we look for some random substring of the test string - p = take (model l) $ drop (model x) s - -- naive reference implementation naive_findSubstring :: String -> String -> Maybe Int - naive_findSubstring p s = listToMaybe [x | x <- [0..length s], p `isPrefixOf` drop x s] + naive_findSubstring p q = listToMaybe [x | x <- [0..length q], p `isPrefixOf` drop x q] -- correspondance between break and breakSubstring prop_breakSubstringBB c l diff --git a/tests/Rules.hs b/tests/Rules.hs --- a/tests/Rules.hs +++ b/tests/Rules.hs @@ -11,6 +11,7 @@ import qualified Data.ByteString.Lazy.Char8 as D import Data.List import Data.Char +import Data.Word import QuickCheckUtils @@ -21,7 +22,15 @@ #endif -prop_break_C x = C.break ((==) x) `eq1` break ((==) x) +prop_break_C :: Word8 -> C.ByteString -> Bool +prop_break_C w = C.break ((==) x) `eq1` break ((==) x) + where + -- Make sure we're not testing non-octet character values, for which + -- C.break is not isomorphic to breaking the corresponding string, + -- Instead start with a byte, and make a character out of that. + x = chr $ fromIntegral w + +prop_break_P :: Word8 -> C.ByteString -> Bool prop_break_P x = P.break ((==) x) `eq1` break ((==) x) prop_intercalate_P c = (\s1 s2 -> P.intercalate (P.singleton c) (s1 : s2 : [])) `eq2` diff --git a/tests/builder/Data/ByteString/Builder/Tests.hs b/tests/builder/Data/ByteString/Builder/Tests.hs --- a/tests/builder/Data/ByteString/Builder/Tests.hs +++ b/tests/builder/Data/ByteString/Builder/Tests.hs @@ -98,10 +98,22 @@ testProperty "hPutBuilder" testRecipe where testRecipe :: (UnicodeString, UnicodeString, UnicodeString, Recipe) -> Bool - testRecipe args@(UnicodeString before, - UnicodeString between, - UnicodeString after, recipe) = + testRecipe args = unsafePerformIO $ do + let (UnicodeString a1, UnicodeString a2, UnicodeString a3, recipe) = args +#if MIN_VERSION_base(4,5,0) + before = a1 + between = a2 + after = a3 +#else + -- See https://github.com/haskell/bytestring/issues/212 + -- write -> read does not roundrip with GHC 7.2 and + -- characters in the \xEF00-\xEFFF range. + safeChr = \c -> c < '\xEF00' || c > '\xEFFF' + before = filter safeChr a1 + between = filter safeChr a2 + after = filter safeChr a3 +#endif tempDir <- getTemporaryDirectory (tempFile, tempH) <- openTempFile tempDir "TestBuilder" -- switch to UTF-8 encoding