Stream correctness and doc fixes

benchmark/Streamly/Benchmark/Data/Stream/Nesting/LogicUnfold.hs

@@ -78,7 +78,7 @@ fairUnfoldSchedEqn maxVal input ints =
 -- GHC 9.14.1 cannot fuse InterleaveState, though 9.10 can
 {-# ANN unfoldCrossBounded (PermitTypes [''Int,''Maybe,''InterleaveState]) #-}
 {-# ANN unfoldCrossBounded (PermitTypeClasses []) #-}
-{-# ANN unfoldCrossBounded (MaxCoreSize 20000) #-}
+{-# ANN unfoldCrossBounded (MaxCoreSize 26000) #-}
 {-# NOINLINE unfoldCrossBounded #-}
 unfoldCrossBounded :: Int -> IO ()
 unfoldCrossBounded maxVal = unfoldCrossEqn maxVal (Type.boundedIntsUnfold maxVal 0)

core/docs/Changelog.md

@@ -32,6 +32,18 @@
 * Fix `Scanl.uniq` and `uniqBy` failing before the first input.
 * Fix `Scanl.partition`, `partitionBy` and `partitionByM` not finalizing one
   scan when the other terminates.
+* Fix `Stream.foldr1` combining the first element last, e.g.
+  `foldr1 (-)` on `[1,2,3]` returned `0` instead of `2`.
+* Fix `Stream.isInfixOf` returning `False` for an empty infix when the
+  stream has fewer than two elements.
+* `enumerateFromTo` and `enumerateFromThenTo` for `Ratio` now round the
+  limit to the nearest step, and `Enumerable` for `Fixed` now steps by the
+  resolution of the type, the same as their `Enum` instances.
+* Fix `enumerateFromToNum` and `enumerateDownFromToNum` emitting one element
+  beyond the limit when the difference between the limits is not a whole
+  number.
+* Fix `enumerateFromThen` and `enumerateFromThenTo` for `Natural` failing
+  with an underflow when enumerating downwards.
 
 * On POSIX platforms, the `Path` based APIs that open an existing file in
   `WriteMode`, e.g. `write` and `withFile` in `Streamly.FileSystem.FileIO`,

core/src/Streamly/Internal/Data/MutArray/Type.hs

@@ -953,6 +953,7 @@ putIndices arr = FL.foldlM' step (return ())
 -- Unsafe because it does not check the bounds of the array.
 --
 -- /Pre-release/
+{-# INLINE unsafeModifyIndex #-}
 modifyIndexUnsafe, unsafeModifyIndex :: forall m a b. (MonadIO m, Unbox a) =>
     Int -> MutArray a -> (a -> (a, b)) -> m b
 unsafeModifyIndex i MutArray{..} f = liftIO $ do
@@ -966,6 +967,7 @@ unsafeModifyIndex i MutArray{..} f = liftIO $ do
 -- | Modify a given index of an array using a modifier function.
 --
 -- /Pre-release/
+{-# INLINE modifyIndex #-}
 modifyIndex :: forall m a b. (MonadIO m, Unbox a) =>
     Int -> MutArray a -> (a -> (a, b)) -> m b
 modifyIndex i MutArray{..} f = do

core/src/Streamly/Internal/Data/Producer.hs

@@ -633,9 +633,10 @@ enumerateFromStep (x, stride) =
 
 -- | State for 'enumerateFromThenToIntegral'. @EnumInit from next to@ is the
 -- starting state carrying the arguments; it transitions to 'EnumYieldUpward'
--- or 'EnumYieldDownward', which carry the current value, the stride and
--- @to - stride@ (checked against before incrementing, so that the increment
--- itself cannot overflow past the bound).
+-- or 'EnumYieldDownward', which carry the current value, the stride and the
+-- limit checked before each step, so that the step cannot overflow past the
+-- bound. 'EnumYieldUpward' carries @next - from@ and @to - stride@,
+-- 'EnumYieldDownward' carries @from - next@ and @to + stride@.
 ANN_TYPE(EnumState,Fuse)
 data EnumState a =
       EnumInit a a a
@@ -675,21 +676,23 @@ enumerateFromThenTo (EnumInit from next to) =
                 then Stop
                 else Skip (EnumSingle from)
             else -- from >= next >= to
-                let !stride = next - from
-                in Skip $ EnumYieldDownward from stride (to - stride)
+                -- Use a positive stride, next - from would underflow for
+                -- unsigned types like Natural.
+                let !stride = from - next
+                in Skip $ EnumYieldDownward from stride (to + stride)
 enumerateFromThenTo (EnumYieldUpward x stride toMinus) =
     pure $
         if x <= toMinus
         then
             let !next = x + stride
              in Yield x $ EnumYieldUpward next stride toMinus
         else Skip (EnumSingle x)
-enumerateFromThenTo (EnumYieldDownward x stride toMinus) =
+enumerateFromThenTo (EnumYieldDownward x stride toPlus) =
     pure $
-        if x >= toMinus
+        if x >= toPlus
         then
-            let !next = x + stride
-             in Yield x $ EnumYieldDownward next stride toMinus
+            let !next = x - stride
+             in Yield x $ EnumYieldDownward next stride toPlus
         else Skip (EnumSingle x)
 enumerateFromThenTo (EnumSingle x) = pure $ Yield x EnumStop
 enumerateFromThenTo EnumStop = pure Stop
@@ -746,42 +749,60 @@ enumerateDownFromThenTo s = fmap (fmap getDown) (enumerateUpFromThenTo s)
 ANN_TYPE(EnumToState,Fuse)
 data EnumToState a =
       EnumToInit a a
-    | EnumToYield !a a
+    | EnumToYield !a !a
     | EnumToStop
 
+-- The loop continues while x <= to - 1, which is the same as x + 1 <= to, so
+-- the last element does not exceed "to" even when "to - from" is not a whole
+-- number, e.g. for Double, Ratio or Fixed values. A single comparison per
+-- element is needed.
+--
+-- from + 1 cannot overflow because from < to. to - 1 cannot underflow (e.g.
+-- for Natural) because from + 1 <= to.
 {-# INLINE_LATE enumerateFromTo #-}
 enumerateFromTo ::
     (Applicative m, Num a, Ord a) => Producer m (EnumToState a) a
 enumerateFromTo (EnumToInit from to) =
     pure $
-        if to < from
-        then Stop
-        else Skip $ EnumToYield from to
-enumerateFromTo (EnumToYield from to) =
+        if to <= from
+        then if to < from then Stop else Yield from EnumToStop
+        else
+            let !next = from + 1
+             in if next > to
+                then Yield from EnumToStop
+                else Skip $ EnumToYield from (to - 1)
+enumerateFromTo (EnumToYield x toMinus) =
     pure $
-        if to > from
-        then Yield from (EnumToYield (from + 1) to)
+        if x <= toMinus
+        then Yield x (EnumToYield (x + 1) toMinus)
         -- Note: Usually multiple yields are problematic for fusion, but only
         -- if we are looping further, for a terminal yield it is in fact better
         -- to yield here rather than skipping to another state. We used to skip
         -- to another state here and because of that equations/unfoldCross
         -- benchmark could not fuse, by moving the yield here it fused
         -- perfectly. It would be nice to investigate the root cause and make a
         -- guideline from that.
-        else Yield from EnumToStop
+        else Yield x EnumToStop
 enumerateFromTo EnumToStop = pure Stop
 
+-- Mirror of enumerateFromTo, the loop continues while x >= to + 1. to + 1
+-- cannot overflow because to < from. x - 1 cannot underflow (e.g. for
+-- Natural) because x >= to + 1.
 {-# INLINE_LATE enumerateDownFromTo #-}
 enumerateDownFromTo ::
     (Applicative m, Num a, Ord a) => Producer m (EnumToState a) a
 enumerateDownFromTo (EnumToInit from to) =
     pure $
-        if to > from
-        then Stop
-        else Skip $ EnumToYield from to
-enumerateDownFromTo (EnumToYield from to) =
+        if to >= from
+        then if to > from then Stop else Yield from EnumToStop
+        else
+            let !toPlus = to + 1
+             in if toPlus > from
+                then Yield from EnumToStop
+                else Skip $ EnumToYield from toPlus
+enumerateDownFromTo (EnumToYield x toPlus) =
     pure $
-        if to < from
-        then Yield from (EnumToYield (from - 1) to)
-        else Yield from EnumToStop
+        if x >= toPlus
+        then Yield x (EnumToYield (x - 1) toPlus)
+        else Yield x EnumToStop
 enumerateDownFromTo EnumToStop = pure Stop

core/src/Streamly/Internal/Data/Stream/Eliminate.hs

@@ -104,11 +104,12 @@ import Streamly.Internal.Data.Stream.Type hiding (splitAt)
 
 {-# INLINE_NORMAL foldr1 #-}
 foldr1 :: Monad m => (a -> a -> a) -> Stream m a -> m (Maybe a)
-foldr1 f m = do
-     r <- uncons m
-     case r of
-         Nothing   -> return Nothing
-         Just (h, t) -> fmap Just (foldr f h t)
+foldr1 f = foldr step Nothing
+
+    where
+
+    step x Nothing = Just x
+    step x (Just acc) = Just (f x acc)
 
 -- | Fold a stream of optional values.
 --
@@ -652,9 +653,16 @@ isInfixOf :: (MonadIO m, Eq a, Enum a, Unbox a)
     => Stream m a -> Stream m a -> m Bool
 isInfixOf infx stream = do
     arr <- fold Array.create infx
-    -- XXX can use breakOnSeq instead (when available)
-    r <- null $ StreamD.drop 1 $ Nesting.splitSepBySeq_ arr Fold.drain stream
-    return (not r)
+    -- An empty infix occurs in every stream. The split below cannot detect
+    -- it in a stream of fewer than two elements, which has fewer than two
+    -- segments.
+    if Array.length arr == 0
+    then return True
+    else do
+        -- XXX can use breakOnSeq instead (when available)
+        r <- null
+            $ StreamD.drop 1 $ Nesting.splitSepBySeq_ arr Fold.drain stream
+        return (not r)
 
 -- Note: isPrefixOf uses the prefix stream only once. In contrast, isSuffixOf
 -- may use the suffix stream many times. To run in optimal memory we do not

core/src/Streamly/Internal/Data/Stream/Enumeration.hs

@@ -221,8 +221,10 @@ enumerateFromThenToNum from next to = Stream step EnumInit
                     then Stop
                     else Skip (EnumSingle from)
                 else -- from >= next >= to
-                    let !stride = next - from
-                    in Skip $ EnumYieldDownward from stride (to - stride)
+                    -- Use a positive stride, next - from would underflow for
+                    -- unsigned types like Natural.
+                    let !stride = from - next
+                    in Skip $ EnumYieldDownward from stride (to + stride)
 
     step _ (EnumYieldUpward x stride toMinus) =
         pure $
@@ -232,12 +234,12 @@ enumerateFromThenToNum from next to = Stream step EnumInit
                  in Yield x $ EnumYieldUpward nxt stride toMinus
             else Skip (EnumSingle x)
 
-    step _ (EnumYieldDownward x stride toMinus) =
+    step _ (EnumYieldDownward x stride toPlus) =
         pure $
-            if x >= toMinus
+            if x >= toPlus
             then
-                let !nxt = x + stride
-                 in Yield x $ EnumYieldDownward nxt stride toMinus
+                let !nxt = x - stride
+                 in Yield x $ EnumYieldDownward nxt stride toPlus
             else Skip (EnumSingle x)
 
     step _ (EnumSingle x) = pure $ Yield x EnumStop
@@ -318,6 +320,11 @@ data EnumToState a =
 -- >>> Stream.toList $ Stream.enumerateFromToNum (254 :: Word8) 255
 -- [254,255]
 --
+-- The last element does not exceed @to@:
+--
+-- >>> Stream.toList $ Stream.enumerateFromToNum 1.1 3.2
+-- [1.1,2.1,3.1]
+--
 -- Equivalent to the following but with better fusion:
 --
 -- >> enumerateUpFromThenToNum from (from + 1) to
@@ -331,15 +338,46 @@ enumerateFromToNum from to = Stream step EnumToInit
     -- Equivalent to the following but with better fusion:
     -- takeWhile (<= to) $ takeEndBy (== to) $ enumerateFromStepNum from 1
 
+    -- The loop continues while x <= to - 1, which is the same as
+    -- x + 1 <= to, so the last element does not exceed "to" even when
+    -- "to - from" is not a whole number, e.g. for Double, Ratio or Fixed
+    -- values. A single comparison per element is needed.
+    --
+    -- from + 1 cannot overflow because from < to. to - 1 is evaluated only
+    -- in the EnumToYield state, which is entered only when from + 1 <= to,
+    -- so it cannot underflow (e.g. for Natural).
     {-# INLINE_LATE step #-}
     step _ EnumToInit =
         pure $
-            if to < from
-            then Stop
-            else Skip $ EnumToYield from
+            if to <= from
+            then if to < from then Stop else Yield from EnumToStop
+            else
+                let !nxt = from + 1
+                 in if nxt > to
+                    then Yield from EnumToStop
+                    else Skip $ EnumToYield from
+    -- (to - 1) in the code below is not stored in the state, so that an
+    -- unfused stream does not allocate a larger state on each element. It is
+    -- not a strict binding either, that would wrap the Stream constructor in a
+    -- case and a stream used twice would then not fuse.
+    --
+    -- A strict binding in the closure, compared in the loop instead of
+    -- to - 1:
+    --
+    -- >    !toMinus = if from + 1 > to then to else to - 1
+    --
+    -- evaluates and unboxes "to" once, when the stream is created. When "to"
+    -- is boxed, the fused loop then does not load it on each element, the
+    -- equations/streamCross (bounded) benchmark takes 1.7 ms instead of
+    -- 3.1 ms. But a stream bound to a variable and used twice, e.g. as the
+    -- outer and the inner stream of StreamK.fairConcatFor after
+    -- StreamK.fromStream, is then shared instead of being inlined at each
+    -- use, the step function is not inlined and each element allocates a
+    -- Step, the equations/fairConcatForK (bounded) benchmark allocates 9.5%
+    -- more.
     step _ (EnumToYield x) =
         pure $
-            if to > x
+            if x <= to - 1
             then Yield x (EnumToYield (x + 1))
             else Yield x EnumToStop
     step _ EnumToStop = pure Stop
@@ -350,15 +388,21 @@ enumerateDownFromToNum from to = Stream step EnumToInit
 
     where
 
+    -- Mirror of enumerateFromToNum, the loop continues while x >= to + 1.
+    -- to + 1 is evaluated only when to < from, so it cannot overflow. x - 1
+    -- cannot underflow (e.g. for Natural) because x >= to + 1.
     {-# INLINE_LATE step #-}
     step _ EnumToInit =
         pure $
-            if to > from
-            then Stop
-            else Skip $ EnumToYield from
+            if to >= from
+            then if to > from then Stop else Yield from EnumToStop
+            else
+                if to + 1 > from
+                then Yield from EnumToStop
+                else Skip $ EnumToYield from
     step _ (EnumToYield x) =
         pure $
-            if to < x
+            if x >= to + 1
             then Yield x (EnumToYield (x - 1))
             else Yield x EnumToStop
     step _ EnumToStop = pure Stop
@@ -844,9 +888,57 @@ instance (CONSTRAINT) => Enumerable TYPE_NAME where {       \
     enumerateFromThenTo = enumerateFromThenToNum }
 
 ENUMERABLE_UNBOUNDED_NUM(Integer,)
-ENUMERABLE_UNBOUNDED_NUM(Natural,)
-ENUMERABLE_UNBOUNDED_NUM((Fixed a),HasResolution a)
-ENUMERABLE_UNBOUNDED_NUM((Ratio a),Integral a)
+
+-- 'Natural' is bounded below by 0, a downward enumeration stops at 0.
+instance Enumerable Natural where
+    {-# INLINE enumerateFrom #-}
+    enumerateFrom = enumerateFromNum
+    {-# INLINE enumerateFromThen #-}
+    enumerateFromThen from next =
+        if next >= from
+        then enumerateFromThenNum from next
+        else enumerateFromThenToNum from next 0
+    {-# INLINE enumerateFromTo #-}
+    enumerateFromTo = enumerateFromToNum
+    {-# INLINE enumerateFromThenTo #-}
+    enumerateFromThenTo = enumerateFromThenToNum
+
+-- Like the 'Enum' instance of 'Fixed', enumerates the underlying 'Integer',
+-- i.e. the step is the resolution of the type, not 1.
+instance Enumerable (Fixed a) where
+    {-# INLINE enumerateFrom #-}
+    enumerateFrom (MkFixed from) = fmap MkFixed $ enumerateFrom from
+    {-# INLINE enumerateFromThen #-}
+    enumerateFromThen (MkFixed from) (MkFixed next) =
+        fmap MkFixed $ enumerateFromThen from next
+    {-# INLINE enumerateFromTo #-}
+    enumerateFromTo (MkFixed from) (MkFixed to) =
+        fmap MkFixed $ enumerateFromTo from to
+    {-# INLINE enumerateFromThenTo #-}
+    enumerateFromThenTo (MkFixed from) (MkFixed next) (MkFixed to) =
+        fmap MkFixed $ enumerateFromThenTo from next to
+
+-- Like the 'Enum' instance of 'Ratio', the limit is rounded to the nearest
+-- step: elements up to @to + stride / 2@ are generated. 'Ratio' arithmetic is
+-- exact, so repeated addition is used instead of the numerically stable
+-- 'RealFloat' enumeration.
+instance Integral a => Enumerable (Ratio a) where
+    {-# INLINE enumerateFrom #-}
+    enumerateFrom = enumerateFromNum
+    {-# INLINE enumerateFromThen #-}
+    enumerateFromThen = enumerateFromThenNum
+    {-# INLINE enumerateFromTo #-}
+    enumerateFromTo from to =
+        takeWhile (<= to + 1 / 2) $ enumerateFromNum from
+    {-# INLINE enumerateFromThenTo #-}
+    enumerateFromThenTo from next to =
+        takeWhile predicate $ enumerateFromThenNum from next
+
+        where
+
+        mid = (next - from) / 2
+        predicate | next >= from  = (<= to + mid)
+                  | otherwise     = (>= to + mid)
 
 #define ENUMERABLE_REAL_FLOAT(FRACTIONAL_TYPE)                   \
 instance Enumerable FRACTIONAL_TYPE where {                      \

core/src/Streamly/Internal/Data/Stream/Generate.hs

@@ -542,7 +542,8 @@ fromFoldable = Prelude.foldr cons nil
 -- |
 -- >>> fromFoldableM = Prelude.foldr Stream.consM Stream.nil
 --
--- Construct a stream from a 'Foldable' containing pure values:
+-- Construct a stream from a 'Foldable' containing monadic actions. The
+-- actions are run in order when the stream is evaluated:
 --
 -- /WARNING: O(n^2), suitable only for a small number of
 -- elements in the stream/